guard the user-action ledger's read path, not just its write path - #5847
Merged
Conversation
#5627 landed the #5605 structural guard on the file backend's `record` path, so an un-redirected suite can no longer write user-action-events.json into the developer's live data/ tree. `list` was left unguarded, so such a suite could still READ the live ledger — machine-local operator records the privacy ADR keeps off the wire and out of untethered test processes. Also hardens the guard test's own cleanup: it snapshotted the pre-existing file's bytes but only ever deleted a file it had created, so a guard regression on a MEMORY_BACKEND=file install left the developer's real ledger with the probe event appended. It now restores the snapshot. The guard message takes the attempted operation as an argument so the read path doesn't report itself as a write.
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
1 participant
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.
Summary
Follow-up to #5627 (merged), which landed the #5605 structural guard on the user-action ledger's write path. Three minor items surfaced in that review but weren't worth another round-trip with the contributor.
makeFileBackend().listcalledloadFileEvents()with no assertion, so an un-redirected suite could still pull the developer's live ledger into the test process vialistUserActions. Those rows are machine-local operator records the privacy ADR keeps off the wire — an untethered suite shouldn't read them either.MEMORY_BACKEND=fileinstall, a guard regression appended the probe event to the developer's real ledger and left it there. Cleanup now restores the snapshot when the bytes changed, and still only deletes a file this run leaked.assertTestDataRootRedirectednow takes the attempted operation as an argument, so the read path doesn't report itself asattempted a file-backend write.server/lib/README.md— thedataRoot.jsrow enumerates the module's exports and was missingresolveCodeRootForModule(added in test: guard user-action-events file-backend writes against leaking to real data/ #5627). The barrel test only matches the filename, so this wasn't caught.Test plan
userActionsDataRootGuard.test.jsgrows a read-path case; both cases bypass-probed — removing either assertion turns the corresponding test red, so neither passes vacuouslylistUserActions/recordUserActioncaller suite (routes/userActions,routes/cosTaskRoutes,routes/cos,services/cosToolRegistry,services/userActionReviewHooks,services/persistentMindUserActions,services/settings*,services/taskSchedule,services/cos*,lib/index,lib/dataRoot) — 680/680 passcd server && npm test— 37,400 pass, zero guard firings across the suite (so no existing suite is falsely tripped by guarding reads) and nodata/user-action-events.jsonafter the run