Fix three false SUPPORTED results in claim verification (v0.5.1) - #27
Merged
Merged
Conversation
An external review of v0.5.0 reproduced three cases where dtc verify reported SUPPORTED with no justifying evidence. All three reproduced against the released code. - admin-authorization matched a combined text that included the FILE PATH, so src/admin/permissions.ts satisfied its "permission" token and reported "1 of 1 administrative route(s) show an authorization check" on a repository with no guard anywhere. Security-adjacent, and the reason for this release. - route-test-coverage accepted name similarity as evidence: an unrelated test named "formats users display names" supported GET /users. Imports matched by substring (superusers counted as users) and HTTP methods were merged. - billing-webhook-signature accepted a verification helper anywhere in the repository, including one nothing called, as protection for every handler. Fixes: - The route extractor captures each route's own call arguments, so a guard is attributed to the route it actually wraps. Comments are stripped; imported but unapplied guards, guards in strings, and guards on a sibling route are no longer evidence. Authentication (requireAuth) is distinguished from authorization; router-level and server-wide guards are reported as unresolved, never as protected. - Test association requires an import matched on exact module stems; route identity keeps the HTTP method; name similarity is reported as an unverified suggestion that cannot raise status. The claim is presented as Route Test Association and no longer says "exercised by tests": static association is not execution coverage. - Webhook signature verification is connected per handler by file. Calls in unrelated modules or in test files no longer protect production handlers. Mixed repositories report per-handler counts. - Routes defined in test, example, and fixture directories are excluded from the application route inventory (Express reported 142 such routes; 9 are application surface). Honest trade-off: route-test-association now abstains on real repositories whose tests exercise routes through a running server rather than by import. Its v0.5.0 numbers were mostly name collisions. Request-based association is the next step. 157 tests (16 new); every reproduction and adversarial case is now a permanent regression test. Claim ids and JSON schema_version are unchanged. Version 0.5.1.
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
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.
Why
An external review of v0.5.0 reproduced three cases where
dtc verifyreported SUPPORTED with no justifying evidence. I re-ran all of them against released code before changing anything - all three reproduced:The admin case is the worst:
src/admin/permissions.tssatisfied the evaluator's"permission"token through its filename, because the matcher included the file path. A file's name became evidence about its behavior - the exact failure DevTime exists to catch, shipped inside DevTime.Fixes
Authorization must be at the route's own call site. The extractor now captures each route's own arguments. Not evidence anymore: guards imported but never applied, named in a comment, named in a string, or applied to a different route in the same file. Authentication is distinguished from authorization (
requireAuthestablishes identity, not permission). Router-level and server-wide guards are reported as unresolved, never as protected. Missing evidence stays WEAK, never CONTRADICTED.Test association requires an import, matched on exact module stems (
superusersno longer counts asusers). Route identity keeps the HTTP method. Name similarity becomes an explicit unverified suggestion that can never raise status. Presented as Route Test Association; the statement no longer says "exercised by tests" because static association is not execution coverage.Webhook verification is connected per handler (same file). Helpers elsewhere, and calls inside test files, no longer protect production handlers. Mixed repos report "1 of 2 handlers verify a provider signature".
Routes in test/example/fixture directories are not application surface. Express's inventory went from 142 routes - nearly all from its own
test/andexamples/- to 9.Honest trade-off, stated in the release notes
route-test-associationnow abstains far more often. On three real repos it reports zero associated routes, because their tests exercise routes through a running server (supertest, TestClient) rather than by importing modules. That's the correct trade - "I cannot establish this" beats a false "your routes are tested" - but it means v0.5.0's numbers were mostly name collisions. Request-based association is the next step, not a claim made today.Verification
The demo fixture was deliberately not edited to make its numbers look better - it honestly reports
0 of 3because its tests don't import the route implementations.Version 0.5.1. No tag, no publish - awaiting approval.