[sfmskywalker]

Closes #239. Program #58.

Problem

Core accepts / in definition, aspect and resource IDs. Clients escape such an ID as one path segment (a/b → a%2Fb). ASP.NET Core decodes every escape in the path except %2F, so the endpoints received a%2Fb. Every lookup by ID then returned 404, and so did writes, versions, activation and schema export.

Change

• A new internal helper, AsterRouteIds.Decode, runs at the 9 endpoints that take an ID:
  • definition get and schema;
  • aspect get, versions and schema;
  • resource get, PUT, versions, activate and deactivate.
• How it decodes:
  • A route value without % passes through unchanged.
  • Otherwise, the helper reads the matching segment of the raw request target and unescapes it once. That is the only way to tell an escaped / (a%2Fb) from the literal text %2F (a%252Fb), because the server has already decoded both to a%2Fb.
  • It first checks that the raw segments line up with the decoded path, including the path base. If they don't (dot-segment normalisation, a rewrite) or there is no raw target (TestServer), it falls back to reading %2F as /.
• The route table, the public API and the Aster.Elsa.Studio client are unchanged.
• The escaping rule is documented in the README and in wiki/AspNetCore-Endpoints.md.
• Separate commit: a nullable warning (CS8602) in an Aster.Elsa.Agents test added in Elsa agents: keep references to deleted agents saveable; refuse saves to deleted agents #235. The Release build of the solution now has 0 warnings.

Verification

• What routing leaves encoded was checked on Kestrel and TestServer (net8/9/10): only %2F/%2f. %25 is decoded, so %252F arrives as %2F.
• New tests, run over both TestServer and Kestrel:
  • Resources_IdsWithReservedCharacters_RoundTripThroughEscapedPaths
  • DefinitionsAndAspects_IdsWithReservedCharacters_RoundTripThroughEscapedPaths
  • Resources_IdsWithReservedCharacters_RoundTripUnderAPathBase
• IDs covered: /, %, +, space, #, ?&=, unicode, and literal %2F (Kestrel only).
• With the fix reverted, 15 of the 47 cases fail (every ID containing /, as a 404). With the raw-target step removed, the 4 literal-%2F cases fail.
• Aster.AspNetCore.Tests: 76/76 on net8.0, net9.0 and net10.0.
• Aster.Elsa.Studio.Tests: 99/99 on each framework.

---

Generated by Claude Code

Summary by CodeRabbit

• Bug Fixes

  • Resource, definition, and aspect endpoints now correctly handle IDs containing escaped slashes, percent signs, and other reserved characters.
  • Preserves literal %2F values when requests are sent through compatible hosting configurations.
  • Supports reliable routing when applications use a path base.

• Documentation

  • Added guidance for escaping reserved characters in endpoint IDs, including hosting-specific behavior.

• Tests

  • Added coverage for reserved-character IDs across resources, definitions, and aspects, including activation history and multiple hosting configurations.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: defaults
• Review profile: CHILL
• Plan: Advanced
• Run ID: 181dfeb2-f0f3-40bd-bfee-58eab93e215e

📥 Commits

Reviewing files that changed from the base of the PR and between bfbcc2c and 8665092.

📒 Files selected for processing: 6

• src/hosting/Aster.AspNetCore/AsterEndpointRouteBuilderExtensions.cs
• src/hosting/Aster.AspNetCore/AsterRouteIds.cs
• src/hosting/Aster.AspNetCore/README.md
• test/Aster.AspNetCore.Tests/AsterEndpointsTests.cs
• test/Aster.Elsa.Agents.Tests/AsterAgentStoreTests.cs
• wiki/AspNetCore-Endpoints.md

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 3 remain after this review.

📝 Walkthrough

The change adds decoding for escaped route IDs and applies it across resource, definition, and aspect endpoints. Tests cover reserved-character IDs with TestServer, Kestrel, and a path base. Documentation describes the encoding behavior.

Changes: Escaped route ID handling

Layer / File(s)	Summary
Recover escaped route IDs src/hosting/Aster.AspNetCore/AsterRouteIds.cs	AsterRouteIds.Decode recovers escaped IDs from the raw request target when possible. It uses a %2F replacement fallback when raw-target decoding is unavailable or does not match the route.
Use decoded IDs in endpoints src/hosting/Aster.AspNetCore/AsterEndpointRouteBuilderExtensions.cs	Definition, aspect, and resource handlers decode route IDs before lookups and related authorization, history, update, schema, and activation operations.
Validate and document escaped IDs test/Aster.AspNetCore.Tests/AsterEndpointsTests.cs, src/hosting/Aster.AspNetCore/README.md, wiki/AspNetCore-Endpoints.md	Tests cover reserved-character IDs across TestServer, Kestrel, and a path base. The documentation describes encoding and decoding behavior.

