Fix .profile persistence marker FP: dot-anchored but identifier-suffix-shaped - #106
Merged
Merged
Conversation
…x-shaped (D-130) Follow-up to D-129's broader FP sweep of persistenceMarkers. Unlike .bashrc/.git/hooks/ (specific, multi-token names), ".profile" alone collides with the extremely common object-property/identifier suffix "profile" -- the bare substring matched user.profileImage, settings.profileData, and bare options.profile, none of them the shell dotfile. isWordMarker's boundary check only applies to markers with zero punctuation, a gap that happened to be safe for the rest of that class but not for this one. Unlike D-129's core.hooksPath finding, this has a genuine structural fix rather than an accepted tradeoff: the real technique always references .profile as a complete path component (quote/separator/ whitespace on both sides), while the FP shape always has an identifier character immediately before the dot. Routed .profile through containsWord -- the same dual-boundary check identifier-shaped markers already use -- via a small explicit boundaryCheckedPunctuationMarkers set, rather than introducing a new mechanism. Mutation-proven: reverting the loop condition to its pre-fix form fails all six constructed benign cases while real-technique cases keep passing either way. Live CLI validation confirms a synthetic install hook with only .profile-shaped property access now scores clean, while a real .profile append combined with a cradle still fires both VC-002f and VC-002g unaffected. 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.
Follow-up to D-129's broader FP sweep of
persistenceMarkers, reviewing every entry for the same class of risk.The gap
Most
persistenceMarkersentries are either word-bounded already or specific enough as literal strings to stay low-risk..profilestood out: unlike.bashrc/.git/hooks//mcp.json(each a specific, multi-token name), "profile" alone is a common object-property/identifier suffix, so the bare raw-substring match fired onuser.profileImage,settings.profileData,analytics.profileId, and even a bareoptions.profileaccessor — none of them the shell dotfile.isWordMarker's dual-boundary check (containsWord) only applies to markers made entirely of identifier characters —.profilecontains a., so it skipped that check entirely and fell back to a plain, unboundedstrings.Contains. That gap happened to be safe for the rest of that punctuation-anchored class (.bashrc,.git/hooks/,/etc/), just not for this one.Confirmed directly against the scanner (no external search needed — this pattern is generic enough to construct minimal reproductions without hunting for a real package, unlike
mcp.json/core.hooksPathin D-129). Live-checked several real complex-postinstall packages (puppeteer's realinstall.mjs, esbuild's real installer, canvas, sharp) — no live hit found, but that carries less weight here: this pattern needs no special-purpose tool to trigger, just any config object with a "profile" field accessed via dot notation.Why this one is fixable, unlike D-129's
core.hooksPathThe reason
core.hooksPathcouldn't be safely narrowed was that the benign and malicious cases are the same string shape with only an unobservable difference in intent. That's not true here. The real technique (fs.appendFileSync(path.join(os.homedir(), '.profile'), payload), or a shell>> ~/.profileappend) always references.profileas a complete, standalone path component — a quote, path separator, or whitespace on both sides. The FP shape always has an identifier character immediately before the.— the name of the property-holder (user,settings,options). That's a genuine, checkable structural difference.The fix
Added
boundaryCheckedPunctuationMarkers, a small explicit set of punctuation-anchored markers that still need the dual-boundarycontainsWordcheckisWordMarkernormally reserves for identifier-shaped markers (systemd,crontab)..profileis the only entry so far. No new mechanism — this reuses the exactcontainsWordhelper already in the file, just extending which markers route through it. Verified directly thatcontainsWordalone already correctly separates every constructed FP case from every real-technique case, including the trickiest one (window.chrome.profile, a bare accessor with nothing following it — the left-boundary check alone excludes it, since "e" from "chrome" precedes the.).Validation
TestPersistenceDotProfilePrecision(dotprofile_fp_test.go), following the exact structure of the pre-existingTestPersistenceStartupPrecisionFP-regression test: six benign cases that must not raise persistence, four real-technique shapes that must.isWordMarker(m)alone) fails all six benign cases with the exact FP behavior found in review, while the real cases keep passing regardless — confirming the fix is genuinely load-bearing and isolated to exactly the case it targets.gofmt -lclean,go build ./...,go vet ./...silent.-raceclean oninternal/installsurface..profile-shaped property access (opts.profile = opts.profile || settings.profileData) now scores clean ofVC-002g— previously would have fired. A real.profileappend combined with a piped-curl cradle still fires bothVC-002f(critical/block) andVC-002g(high/gate-eligible) correctly, unaffected by the fix.Files
internal/installsurface/analyze.go—boundaryCheckedPunctuationMarkersand the loop-condition fix.internal/installsurface/dotprofile_fp_test.go— new regression test.docs/DECISIONS.md— D-130.Generated by Claude Code