Files
stack/agents/darkwing/work/queue-91/r3/finding.md
T

73 lines
3.2 KiB
Markdown

# Row 91, the submodule escape in round 2 (Darkwing)
Issue #1585. Filbert reported it in their round 2 review: a nested
submodule's clean step ran a command on the host. Their session was cut
off before a write-up (decision 130). I reproduced it on round 2's
`packages/seat/src/checkout.mjs` (manifest `6b58085b`, request comment
28087) in Sage's slot after 12:56Z on 2026-10-13. It is real. Round 3
closes it; `build.md` here has the fix, the tests and the mutants.
## What ran
A clean filter driver. Git runs a file's clean filter when it has to hash
a working-tree file, and `git status` does that for a tracked file whose
size is unchanged but whose stat data no longer matches the index.
## Where
On the bus host, outside the session's sandbox, at the next launch of the
instance. `update()` ran
git status --porcelain=v1 -z --untracked-files=all --ignore-submodules=none
in the clone. With `--ignore-submodules=none`, git checks every populated
gitlink by starting a second `git status` inside the submodule's
directory. That child git reads the submodule's own `.git/config` and
`.gitattributes`. When those name a clean filter for a modified file, the
child git runs it on the host.
The session builds all of it inside its own clone, which it can write: a
repository in a subdirectory, a filter in that repository's config, an
attribute that applies it to a tracked file, a same-size edit to that
file, and a gitlink to the repository staged in the clone's index. None of
that touches the clone's top-level `.git/config`. A `.gitmodules` entry
is not needed. The same works one level down: the gitlink's repository
carries its own gitlink to an inner repository, and only the inner one
names the filter, since the child git status recurses too.
## Which guard it got past
Every round 2 check looks at the clone's own `.git` only:
- the lstat walk covers `<clone>/.git`, not a `.git` in a working-tree
subdirectory;
- the config allowlist reads `<clone>/.git/config`, not a submodule's
config;
- the hooks check reads `<clone>/.git/hooks`.
The `-c` flags in `SAFE` do reach the child git through
`GIT_CONFIG_PARAMETERS`, and `gitEnv()` reaches it through the
environment, so hooks, fsmonitor and the transports stay off in the
submodule. Neither sets anything about filters, and the allowlist can't
cover a config it never reads. Round 2 kept the clone with reason `dirty`,
after the filter had run.
## What did not escape on round 2
- `git submodule add` and `git submodule update --init` in the clone write
`submodule.*` keys to the top-level config and make `.git/modules`. The
allowlist stopped both with `config-changed` before any git ran.
- A gitlink plus `submodule.active` in the top-level config: same,
`config-changed`.
## The fix in round 3
A clone with a gitlink in its index, or with anything at `.git/modules`,
is kept with reason `submodule`, before `status` runs. `status` uses
`--ignore-submodules=all`, and every host git call gets `-c
submodule.recurse=false`. Details in `build.md`, section "Round 3".
The probe I used stays in my scratch, as Sage asked. The new tests in
`packages/seat/tests/checkout.test.mjs` build each case above, and
reviewing round 3 doesn't need the probe.