Repository navigation
Create the host's own files private to the user - #2698
Merged
Merged
Conversation
The host's own files carry the user's content, and a write that names no mode takes the process umask: on a stock 022 the saved team, a Bot's memory, the chat index, the decision log, the OCR cache and inflight.json land 0644 in a 0755 home, and an atomic rewrite widens a file the owner narrowed to 0600. core/src/host/private-files.js is the one place those modes live: a file is 0600, the folder holding it 0700, and an atomic rewrite chmods the temp name to the mode of the file it replaces before the rename, so a chmod the owner made by hand survives the next remember. The chat index is tightened after SQLite creates it, and the files that stay ambient are the ones that are public or computed. Fixes #2697
Collaborator
Author
|
Verdict: PASS at fa7da82 Ran at this tip: Red first, at the same tree with Review:
|
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.
Written-by: Dev
Fixes #2697
Owner: Dev
Reviewer: Reviewer
What changed
The host's own files were created with whatever
umaskthe process inherited, and an atomic rewrite re-created them with that mode too, so on a stock022the saved team, a Bot's memory and its frozen snapshot, the chat index, the decision log, the OCR cache andinflight.jsonwere0644in a0755home — and achmod 0600an owner made by hand came back0644after the nextremember.core/src/host/private-files.jsis now the one place those modes live:ensurePrivateDir/ensurePrivateDirSync(mkdirwith0o700, thenchmod, becausemkdirleaves an existing folder alone and the umask filters the mode it does set) andstagePrivateFile/replacePrivateFile(write the temp name,chmodit to the mode the file being replaced already has, thenrename), pluswritePrivateFilefor the in-placeinflight.jsonwrite andtightenFilefor SQLite's own files. The writers changed:memory.js(MEMORY.md, topics, snapshots, journal, archive folder),store.js(homemkdirandstate.json),chat-index.js(the index and its-wal/-shm, tightened right after SQLite makes them, with the0700home covering a WAL file that appears between two checks),decisions.js(decisions.jsonland its trim),images.js(ocr.json) andresume.js(inflight.json).inflight.json(resume.js:78) carries no message body of its own: it records the Session, the room, the origin and the trigger of a turn that was running, plus the arguments of a delivery no turn has taken up yet — and those arguments are the message text. So it is the same kind of private thing asstate.json, and it is handled with the same rule (created0600in a0700home; an existing file keeps the mode it has, because it is written in place).Content that is public or computed may keep the ambient mode, but as a decision written where it is made: every writer this change touches names the mode of a file holding user content, and the writers outside the host's own state (
workflow-store.js,journal.js,secrets.js, plugin exports) are not changed here.docs/subsystems/core-host.mdstates the invariant. Tests:core/test/private-files.test.jssetsumask 022in-process (restored after each test), assertsmode & 0o777is0600/0700for each file and folder, covers a seeded-0600state.jsonafter a real save and achmod 0400MEMORY.mdafter the nextremember, and drives each writer's real module in a scratch home.Ran at this tip:
The new file is 7/7 green at the tip; the same file against the unmodified base blobs is 0/7 under
umask 022. The affected suites (memory,memory-empty,memory-count-read-failure,memory-panel-readonly,memory-search,decisions,follow-through,history,readonly-window-reads,retired-bot-paths,state-journal,legacy-state,state-link-answer,state-link-hardlink,project-state,project-state-unreadable,connector-access,i18n,project-board*,project-team,task-handoff-route,chat-link-open,bot-messages,private-files) are 490 tests green in three batches.Not run: the full
npm run verify(as instructed), and the wholecoresuite in one process — this sandbox runs out of threads past ~250 test files (pthread_create: Resource temporarily unavailable,spawn … EAGAIN), so the affected suites were run instead. The smoke test was not run (no desktop bench on this host).Checkpoint
{
"pr": 2698,
"claim": "Dev",
"issue": 2697,
"summary": "Every host file holding user content is now created 0600 inside a 0700 folder, whatever umask the process inherited. An atomic rewrite reads the mode of the file it replaces and chmods the temp name before the rename, so a chmod the owner made by hand survives the next remember. The chat index and its WAL files are tightened right after SQLite creates them, and inflight.json is created private. Public or computed content keeps the ambient mode as an explicit choice, and core-host.md states the invariant.",
"files": [
"core/src/host/private-files.js",
"core/src/host/memory.js",
"core/src/host/store.js",
"core/src/host/chat-index.js",
"core/src/host/decisions.js",
"core/src/host/images.js",
"core/src/host/resume.js",
"core/test/private-files.test.js",
"docs/subsystems/core-host.md",
"changelog.d/2697-private-files.fixed.md"
],
"checks": {
"ran": "node scripts/ds-bot-patches.mjs check && node scripts/ds-bot-patches.mjs canonical && node core/scripts/build-client.mjs --check && cd core && npm run lint && node scripts/lint-plugins.mjs && node .github/scripts/changelog.mjs check",
"result": "green",
"at": "fa7da8269e177d28847096f6629112f2a010783f"
},
"next": "Reviewer: core/src/host/private-files.js is the whole rule; the rest is call sites."
}