Repository navigation
windows: apply the mandatory write denies, and keep the directories above a kept deny in place - #579
Open
ronleizrowice-ant wants to merge 24 commits into
Open
ronleizrowice-ant wants to merge 24 commits into
ronleizrowice-ant wants to merge 24 commits into
Conversation
The Linux and macOS backends resolve the dangerous files and directories, plus .git/hooks and .git/config, against the working directory and deny writes to them whatever the configuration says. Windows had no counterpart. windowsGetCwdMandatoryDenyPaths reads the same two definitions, so the three platforms protect the same names. It returns only the paths that are there: a Windows deny is an ACE and an ACE needs an object, and the placeholder chain srt-win materializes for a missing deny target is permanent by design, which is the right answer for a denyWrite entry someone asked for and the wrong one for a name nobody did. The existence filter also covers what the Linux producer spells out as a .git-is-a-directory gate: nothing exists beneath a .git pointer file.
computeWindowsFsAccessSet unions the producer into denyWrite, so the paths are stamped once at initialize() alongside the configured denies. A path that is both granted and denied gets both ACEs and srt-win writes the deny first, so allowWrite does not lift one; filesystem.allowGitConfig lifts the .git/config deny as it does on the other platforms, and joins the raw inputs updateConfig's short-circuit compares. The M rows drive it on the Windows legs: a write to .git/hooks/pre-commit and to .bashrc refused with the hooks directory explicitly granted, an ordinary project file written, allowGitConfig lifting one deny and not the other, and git status still working with the set stamped.
The mandatory-deny section carried a Linux note and no Windows one, and the Windows known-limitations list named five other gaps but not this one.
Closed
Comments only: the JavaScript emitted with comments removed is byte-identical for every file, and the test names are unchanged. Kept: every invariant and what could be done without it, the documentation of exported names, and the reasons for what is not done the obvious way. Cut: how the code came to be as it is, narration of the lines that follow, and explanations repeated at several sites.
… path test on Windows Three Windows-only rows assert the safe outcome: a hook is not written after `.git` is renamed aside and recreated; `.git/config` stays denied after the host replaces it by rename, as git's own rewrite does; initialize() and reset() each take under 30 seconds in a working directory of 20,000 files, with the times printed. windows-mandatory-denies.test.ts joins the Windows legs' file list: it never ran there. README: what does lift a mandatory deny on Windows (allowGitConfig, and an allowWrite entry beneath a denied directory).
Mandatory and configured denies are computed at wrap time and passed through srt-win exec --deny-*, held under the exec PID and released when it ends. Session initialize only applies grants. srt-win pins each real ancestor between a deny target and its modify-grant root with an object-only DELETE|WRITE_DAC deny.
…planted link cannot lift Joins the per-command deny set with what this branch had. One function decides the mandatory set. The working directory's own names (`.git/hooks`, `.git/config`, `.claude/commands`, the dangerous dotfiles) are taken at any `mandatoryDenySearchDepth`: at the schema's minimum of 1 the scan alone dropped them. What was stamped at initialize() and is now dead is removed. srt-win resolves junctions and keeps the last mask a holder asks for a path. With `allowWrite: ["."]` and `denyRead: ["./secrets"]`, a sandboxed command could plant a junction `.vscode -> secrets`; the next command's mandatory write deny on `.vscode` then replaced the read deny on `secrets`, which became readable. A write deny whose canonical path is also read-denied is now dropped, under the lock. Tests: the two path suites are one file, in the Windows legs' list. M6 also times one wrapped command, since the stamps now cost per command. M7: the planted junction does not make the denied file readable.
…DACL write that changes nothing Stamping the whole deny set for every command cost seconds: a deny on a name puts an inheritable FILE_DELETE_CHILD deny on its parent, every DACL write on a directory re-propagates through its tree, and each row was written at the start of a command and again at its end. One `cmd /c echo` in a directory of 20,000 files took 6 to 8 s. The denies on directories (the parent's, and the pins above `.git`) are now held under the session: stamped at initialize(), released at reset(), recovered like any dead holder's. One that first becomes necessary later is added under the session by the first command that needs it (`srt-win exec --session-holder-pid`). The deny on each target itself stays per command, so a file the host has replaced is still covered. A DACL is written only when its explicit ACEs differ from those read from disk just before, never judged by the state database, so ACEs stripped from outside are put back by the next command. Each path is converged once per batch. A session-held directory that was removed and recreated is stamped afresh instead of refusing every later command. The session schema version goes to 9 for `deny_pin`. Tests: M6 times three commands in a tree with a large `.git`; M8 to M12 cover a name that appears later, `icacls /reset`, a recreated `.git`, the DACLs after reset(), and two holders.
A directory made under %TEMP% carries its parent's ACEs unmarked. The first DACL write puts it in auto-inherit form, which marks them (I) and changes no right, so the text after reset() could never equal the text before initialize(). The row now compares each ACE as account and rights, and its message is short enough for the log to keep whole.
… each end Release dropped the rows of every hold before making any write, so a kill during the first write, which takes seconds on a large tree, left every path not yet reached with its ACEs on disk and no row for recovery to go by, write grants included. One path's rows are dropped, that path is written, then the next. initialize() ran `acl grant` and then `acl stamp`, and reset() `acl revoke` and then `acl restore`, so the working directory, which carries the grant and a deny, was written twice at each end: twice the tree walk. `acl grant` now takes the denies too and `acl revoke --with-denies` releases both, and each path is written once. The order across paths is fixed. Another process of the sandbox account may be running, so the account must never hold Modify on a root while a deny recorded under it is missing from disk: a batch that adds writes the grant roots last, one that releases writes them first, and so does crash recovery, which used to go row by row and took the deny off the working directory before the grant. Adding stops at the first write that fails; releasing stops when a grant root cannot be written. srt-win says on stderr which DACLs it wrote. Tests: M6 times a session that only grants and the full one in the same run. M13 to M16 rename the directories above a nested `.git` aside, on a plain tree and with Full control for Users on the root.
… command Since the deny set became per command, the deny on each denied path was on disk only while a command of the session ran, and the write grant for the whole session. `main` holds configured denies and the credential files' read denies from initialize() to reset(). They are held for the session again, the mandatory names with them. Every command still works the whole set out afresh. Where the object at a path is the one recorded and its ACEs are as they should be, nothing is written; a new or replaced one is stamped under the session. So a command writes no DACL in the ordinary case. Only what a command adds through its own configuration ends with it. Within a session a deny only grows: one that updateConfig() or a change of working directory no longer asks for stays until reset(), and a path moved from denyRead to denyWrite stays unreadable. That fails closed, and no release can race a running command. Where a group gives the sandbox account Full control: the parent of every pinned directory gets the delete-child deny that the parent of every denied path has, so a pin cannot be renamed through the directory above; and a write deny also denies WRITE_DAC, WRITE_OWNER and FILE_DELETE_CHILD, so it cannot be taken off from inside. The session schema version goes back to 8 and is never to be bumped: a database whose version differs is renamed away, in either direction, by this binary and by every shipped one. README: what one sandbox account per machine means. Tests: M17 to M22.
…, and four smaller things An owner may rewrite an object's DACL whatever the DACL says, and what a sandboxed command makes is owned by the sandbox account. Honest commands make denied names all the time (`git clone`, a hook installer, scaffolding), so their deny could be taken off by a later command. Before a deny is written, the caller takes over the target, each pin and each parent that the account owns, and, under an allowWrite root, everything beneath a denied directory. It is permanent and needs no record. Where it fails the batch fails, naming the object. Every deny names WRITE_DAC and WRITE_OWNER, so that each denied target, each pin, each pin's parent and the granted root above them carries that in an ACE of its own and not by inheritance. The `.git` file of a linked worktree or a submodule is denied, and the `hooks` and `config` of the directories it names where they lie under an allowWrite root. A file with more than one hardlink was refused a deny, because the rows are keyed by path and the names share one descriptor. The link count is something a sandboxed command may be able to change, so refusing let it stop every later session, and skipping would have let it switch the protection off. A path is now composed from the rows of every name of its file, the widest of a kind winning, and the refusal is gone. A stamp that fails says which paths and why as data: `failed` on WindowsSandboxError, `acl_takeover_failed`, and exit 18 with one JSON line from `srt-win exec`, which a sandboxed command can forge: show it, never act on it. An ACE on a file carries no inheritance flags. Windows drops them, so what was read back never equalled what would be written, and every command rewrote the denied files. Tests: M23 to M28; M8 and M17 corrected; smoke A30 rewritten.
…temp directory M23, M24 and M26 got no owner at all: Windows PowerShell printed nothing for Get-Acl on the runners, and pwsh does. Four cases for the `.git` file compared the short form of the temp directory with the real one that the code returns.
…follows the write An object the sandbox account owned has a DACL the account could set as it liked. Taking the owner over left that in place: a file beneath a denied directory, protected and granting the account everything, stayed writable and out of the inherited deny's reach. What is taken over now also loses the account's explicit ACEs and its protected flag. Elsewhere that flag is kept as an administrator's deliberate lock-down; on such an object it is not one. The walk beneath a denied directory follows the write of its deny, which refuses every later create there, and only a write is followed by a walk. So a command that writes nothing walks nothing, however large the directory. A row marks a directory from just ahead of the write until the walk is done: a write or a walk that fails or is killed is done again. A deny input that could not be resolved takes every grant of the call with it. `acl grant` used to write the grant, skip the deny and exit 2, so the grant was on disk without its deny until the caller revoked it. A denied name under an allowWrite root that the caller may not even open fails as `acl_takeover_failed`, like one whose owner cannot be changed. Tests: M25 extended; M29 to M31.
…on it main's Windows backend has changed under this branch: write roots are granted once and kept, every command runs under the write-restricted token, and the denies beneath a kept root are kept with it. That closes three of the five ways around a deny this branch dealt with, and replaces the session holds and the refcounts it built. So nothing of the branch's own tree is carried over here; what is still needed follows in the next commits, on today's backend. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FydxqUESqxwSCVFnhk3gfv Co-authored-by: ronleizrowice <ronleizrowice@anthropic.com>
Under allowWrite ["."] a sandboxed command on Windows could write a hook for the user's next commit. The names Linux and macOS always deny are now denied there too: the dangerous files and directories, .git\hooks, .git\config unless allowGitConfig is set, and a .git file with the same names where it leads. - windowsMandatoryDenyPaths() finds the names that are there, below the working directory and under a write root. They join denyWrite at initialize(), so they rank, are stamped and are kept like any other deny. - Every command looks again, and has stamped what is new or is another object than the one stamped: git and most editors replace the file they write. Kept as well, so it is written once and costs no later command anything. - srt-win: the directories between a kept deny and its root get the delete deny a placeholder's intermediates get. A deny is on the object, so renaming .git aside would leave the name .git\hooks free to be made anew. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FydxqUESqxwSCVFnhk3gfv Co-authored-by: ronleizrowice <ronleizrowice@anthropic.com>
…taking ownership Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FydxqUESqxwSCVFnhk3gfv Co-authored-by: ronleizrowice <ronleizrowice@anthropic.com>
- The directories above a kept deny are kept in place up to the OUTERMOST kept root. They stopped at the innermost, so a session that held an outer root could rename the inner one aside and make the denied name anew. - A link or junction is passed over, at a protected name and on the way to the directories a .git file leads to. srt-win would have put the deny on what it leads to (.vscode -> . closed the root), and a link to a share would have had the host process call it. - A deny written at a command outranks a write grant the session holds beneath it, as one written at initialize() does. - Tests: icacls /reset where /grant could not fail, nested roots in the M rows, acl forget takes the pins off, both settings read at initialize() and at a command, the order in which a led path is looked at. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FydxqUESqxwSCVFnhk3gfv Co-authored-by: ronleizrowice <ronleizrowice@anthropic.com>
… found - windowsMandatoryDenyPaths() returns its paths sorted. Three cases expected .git/config before .git/hooks, which is the order one disk lists them in and another does not. - A configured deny on a share is not looked at for its file index. - Tests: the 4 KiB case led nowhere and could not fail; a working directory that is a link is resolved; the next session starts from what it stamps. - README: which kept roots count, and what a .git file of the command's own making can have denied. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FydxqUESqxwSCVFnhk3gfv Co-authored-by: ronleizrowice <ronleizrowice@anthropic.com>
ronleizrowice-ant
enabled auto-merge (squash)
October 20, 2026 14:21
Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FydxqUESqxwSCVFnhk3gfv Co-authored-by: ronleizrowice <ronleizrowice@anthropic.com>
Both backends take their lists from computeWindowsFsAccessSet(), so with main merged in the names found at initialize() went to MXC as well. MXC has one kind of deny, which takes reads away too: .git\config could no longer be read, and git does not run without it. The mxc backend stays as it is on main, and the README says so. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FydxqUESqxwSCVFnhk3gfv Co-authored-by: ronleizrowice <ronleizrowice@anthropic.com>
The reason given was too wide: it is srt that hands MXC a write deny as a full one. The schema text, the policy mapping in mxc-sandbox-utils.ts and the Windows notes in the README name the backend they hold for, and the case reads the request of a command as well as the one initialize() probes. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FydxqUESqxwSCVFnhk3gfv Co-authored-by: ronleizrowice <ronleizrowice@anthropic.com>
Collaborator
Author
|
The job's log cannot be read from where I run, so I do not know which suite ended it, and I know of no fix. I will re-run the job once when the run has finished. A second failure I will take as real and look into. Generated by Claude Code |
Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FydxqUESqxwSCVFnhk3gfv Co-authored-by: ronleizrowice <ronleizrowice@anthropic.com>
ronleizrowice-ant
disabled auto-merge
November 11, 2026 16:13
This branch has not been deployed
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.
Human: @ronleizrowice-ant
Requested by Ron · project thread
Windows sandboxes get the mandatory write denies Linux and macOS apply:
.git\hooks,.git\configunlessfilesystem.allowGitConfigis set, and the dangerous dotfiles and directories. Before, underallowWrite: ["."], a sandboxed command could write a hook for the user's next commit.Rewritten on 2026-10-20, on today's backend
+720 −18 in 9 files: 214 lines of TypeScript, 44 of Rust and 25 of its unit tests, 12 of README, 425 of tests. The branch as it stood was +3,334 −522 in 17 files, 122 commits behind
main, and marked not ready.main's Windows backend changed under it: write roots are granted once and kept, every command runs under the write-restricted token, and the denies beneath a kept root are kept with it. So the merge commit takesmain's tree as it is, and the four commits on top hold all of this pull request. The branch's history is kept.How it works
windowsMandatoryDenyPaths()finds the names that are there, down tomandatoryDenySearchDepthlevels below the working directory and under anallowWriteroot, from the lists the other platforms read. Where.gitis a file (a linked worktree, a submodule), the file is denied and so arehooksandconfigwhere it leads.initialize()they joindenyWrite, so they rank againstallowWrite, are stamped and are kept exactly as a configured deny is. One written at a command outranks a write grant the session holds beneath it in the same way.wrapWithSandboxArgv()looks again, in thecwdit is given, and has stamped what is new or is another object than the one stamped (by file index): git and most editors write a file by putting a new one in its place. That is one moresrt-win acl grant --persistentwith nothing but those denies, only when there is something to stamp. Kept too, so no later command pays for it.srt-win: the directories between a kept deny and its root get the object-only delete deny (and the delete-child deny on the parent) that a placeholder's intermediates already get. A deny is on the object, so renaming.gitaside would leave the name.git\hooksfree to be made anew. This holds for configureddenyReadanddenyWriteentries under a kept root as well. Of kept roots one inside another the outermost counts, since a session that holds it could move the inner one.The old branch's five ways around a deny
Usershas Full controlmain: the kept grant carries neither rightmain:OWNER RIGHTSis read-control only under a kept root.gitfile of a worktree or submodulemainrefuses a deny on a file with two links, so the session or the command fails and names the file. It fails closed. Denying under every name took most of the old branch's 669 lines instate_db.rsAlso not carried over, because the kept denies replace them: the session holds and refcounts (
--session-deny-*,--session-holder-pid,--dirs-only,--with-denies), taking over what the sandbox account owns (acl_takeover_failed), and failures as fields (WindowsAclFailure,--jsononacl stampandacl grant).Limits
windows.backend: 'mxc'none of them is denied: see "main merged in on 2026-11-06" below..git\config, once denied, stays denied for a session withallowGitConfiguntilsrt-win acl forget <root>. That is how every deny under a kept root behaves onmain..gitfile leads: neither followed nor denied.srt-winwould put the deny on what it leads to, and a link to a share would have the host process call it.acl_grant_failed, which names the file, until one of the two names is removed from outside.denyWrite(customConfig) gets no pins: it is held for that command only, as before.gitdir:line spelled through a link of the user's own above the write root is not followed either. Git writes the resolved path, or a relative one..gitfile of its own making a sandboxed command can havehooksandconfigdenied in any directory under the root, untilacl forget.Review
The whole diff against
mainwas reviewed at 37749b7, with 42 mutations of the TypeScript. One finding had to be fixed (the pins stopped at the innermost of nested kept roots), three should have been (links at a protected name, the hard-link wedge undocumented, tests that could not fail), and the rest were small. All are in 8aa1749, each with a case that fails without it.The whole diff was reviewed again at 8aa1749, where both Windows legs were green. Nothing had to be fixed. One case could no longer fail, three changes had no case, and a configured deny on a share was looked at for its file index: all in f151d32, with the sort that the red Linux arm64 leg asked for (three cases leaned on the order a directory is listed in). Read in Rust and found sound: a pin that is itself a kept root,
acl forgetof nested roots in either order, crash recovery, a stale wide root (one walk per level, once; the ACEs name the sandbox group only).Testing
.gitfiles, links and junctions, and five pointers that must not be followed or read.srt-winis handed, under the platform spy: atinitialize(), per command (new, replaced, nothing), after a failure, withallowGitConfigandmandatoryDenySearchDepthat both, andfilesystem.disabled.icacls /resetortakeowneither, and the rest of the root is written; M2 what one command makes is denied to the next, so is a file the host replaced, and a junction named.vscodecloses nothing; M3 the host goes on writing all of it, andacl forgettakes every deny off.dirs_betweenandpinsin Rust.Only CI's Windows legs run the Windows code.
main merged in on 2026-11-06, and the MXC backend
mainat27fce0dis merged in at18e1aeb, without a conflict in the text. There was one in the meaning, which6cb9e4eandfb6e0ecsettle.mainnow has a second Windows backend,windows.backend: 'mxc'. Both backends take their lists fromcomputeWindowsFsAccessSet(), where this pull request adds the mandatory denies. A write deny reaches MXC among itsdeniedPaths, which take reads away as well. So under mxc.git\config,.git\hooks,.gitmodulesand the rest could no longer have been read, and git does not run without reading.git\config.The mxc backend therefore gets none of them and stays as it is on
main. The README says so among its known limitations and at the list of names, and so do the schema text ofwindows.backendand the policy mapping at the head ofmxc-sandbox-utils.ts. One new case under the platform spy reads the requestinitialize()probes and the request of a command. It fails with.git/configand.git/hooksamong the denied paths when the guard is taken out, and whenspawnSandboxed()is made to look for the names itself.The rest of
main's change was read against this one: under mxcwrapWithSandboxArgv()throws before the per-command look,reset()has nothing of this to clear, no Rust changed, and the new boundary suite (mxc.test.ts, on srt-win in CI) touches none of the names and moves no directory above a deny.Left open, for whoever has a BaseContainer host.
buildMxcRequest()notes that a read-only entry beneath a write grant makes that subtree read-only. Sent asreadonlyPaths, the names might therefore be closed to writes and still be read. Whether that also stops a rename over the file, or moving.gitaside, has to be measured on a real host, and CI runs none. So it is not tried here.The 25 lines of
6cb9e4ewere reviewed againstmain's change, with the whole of this pull request beside them. Nothing had to be fixed. What it found in these lines is infb6e0ec: the reason given was too wide, three texts did not name the backend, and the case read only the request ofinitialize().main merged in again on 2026-11-09
mainat951e641is merged in atbe119d2, without a conflict. It brought a narrower profile read grant and aninitialize()that areset()cancels. Read against this pull request:persist.rschanges in other functions than the pins, no call ofacl grant --persistentchanges, and this pull request adds noawaittoinitialize(). A session cancelled after its grant leaves the record of what was denied as it was, and the nextinitialize()replaces it. What this pull request changes is the same as before, +720 −18 in 9 files.Claude session #01FydxqU
Generated by Claude Code