test(web): hold every HTTP route to a declared auth expectation - #759
Conversation
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Automated approval for maintainer PR
All automated quality gates passed. See SECURITY_CONTROLS.md for compensating controls.
✅ Mutation Testing ResultsMutation Score: 80.00% (threshold: 60%)
What is mutation testing?Mutation testing measures test quality by introducing small changes (mutations) to the code and checking if tests detect them. A higher score means better test effectiveness.
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #759 +/- ##
==========================================
+ Coverage 87.64% 87.72% +0.07%
==========================================
Files 90 90
Lines 12054 12057 +3
==========================================
+ Hits 10565 10577 +12
+ Misses 1198 1191 -7
+ Partials 291 289 -2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Blocked on #756, not by anything in this branch.
Verified this branch adds none of its own: gosec on |
The authorization boundary was one prefix rule plus an allowlist inside
authMiddleware: anything under /api/ needs a token, everything else does not.
A route registered outside /api/ therefore shipped reachable without a token
and nothing failed. The two registration sites — NewServerWithAuth and
RegisterHealthEndpoints — each built their own mux, so neither the reviewer nor
a test saw the whole surface at once.
Extract the routes into one table that both sites consume, with an explicit
public flag per entry, and test the table four ways:
- every route is replayed through the real middleware chain and must answer
401 without a token unless it declares itself public
- the set of public routes is pinned to a literal list, so widening the
token-free surface means editing the test on purpose
- registering a route anywhere other than through the table fails, including
inside newMux, which would otherwise escape the replay
- authMiddleware may not dispatch on a path beyond its declared exemptions,
closing the variant where a handler is served from the middleware itself
and never registered on a mux at all
No route changed its reachability; the table records what the code already did.
Each of the four was verified by breaking it: marking /api/jobs/delete public,
registering /sneaky inside newMux, and adding a /backdoor path check to
authMiddleware each produce exactly the finding they should, naming the route.
Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
888dfda to
6390d30
Compare
|
There was a problem hiding this comment.
Automated approval for maintainer PR
All automated quality gates passed. See SECURITY_CONTROLS.md for compensating controls.
The authorization boundary was one prefix rule plus an allowlist inside
authMiddleware: anything under/api/needs a token, everything else doesnot. A route registered outside
/api/therefore shipped reachable without atoken and nothing failed. The two registration sites —
NewServerWithAuthandRegisterHealthEndpoints— each built their own mux, so neither a reviewer nora test saw the whole surface at once.
This extracts the routes into one table both sites consume, with an explicit
publicflag per entry, and tests it four ways:TestRouteAuthExpectationsTestPublicRoutesAreExactlyDeclaredTestRouteRegistrationIsCentralizednewMux— would escape the replayTestAuthMiddlewarePathDecisionsMatchAllowlistNo route changed its reachability. The table records what the code already
did; the flags were read off the existing behaviour.
Verification
Each gate was verified by breaking it, not by assuming it works:
/api/jobs/deletepublic →TestRouteAuthExpectations//api/jobs/deleteandTestPublicRoutesAreExactlyDeclaredboth fail/sneakyinsidenewMux→ "newMux registers a route outside the route table", with the file and line/backdoorpath check toauthMiddleware→ the allowlist diff names/backdoorgo test ./web/passes and golangci-lint reports 0 issues on the package.