Agent store test adjustment

Layer / File(s)	Summary
Access the latest test version test/Aster.Elsa.Agents.Tests/AsterAgentStoreTests.cs	The test applies the null-forgiving operator when assigning the latest version and accesses its configuration aspect directly.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 86650

The escaped-ID handling is ready to merge after normal checks.

Security architecture risk: 🟡 Moderate · up to 86650

Existing tenant and resource authorization checks remain in place. However, when raw-path recovery fails, the fallback changes which resource some previously valid URLs identify. Requests intended for an ID containing literal “%2F” can instead read, update, or activate the slash-containing ID. Normal Kestrel recovery preserves the distinction; production exposure depends on the hosting and rewrite pipeline.

Retained concerns

• Medium · architecture · inferred: When raw-target recovery is unavailable or fails alignment checks, the fallback turns literal “%2F” into “/”. A correctly escaped request for a%2Fb can therefore select a/b, including for PUT and activation/deactivation. Compared with the base, this redirects previously working literal-ID requests. Successful mutation requires the selected resource to exist and pass its authorization and version checks; no authorization bypass is established. Normal Kestrel recovery distinguishes the IDs, but production rewrite and hosting exposure remains unresolved.

Security review details

Security Blast Radius

• inferred — The affected scope is ID-addressed definition, aspect, and resource operations in the host-selected tenant. The conditional integrity impact includes updates and activation state of the substituted resource. The inspected flow does not establish cross-tenant access or additional downstream privileges.

Security Findings and Attack Paths

• inferred — A caller-controlled escaped segment can resolve to a different logical ID when raw recovery fails. For a%252Fb, the fallback selects a/b rather than literal a%2Fb. A write then depends on the selected resource's existence, authorization, and applicable version checks. This establishes conditional target substitution, not a verified unauthorized attack path.

Trust Boundaries and Controls

• observed — Resource authorization receives the same recovered ID used by mutation, providing counterevidence to an internal authorization/target mismatch. Authentication policies, identity-bound tenant selection, and resource authorization remain host responsibilities. Default-tenant and allow-all behavior predates this PR and is not treated as a newly introduced vulnerability.

Resilience and Maintainability Implications

• observed — After target selection, the PR preserves lookup-before-authorization-before-mutation ordering, BaseVersion forwarding, activation flags, cancellation propagation, and awaited completion. Manager-owned concurrency and lifecycle sequencing are unchanged; provider-specific atomicity and recovery guarantees were not independently established.

Hardening Proposals

• proposed — When raw recovery cannot disambiguate an escaped slash from literal “%2F”, reject the ambiguous request or enforce an explicit host-specific identifier restriction rather than silently selecting another identity. Validate the policy with both colliding IDs present, including mutation and path-rewrite cases.

Pre-merge checks: 3 · 2

Failed checks: 2 warnings

Check name	Status	Explanation	Resolution
Out of Scope Changes check		The Aster.Elsa.Agents test change fixes a nullable warning in a test from #235. It has no connection to issue #239 or to escaped route IDs. The route helper, endpoint changes, route tests, and documen…	Remove the unrelated Aster.Elsa.Agents test change from this pull request, or submit it with the issue that owns the nullable-warning work.
Docstring Coverage		Docstring coverage is 15.79% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 19 functions across 4 files. (2 skipped: …	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 3 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely describes the main change: decoding escaped IDs in Aster.AspNetCore route values.
Linked Issues check		Issue #239 requires escaped resource IDs to reach MapAster endpoints, TestServer round-trip coverage, and client documentation. The PR adds AsterRouteIds.Decode and applies it before resource, definit…

Full details: Linked Issues check

Explanation

Issue #239 requires escaped resource IDs to reach MapAster endpoints, TestServer round-trip coverage, and client documentation. The PR adds AsterRouteIds.Decode and applies it before resource, definition, and aspect service calls. The helper handles encoded slashes and preserves literal %2F when the raw request target is available. The reported TestServer tests cover resource get, update, versions, activation, and deactivation for /, %, +, space, and Unicode IDs. README and wiki documentation describe the escaping rule.

Full details: Out of Scope Changes check

Explanation

The Aster.Elsa.Agents test change fixes a nullable warning in a test from #235. It has no connection to issue #239 or to escaped route IDs. The route helper, endpoint changes, route tests, and documentation are within scope.

Resolution

Remove the unrelated Aster.Elsa.Agents test change from this pull request, or submit it with the issue that owns the nullable-warning work.

Full details: Docstring Coverage

Explanation

Docstring coverage is 15.79% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 19 functions across 4 files. (2 skipped: 2 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

✨ Finishing Touches: 💡 1

📝 Generate docstrings: 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{Comment @coderabbitai help to get the list of available commands.}