test(privacy): pin the compiler/repair no-cookie guarantee with a tripwire - #117
Open
myselfsiddharth wants to merge 2 commits into
Open
test(privacy): pin the compiler/repair no-cookie guarantee with a tripwire#117myselfsiddharth wants to merge 2 commits into
myselfsiddharth wants to merge 2 commits into
Conversation
…pwire Closes #101. SC-04 was true and unpinned. `src/compiler/types.ts`'s Trajectory / TrajectoryStep / Fingerprint and `src/runner/types.ts`'s PageStateSnapshot have no cookie- or storage-shaped field, so session material was never representable in the compiler's input or the repair model's. Nothing made that fail loudly if it changed. The obvious test does not work. Serializing a real trajectory and grepping its keys passes for the wrong reason: adding `cookies?: string[]` to Fingerprint does not populate it in a committed fixture, so an instance-only check stays green through exactly the change it exists to catch, and it cannot see an `additionalProperties: false` someone relaxed. So the tripwire asserts at the three levels the guarantee actually rests on: - Type surface, via the TypeScript compiler API, walked recursively from each root type. Catches a field *declared* on any reachable interface, populated or not — which is erased at runtime and invisible to an instance walk. - Contract schema. Catches a forbidden property name, and an object definition that stopped closing itself. - Instances: real recordings, their compiled bundles, and a live capturePageState against a context that genuinely has a cookie set. Guard-proven, not asserted. Adding an unpopulated `cookies?: string[]` to Fingerprint fails three type-surface cases (including two-hop reachability from Trajectory) while all twenty schema and instance assertions stay green. Relaxing $defs.fingerprint.additionalProperties, declaring a `storage_state` property, and making capturePageState return the context's cookies each fail exactly their own level. Keys are compared after normalization (lowercased, separators stripped) so storage_state / storageState / StorageState are one entry, not three near-misses. The schema check is scoped to nodes that define an object type. An applicator fragment — cache-row.schema.json's allOf if/then pair — carries `properties` without being an object definition, and closing one would reject every property the branch does not itself mention. Not new enforcement: SC-04 stays "enforced by construction", now + test. npm run ci # green — 197 unit, 14 integration, lint-docs clean (47 docs) npm run test:canary # 13 pass Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
Author
|
@copilot Review this PR |
Co-authored-by: myselfsiddharth <99870733+myselfsiddharth@users.noreply.github.com>
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 #101. Follow-up to #37 (SC-04). Branched from
main; no overlap with #107–#110.SC-04 was true and unpinned.
src/compiler/types.ts'sTrajectory/TrajectoryStep/Fingerprintandsrc/runner/types.ts'sPageStateSnapshothave no cookie- or storage-shaped field, so session material was never representable in the compiler's input or the repair model's. Nothing made that fail loudly if it changed — which is what the issue asked for.The obvious test does not work
Serialize a real trajectory, grep its keys, assert none is
cookies. That passes for the wrong reason: addingcookies?: string[]toFingerprintdoes not populate it in a committed fixture, so an instance-only check stays green through exactly the change it exists to catch. It also cannot see anadditionalProperties: falsethat someone relaxed.So the tripwire asserts at the three levels the guarantee actually rests on, and each catches something the others structurally cannot:
propertiesnames, plusadditionalProperties: falseon every object definitioncapturePageStateThe type-surface level is the one that answers #101's actual worry ("a future field addition to any of these types"), and it is the one an instance walk cannot substitute for.
Guard-proven, not asserted
Every level was sabotaged and the failure observed, rather than claimed:
cookies?: string[]added toFingerprint, left unpopulated$defs.fingerprint.additionalProperties→truestorage_stateproperty declared in the schemacapturePageState()returnspage.context().cookies()That first row is the whole argument for the type-surface level: 20 of 23 assertions cannot see the change at all.
Note the first row also fails via two-hop reachability — the field is on
Fingerprint, but theTrajectoryandTrajectoryStepcases fail too, because the walk descendsTrajectory → steps[] → TrajectoryStep → pre_state → Fingerprint. There is a dedicated case pinning that the recursion actually reaches nested types, so the others cannot pass vacuously on a root they never opened.Two details worth the review
Keys are normalized before comparison (lowercased, separators stripped), so
storage_state,storageStateandStorageStateare one entry rather than three near-misses. A future field arrives in whichever convention its author reached for; a tripwire that knows one spelling has a gap in it. Thestorage_statesabotage above is caught by this and not by an exact-match list.The schema check is scoped to nodes that define an object type (
"type": "object"withproperties), and my first cut got this wrong — it flagged four real nodes.cache-row.schema.json'sallOfif/thenpair carriespropertieswithout being an object definition, and closing one would be a bug, not a tightening:additionalProperties: falseinside athenrejects every property that branch does not itself mention, i.e. the rest of the row. Called out in a comment at the check, because it is exactly the kind of thing a future reader would "tighten" into a false-positive machine.The live-browser case sets a real cookie on the context before capturing, so "no cookies present" is not the reason it passes — and it asserts the cookie's value did not ride along inside a permitted field either.
Scope
This is not new enforcement. SC-04's status goes from "enforced by construction" to "enforced by construction + test" — the construction was already correct and is unchanged. No source file is touched; the diff is one test plus the SC-04 write-up.
tests/unit/page-state.test.tsalready launches Chromium undertests/unit, so the live case follows an established convention rather than introducing browser usage to that directory.🤖 Generated with Claude Code