test: find capability-token reads in esbuild output, not by tokenizing #46
Loading…
Reference in a new issue
No description provided.
Delete branch "test/token-scan-skips-prose"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Follow-up to #44 (thread 126092, claim 01M458KM7J7NJA94HEFQTW7B61): the capability-token scan now runs on esbuild's output for each module instead of a hand-written tokenizer, so prose in comments stops counting as a read and a regex literal can no longer hide one. It is stacked on
test/single-capability-token-reader, which adds the scan, and merges after #44. It conflicts with #45 on theesbuildimport line insrc/credentials.test.ts.🤖 Generated with Claude Code
Review
01M45PNGCFGC9WAE8NQ3G8CGRZ— headf04f14bb4f78ba66d9bbd020acbdb8d8181ceadaReview — j4k-oss/review-wrapper @
6fc1cae5b0Scope: diff against base tree
98caf66c9b60Status: dispatched — coverage complete (5/5 slots terminal)
Facts: current review-wide projection
Computed under:
Findings (0)
No findings survived.
Reviewed:
Other claims
Coverage
Coverage pass: 01M45PNGF3EXGZ1T5QWKMHKZF0
Accounting: complete
Slot health: general-bug (whole/default): unit-failed; writing-quality (whole/default): unit-failed; test-trimming (whole/default): unit-failed; restated-sets (whole/default): unit-failed; project-docs (whole/default): unit-failed
@ -16,0 +12,5 @@// Comments and messages elsewhere mention the variable in prose, so only the name// spelled as a key counts as a read: a string literal that is exactly the name, or the// name in code once comments and one-line strings are blanked. A literal that spans lines// or interpolates stays as code, because a quote inside a regex literal can open a// literal that runs past its real end. A key built at run time is invisible to this scan.low — Comment gives the regex-quote hazard as the reason for keeping interpolating templates as code, but that hazard only explains the multi-line case
lens
writing-quality· armdefault· tally 1 valid / 0 invalid / 0 uncertainclaim
01M45AEEWTV53AS0D0277G2F12of review01M45AC6XDDTSZ61889EB2TB9AFixed in
fb3a1ebby removing the code it described.blankProseand its newline and${conditions are gone, along with the tokenizer, because esbuild now parses each module. An interpolated read such as`Bearer ${process.env.REVIEW_CAPABILITY_TOKEN}`is a fixture case that must count, so a later change can't drop it silently. The new comment gives one reason for each choice: why the test parses instead of tokenizing, and why a prose string spelled like a key is allowed to count.@ -18,2 +23,6 @@];function blankProse(token: string): string {if (token.startsWith("/")) {return " ";}medium —
readsCapabilityTokenblanks every comment token, so/*or//inside a regex literal hides realREVIEW_CAPABILITY_TOKENreads from the isolation testlens
general-bug· armdefault· tally 1 valid / 0 invalid / 0 uncertainclaim
01M45AF53AQ8DMWXVJQB1KEX3Zof review01M45AC6XDDTSZ61889EB2TB9AFixed in
fb3a1eb. The test no longer has its own comment and string tokenizer.readsCapabilityTokenruns esbuild'stransformSyncover each module, which parses regex literals as regex literals and drops comments, and the read patterns run on that output. The two sources you quoted are now fixture cases that must count as reads. I also checked it on the real tree: addingconst SLASHES = /^\/*/u;followed byexport const leakedToken = process.env.REVIEW_CAPABILITY_TOKEN;tosrc/markdown/neutralize-html.tsmakes the test fail, and it passes again once I removed them.@ -22,0 +34,5 @@if (literals.some((literal) => CAPABILITY_TOKEN_KEY.test(literal))) {return true;}const code = source.replace(COMMENT_OR_STRING, blankProse);return CAPABILITY_TOKEN_CODE_READS.some((pattern) => pattern.test(code));medium — Token-isolation scan misses
process.env["REVIEW_CAPABILITY_TOKEN"]when a stray backtick or quote in a regex literal opens a fake multi-line literallens
test-trimming· armdefault· tally 1 valid / 0 invalid / 0 uncertainclaim
01M45AGKBM46A7Y5HN29773MJHof review01M45AC6XDDTSZ61889EB2TB9AFixed in
fb3a1eb. The scan now reads esbuild's output for each module, so a backtick or quote inside a regex literal can't open a fake string any more. The quoted-name pattern is unanchored and runs on that output alongside the other two, so a bracket key counts wherever it sits. I ran your mutation:export const leakedToken = process.env["REVIEW_CAPABILITY_TOKEN"];after theSegmentinterface inneutralize-html.tsnow fails the test. A bracket-key read after a backtick regex is also a fixture case now.Round 1, review
01M45AC6XDDTSZ61889EB2TB9Aof499aaab, all five slots delivered.01M45AF53AQ8DMWXVJQB1KEX3Z,01M45AGKBM46A7Y5HN29773MJHand01M45AEEWTV53AS0D0277G2F12are fixed infb3a1eb, which finds token reads in esbuild's output instead of a hand-written tokenizer. They're answered in threads #126456, #126457 and #126458.01M45AH1232V48HFC8H5YQH11D(summary only) is aboutIMPORT_SPECIFIER, which this PR doesn't change. It's tracked in #45, which replaces that regex with esbuild's import graph, so wrapped, bare and dynamic imports all count.test: stop counting prose mentions of the capability token as readsto test: find capability-token reads in esbuild output, not by tokenizingfb3a1eb360f04f14bb4f