test(httpapi): pin dot-segment collapse targets at 404 on trailing slash - #1015
Open
AmirF194 wants to merge 1 commit into
Open
test(httpapi): pin dot-segment collapse targets at 404 on trailing slash#1015AmirF194 wants to merge 1 commit into
AmirF194 wants to merge 1 commit into
Conversation
DELETE .../account/suppressions/.. and DELETE .../agents/{email}/suppressions/..
collapse onto deleteAccount and deleteAgent with a trailing slash (tokencanopy#792).
Today chi answers both with the canonical 404 envelope only because no
RedirectSlashes/StripSlashes/CleanPath middleware is registered on the
root router; nothing in the repo asserted it. Add a regression test
pinning that 404, so a future chi upgrade or middleware addition fails
this test instead of turning the collapse into a live destructive hit.
Priority 1 (the SDK guard) shipped in tokencanopy#909. Priorities 3 (web helper
rollout) and 4 (confirmation scaling) from the issue are out of scope
here.
Refs tokencanopy#792
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A dot-segment path collapse (
DELETE /v1/account/suppressions/..,DELETE /v1/agents/{email}/suppressions/..) resolves to the destructivedeleteAccount/deleteAgentroutes with a trailing slash. It's safe today only because chi's root router registers noRedirectSlashes/StripSlashes/CleanPathmiddleware, so the trailing-slash form 404s: a library default the repo never asserted. This adds a regression test pinning that 404, followingrouter_errors_test.go's existing table-test pattern, so a future chi upgrade or an accidental trailing-slash middleware addition fails CI instead of silently turning the collapse into a live destructive hit.Priority 1 from #792 (the SDK-side guard) already shipped in #909. This covers priority 2 (the router regression test). Priorities 3 (web
fetchcall-site hardening) and 4 (scaling confirmation to blast radius) are out of scope here.Operational risk
None, test-only change, no production code touched.
Test plan
go test ./internal/httpapi/...: full package suite passes, including both new sub-tests (account, agent).root.Use(middleware.RedirectSlashes)to confirm the new test goes red (301 instead of 404) before reverting; it catches the regression it's meant to catch.go vetandgofmt -lclean on the changed file;go test -coverputs the package at 87.0%, above the 73% floor in.testcoverage.yml.I ran the package suite standalone against a local Postgres container rather than through
make cover's full fixture wiring, so I can't speak to the coverage-gate job's exact numbers, only that this package's own floor is comfortably cleared.