Skip to content

fix: normalize root_path_skip_prefixes in ProcessLinksMiddleware - #199

Open
lhoupert wants to merge 1 commit into
mainfrom
fix/normalize-skip-prefixes-in-middleware
Open

fix: normalize root_path_skip_prefixes in ProcessLinksMiddleware#199
lhoupert wants to merge 1 commit into
mainfrom
fix/normalize-skip-prefixes-in-middleware

Conversation

@lhoupert

@lhoupert lhoupert commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Addressing edge case 1 in #200: If you pass a skip prefix with a trailing slash straight to the middleware (e.g. ["/raster/"]), it never matches any link. The skip list just stops working and links to sibling apps get ROOT_PATH added again (the issue #198 tried to prevent). Configuration via env vars is unaffected as the Settings validator normalizes the prefixes (strips trailing slashes, drops empties) before they reach the middleware, but not ProcessLinksMiddleware when it is constructed directly (prefixes passed directly to ProcessLinksMiddleware dont get such normalization)

A trailing slash in a directly-passed prefix (e.g. ["/raster/"]) matched
nothing, silently disabling the skip and re-adding ROOT_PATH to sibling-app
links. Env-var configuration was unaffected (the Settings validator
normalizes), but ProcessLinksMiddleware is exported from
stac_auth_proxy.middleware and gave no error. Restore the middleware-level
normalization and its regression test.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@github-actions github-actions Bot added the fix label Jul 24, 2026
@claude

claude Bot commented Jul 24, 2026

Copy link
Copy Markdown

Claude finished @lhoupert's task in 1m 2s —— View job


Review complete

  • Gather context (diff, related code)
  • Review correctness/security/best-practices
  • Ponytail pass for over-engineering
  • Post final review

✅ No blocking issues — safe to merge.

The fix is minimal and correctly targeted: ProcessLinksMiddleware.__post_init__ (src/stac_auth_proxy/middleware/ProcessLinksMiddleware.py:34-40) normalizes root_path_skip_prefixes the same way config.py's parse_root_path_skip_prefixes validator does (strip trailing /, drop empties), fixing the gap where prefixes passed directly to the middleware (bypassing Settings) weren't normalized.

  • Confirmed JsonResponseMiddleware (the parent class) has no existing __post_init__ to shadow, so this doesn't skip any inherited initialization.
  • tuple(...) construction doesn't mutate the caller's original list, and dataclass field reassignment is safe here.
  • The new test case (tests/test_process_links.py:560-571) directly reproduces the reported bug (["/raster/"] silently matching nothing) and verifies the fix.
  • Edge case of a single "/" prefix rstrips to "" and is silently dropped — consistent with the config-layer behavior in spirit (config raises there, middleware just drops), which is a reasonable difference since this layer only normalizes rather than validates.

No security or correctness concerns. No test or lint gaps.

Simplify (ponytail)

Nothing to cut — this is already the minimal fix (one __post_init__, no new abstractions).


💰 Estimated review cost: $0.41 · 1m01s · 15 turns

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant