refactor: one adapter core, so a fifth copy cannot drift - #37
Merged
Conversation
Express, Fastify, Next.js (twice — the middleware and the Pages wrapper) and the fetch guard each carried their own copy of the same decisions: skip-path matching, the 429 payload, the 403 payload, honeytoken arming. Five copies of one set of rules. Not a tidiness complaint. The leftmost-X-Forwarded-For bug survived in two adapters after the same class had been fixed elsewhere, precisely because there was no one place to fix. Every copy is a place the next correction can fail to land, and each reads sensibly on its own, which is why review does not catch it. adapter-core.ts holds the decisions. The adapters keep the I/O, which differs genuinely: Express intercepts res.write/res.end, Fastify uses an onSend hook, a fetch handler rebuilds a Response, and the detail in each is hard-won. Fastify keeps its awaited arming. It derives during plugin registration, which is already an async boot phase, so it has no window where early requests are served without the link -- forcing it onto the fire-and-forget getter would have REGRESSED it. deriveAndArm() is the same logic with the other timing, not a second implementation. Every existing honeytoken-injection test passes unchanged, which was the acceptance criterion: the response mechanics are untouched. Three invariants stop the sixth copy, and writing them found a false positive worth keeping: nextjs/honeytoken.ts derives a token without arming, deliberately -- it is the render-side helper a layout calls, and the developer arms in middleware from the same HMAC. A second caller, not a second answer. Excluded with that reason rather than by loosening the rule. Closes WebDecoy/app#739
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.
Closes WebDecoy/app#739.
The duplication
Express, Fastify, Next.js twice (the middleware and the Pages-API wrapper) and the fetch guard each carried their own copy of: skip-path matching, the 429 payload, the 403 payload, honeytoken arming. Five copies of one set of decisions.
Not a tidiness complaint. The leftmost-
X-Forwarded-Forbug survived in two adapters after the same class of bug had been fixed elsewhere, precisely because there was no one place to fix it. Every copy is a place the next correction can fail to land — and each one reads perfectly sensibly on its own, which is why review does not catch it.What moved, and what deliberately didn't
adapter-core.tsholds the decisions. The adapters keep the I/O, which differs genuinely: Express interceptsres.write/res.end, Fastify uses anonSendhook, a fetch handler rebuilds aResponse. The detail in each is hard-won — the Angular SSRheadersSentcase, Content-Length correction — and none of it belongs in a shared module.Fastify keeps its awaited arming. It derives during plugin registration, which is already an async boot phase, so unlike Express it has no window where early requests are served without the link. Forcing it onto the fire-and-forget getter would have regressed it.
deriveAndArm()is the same logic with the other timing — a second entry point, not a second implementation.Verification
Every existing honeytoken-injection test passes unchanged, which was the acceptance criterion — the response mechanics are untouched. 455 tests, 20/20 turbo tasks, three edge entry points still clean.
Three new invariants stop the sixth copy: the refusal response, skip-path matching, and honeytoken derivation each exist in exactly one file. Verified by reintroducing a copy in the Express adapter and watching the test name it (
express/src/middleware.ts:336).The false positive worth keeping
The honeytoken invariant first flagged
nextjs/src/honeytoken.ts. That one derives a token without arming, on purpose: it is the render-side helper an App Router layout calls to print the link, and the developer arms the tripwire in their middleware from the sameactivePaths. Both sides derive the same HMAC from the API key, which is what makes them agree.A second caller, not a second answer. Excluded with that reason written down rather than by loosening the rule — a rule loosened to stop complaining stops working.
That's the second false positive one of these has caught today. Both times the fix was to sharpen the rule, and both times the rule was worth keeping.