OPU-34: generalize the cradle check past the sample it was tuned against - #102
Merged
Merged
Conversation
…nst (D-126) VC-002f (critical/block) generalized 0-for-3 against faithful reproductions of three independently-disclosed 2026 npm campaigns (axios/plain-crypto-js, Mastra/easy-day-js, a Yandex-linked dependency-confusion cluster), even though VC-002e caught all three. Root cause was two structural gaps: asyncCradleRe stopped at the first statement after a fetch call (real callbacks routinely run several before the actual spawn), and interpreterSpawnRe only matched the interpreter as an isolated literal argument, missing execSync-with-a-command-string. Fix replaces the statement-boundary heuristic with a structural one: a regex-candidate span plus a Go-level check (namedFunctionDeclRe) that rejects any candidate crossing into a separately-declared named function — the shape a legitimate installer (esbuild) uses, versus the anonymous nested callbacks the three real campaigns use. interpreterSpawnRe was broadened to the full exec/execSync/execFile(Sync)?/spawn(Sync)? family. Independent review before merge found the shipped esbuild-shape negative test passed for the wrong reason (window-distance exclusion via 1500 bytes of filler, never reaching the named-function filter it claimed to prove). Fixed by adding a regression against the real esbuildLikeInstallJS fixture, where the same shape sits well within the window and is excluded only by the structural filter — mutation-proven to fail when that filter is removed, unlike the original. All three mechanisms (window widening, interpreterSpawnRe broadening, named-function filter) independently mutation-proven. Live CLI validation against a real npm project + lockfile confirms the full three-way table: BRIDGEHEAD still blocks, the esbuild FP still stays clean, all three campaign reproductions now block. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PLd1shywzWPsLgkpLxEyPj
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.
The gap
A true-positive generalization sweep (requested after OPU-32/33 merged) built faithful structural reproductions — IOCs replaced with inert RFC 5737/.invalid placeholders — of three independently-disclosed 2026 npm campaigns:
Run through the OPU-32/33 binary:
VC-002e(obfuscation+exec) generalized cleanly, 3/3.VC-002f(critical/block, the cradle check) generalized 0/3.Two distinct structural causes:
asyncCradleRe's window stopped at the first;after the fetch call. Real callbacks routinely run several statements (res.on('data', ...), buffering) before the actual spawn — the norm, not the exception.interpreterSpawnReonly matchedspawn/spawnSyncwith the interpreter as an isolated literal argument. Axios's real technique ischild_process.execSync(\cscript.exe //nologo //B "${path}"`, ...)` — wrong function name and wrong argument shape.The fix
asyncCradleCandidateRenow captures a broad candidate span (fetch call → callback introducer → up to 1,000 chars, now just a sanity bound). A new Go-level check,namedFunctionDeclRe, rejects any candidate whose span crosses into a named function declaration — the shape a legitimate installer (esbuild) uses (exec lives in a separately-declared, separately-called function), versus the anonymous nested callbacks the three real campaigns use. Same regex-candidate-plus-Go-filter pattern the codebase already uses for OPU-27's runner-target check.interpreterSpawnRewas broadened to the fullexec/execSync/execFile(Sync)?/spawn(Sync)?family, matching the interpreter name as a whole-word token anywhere inside the first quoted/backtick argument.A first attempt widened the window to a flat character count instead — it passed its own tests but immediately failed the repo's existing
esbuild_regression_test.go, because character distance isn't a stable signal (it depends entirely on how much unrelated code sits between two calls, which varies by an order of magnitude between a real file and a same-shape test double). That's documented in D-126 rather than hidden.Review finding and correction
Independent review before merge mutation-proved all three claimed mechanisms individually (revert → confirm expected test subset fails → restore → reconfirm green). The window widening and
interpreterSpawnRebroadening were both confirmed genuine and independently load-bearing.The named-function structural filter's proof took two attempts to get right. A self-authored corrected test using
execSync(as the trigger verb never matched at all (the exec-family alternation requires the literalchild_process.execimmediately followed by\s*\(). More significantly, the shippedopu34_test.go's own esbuild-shape negative test passed for the wrong reason: its fixture pads 1,500 bytes of filler between the fetch and the exec call, which alone pushes the span pastasyncCradleCandidateRe's 1,000-char window — the test never reachesnamedFunctionDeclReat all. Confirmed by removing the named-function rejection and observing the shipped test still passed.Fix: rather than patch the shipped synthetic, added
TestEsbuildInstallerNamedFunctionExcludesCradletoesbuild_regression_test.goagainst the real, un-paddedesbuildLikeInstallJSfixture, where the nearest exec call sits ~294 characters from the fetch call — well within the window, so exclusion can only come from the named-function filter itself. Mutation-proven: removing the filter's rejection fails this new test while leaving the shipped window-distance-based test passing, isolating exactly what each test does and doesn't prove. The shipped test's own comment was extended to document the distinction rather than leave a misleading claim standing.Validation
gofmt -lclean,go build ./...,go vet ./...silent.-raceclean oninternal/installsurface.Residual limitations (disclosed, not closed)
command.push('powershell.exe'); exec(command, ...), sudo-prompt's real pattern) still evadeinterpreterSpawnRe— needs light dataflow tracking, a bigger change than this patch.function launch(cmd) { spawn(cmd); }) would now be excluded even though it's genuinely part of the same cradle. Not observed in any of the three real samples; worth a note for the next FP/TP sweep.Files
internal/installsurface/analyze.go— the two structural fixes.internal/installsurface/opu34_test.go— shipped tests, with the esbuild-shape test's comment corrected to disclose what it actually proves.internal/installsurface/esbuild_regression_test.go— new non-vacuous regression against the real fixture.docs/DECISIONS.md— D-126.Generated by Claude Code