diff --git a/docs/PRD.md b/docs/PRD.md index 806ea7d3..b8791b85 100644 --- a/docs/PRD.md +++ b/docs/PRD.md @@ -1529,6 +1529,59 @@ All work is **alpha** (< 0.1.0) until Jason approves 0.1.0 beta release. --- +## Workspace placement guard hardening (#1174) + +### Problem and objective + +The Bash pre-tool guard must prevent Git checkouts and repository state from being placed under +`$HOME` without refusing ordinary Git commands merely because a source, option value, branch name, +or metadata mentions `$HOME`. A guard that over-blocks routine work is unsafe because operators +will route around it. + +### Scope and requirements + +1. `WPG-REQ-01`: `git clone` and `git worktree add` placement SHALL be judged from their placement + operands, not from every HOME-shaped word in the command. +2. `WPG-REQ-02`: Clone sources, references, templates, environment assignments, and non-placement + worktree metadata MAY resolve under HOME when all placement operands resolve elsewhere. +3. `WPG-REQ-03`: Both attached and separate-value `--separate-git-dir` forms SHALL remain placement + operands and SHALL be refused when they resolve under HOME. +4. `WPG-REQ-04`: Option classification SHALL account for Git's rule-generated boolean negations + without relying on an enumerable allowlist of flag spellings. +5. `WPG-REQ-05`: Quote removal, escapes, shell command boundaries, redirections, and end-of-options + handling SHALL preserve existing fail-closed checkout coverage. +6. `WPG-REQ-06`: Absolute placement aliases SHALL resolve shell-known HOME spellings, dot segments, + repeated separators, and existing symlink parents before the HOME boundary comparison. +7. Relative targets whose effective path depends on the shell cwd are out of scope and tracked by + #1197. + +### Acceptance and verification + +1. Git's own option parser accepts each tested flag, including generated `--no-*` forms, while the + guard allows a HOME-valued source with an explicit safe destination. +2. Equivalent clone and worktree fixtures cover rule-generated negations and remain discriminating + against the prior head where the defect existed. +3. Real HOME destinations and both `--separate-git-dir` forms remain blocked, including placements + after shell command boundaries. +4. The full hermetic guard suite, syntax/static checks, adversarial probes, independent review, and + terminal-green CI pass before merge. +5. Any option-classification residual is documented with its deliberate failure direction. + +### Constraints, risks, and assumptions + +- Security and usability are co-equal: neither a placement bypass nor routine over-block is an + acceptable repair. +- `ASSUMPTION:` The value-taking option surface exposed by the installed Git version is closed and + measurable through Git's own parser/help output; rationale: boolean flags are rule-generated, + while separate-value options have explicit grammar and must be classified as such. +- Risk: a future Git release may add a new value-taking placement option. Mitigation: document the + chosen residual direction and pin every currently supported placement option in behavior tests. +- Risk: a symlink can be replaced after pre-execution canonicalization. Mitigation: resolve every + existing parent physically and document the remaining inherent TOCTOU window; the worktree helper + remains the authoritative path-derivation mechanism, with atomic closure tracked by #1199. + +--- + ## Assumptions 1. RESOLVED: **pgvector is sufficient** for semantic search at v0.1.0 scale (personal/family/team = thousands to low hundreds-of-thousands of vectors). `@mosaicstack/memory` defines a `VectorStore` interface with pgvector as the default adapter. The interface boundary makes Qdrant a drop-in migration if PG resource contention or scale demands it later. Zero additional infrastructure for v0.1.0. Rationale: Reduces ops burden; pgvector HNSW indexes are fast at this scale; interface abstraction costs almost nothing now. diff --git a/docs/scratchpads/1174-wrapper-guard-round10.md b/docs/scratchpads/1174-wrapper-guard-round10.md index e6886619..3629652f 100644 --- a/docs/scratchpads/1174-wrapper-guard-round10.md +++ b/docs/scratchpads/1174-wrapper-guard-round10.md @@ -1,4 +1,4 @@ -# #1174 — Wrapper guard round 10 +# #1174 — Wrapper guard rounds 10–11 ## Objective @@ -28,3 +28,62 @@ Make checkout enforcement judge Git placement operands rather than every HOME-sh - Relative destinations whose effective path depends on cwd are tracked separately by #1197 and remain out of scope. - Unknown future Git options with a separate following value fail closed when that value is HOME-shaped. This may require classification when Git adds an unrelated path-taking option, but prevents a new placement option from silently bypassing the guard. + +## Round 11 objective and intake + +- **Issue / PR:** #1174. +- **Objective:** Remove the finite boolean-flag allowlists that turn accepted clone/worktree flags into fake placement operands, while preserving all real HOME placement blocks. +- **Scope:** `wrapper-guard.sh`, its hermetic fixtures, and task documentation. Relative cwd-dependent destinations remain in #1197. +- **Surfaces:** security-sensitive Bash hook behavior and shell/Git option grammar; no API, DB, UI, auth, deploy, or dependency changes. +- **Budget assumption:** 25K working tokens; reduce exploratory matrices before reducing acceptance coverage. + +### Round 11 plan + +1. Use Git itself to classify accepted/rejected clone and worktree options, and Bash itself to resolve path-word expectations. +2. Add RED fixtures for all six reported clone flags, generated negations, and equivalent worktree grammar. +3. Replace the open-ended unknown-option fail-closed fallback with a parser based on the closed value-taking option surface; keep explicit placement options special. +4. Run the full corpus, historical discrimination, shell/static checks, targeted probes, independent code/security review, one push, and exact-head CI. + +### Root-cause evidence + +- Git 2.39.5 accepts all six reported clone flags and the broader generated family measured in the brief: `--bare`, `--mirror`, `--ipv4`, `--ipv6`, `-4`, `-6`, `--no-local`, `--no-reject-shallow`, `--no-bare`, `--no-sparse`, `--no-dissociate`, `--no-shallow-submodules`, `--no-quiet`, `--no-progress`, and `--no-recurse-submodules`; it rejects `--relative-paths` as unknown. +- Git 2.39.5 accepts worktree negations including `--no-force`, `--no-detach`, `--no-lock`, `--no-guess-remote`, and `--no-track`; the current finite worktree flag list does not describe that generated family. +- `bash -c "printf '%s' "` resolves `$HOME/source`, `${HOME}/source`, and `"$HOME"/source` under HOME while `/src/wt` remains outside it. +- **Hypothesis:** only separate-value options need positive classification. Treat every other option token as a no-value flag unless it is the explicit placement option; this matches Git's non-enumerable boolean family and confines the residual to genuinely new future value-taking options. + +### TDD and verification checkpoints + +- RED against the unmodified `91cc37bc` guard: 253 pass / 22 fail in the initial expanded 275-fixture suite. Failures include all 15 accepted clone flags, accepted long abbreviations, short value-taking bundles, abbreviated placement, worktree metadata abbreviation, and both directions of bundled worktree branch parsing. +- An exploratory fail-closed residual test drove emission of every worktree positional. Re-review correctly showed that this over-blocked HOME-shaped commit-ish metadata; a new commit-ish fixture failed RED against that intermediate implementation (278 pass / 2 fail, including one transient message assertion) and the parser was restored to emit only the actual path. +- GREEN after remediation: 280/280. +- Ultron's 13-shape option probe: 13/13 correct, including the six reported over-blocks, HOME destinations, end-of-options, worktree controls, and a later-command placement. +- Round-10 probes remain green: 7/7 subject-placement expectations and 4/4 `--separate-git-dir` controls. +- Earlier shell/path probes remain green: 60/60, 24/24, and 17/17. +- `bash -n`, ShellCheck warning-or-higher, and `git diff --check`: pass. + +### Deliberate residual + +A future Git release could add a new separate-value option absent from the closed value grammar. It defaults to no-value flag parsing, which leaves the following word positional. For clone, this can fail open if that future option itself creates repository state at its value. For worktree, it can shift which word is read as the path. This hypothetical future ambiguity is accepted deliberately because failing closed on every unclassified option is proven to over-block Git's open-ended present-day boolean/`--no-*` family. Every value-taking and placement option Git currently supports is classified, including accepted abbreviations of `--separate-git-dir`. Relative cwd-dependent targets remain in #1197. + +### Independent review checkpoint + +- Initial Codex code/security review raised `--orphan` as value-taking. Upstream Git `master` contradicts that premise: the synopsis is `[--orphan] [(-b | -B) ] []`, and the prose derives the branch from the path when `-b`/`-B` is absent. `--orphan` is therefore correctly handled as a boolean flag. +- The security review separately identified the generic future worktree shift residual. An attempted fail-closed remediation emitted every positional, but code re-review correctly rejected it because valid grammar has only one placement positional and an optional commit-ish. Final behavior checks only the path and documents the hypothetical future option shift deliberately; paired actual-grammar `--orphan` fixtures cover safe/HOME paths and `-b` metadata. +- Security re-review initially had no findings. Code re-review's commit-ish blocker was remediated with a RED fixture and path-only restoration; final code re-review approved with no findings. +- Final security review then found non-canonical absolute and symlink aliases. Eight lexical fixtures failed RED against the prior implementation, followed by three symlink fixtures failing RED. Remediation expands only shell-visible HOME tokens, resolves the longest existing directory prefix physically, and lexically normalizes the nonexistent suffix. The suite is now 292/292. +- Inherent residual: a symlink can be replaced between pre-tool inspection and Git execution. Existing aliases are resolved; eliminating the race requires enforcement inside the filesystem mutation path rather than a text pre-hook. Security review classified this medium, and architectural closure is tracked in #1199. +- Final independent code review: APPROVE, 0 findings. Final security review: no critical/high findings; the single medium TOCTOU residual is explicitly tracked in #1199. + +### Final local evidence + +- Final hermetic suite: 292/292; the same suite against `91cc37bc` discriminates at 256 pass / 36 fail. +- Ultron option probe: 13/13; round-10 probes: 7/7 plus 4/4 controls; earlier shell/path probes: 60/60, 24/24, and 17/17. +- `bash -n`, ShellCheck warning-or-higher, `git diff --check`, sanitization gate, and test-enumeration gate (population 55; 38 enumerated; 18 signed exclusions): pass. +- Independent code review: APPROVE, 0 findings. Security review's remaining medium TOCTOU architecture residual is tracked in #1199; no critical/high findings remain. +- Repository-wide TypeScript gates require dependencies absent from this worktree; the canonical Woodpecker pipeline will run them against the pushed exact head. + +### Documentation checklist + +- `docs/PRD.md` updated with WPG requirements, acceptance, canonicalization, and residual risk. +- Task scratchpad updated in the same logical change set; `docs/TASKS.md` remains orchestrator-only. +- No API, auth, UI, navigation, deployment, user-guide, or admin-guide surface changed; OpenAPI, endpoint index, sitemap, and publishing are not applicable. diff --git a/packages/mosaic/framework/tools/git/test-wrapper-guard.sh b/packages/mosaic/framework/tools/git/test-wrapper-guard.sh index 45a4d102..a72fe1e1 100755 --- a/packages/mosaic/framework/tools/git/test-wrapper-guard.sh +++ b/packages/mosaic/framework/tools/git/test-wrapper-guard.sh @@ -40,6 +40,12 @@ FIXTURES="$TMP/fixtures.tsv" printf '2\t{"tool_input":{"command":"git clone x \\"$HOME\\"/wt"}}\ta closing quote between HOME and slash does not hide the path\n' printf '2\t{"tool_input":{"command":"git clone x ${HOME}/wt"}}\tthe braced HOME spelling is the same home path\n' printf '2\t{"tool_input":{"command":"git clone x \\"${HOME}\\"/wt"}}\tbraced HOME may also end a quoted span before the slash\n' + # Lexically equivalent absolute paths must be compared after shell-known HOME + # expansion and dot-segment normalization, without resolving filesystem links. + printf '2\t{"tool_input":{"command":"git clone x /var/../$HOME/wt"}}\tHOME expansion after parent traversal is normalized before comparison\n' + printf '2\t{"tool_input":{"command":"git worktree add /var/../${HOME}/wt"}}\tworktree placement also normalizes embedded HOME expansion\n' + printf '2\t{"tool_input":{"command":"git clone --separate-git-dir=/var/../$HOME/gd x /src/wt"}}\tseparate Git state cannot hide behind parent traversal\n' + printf '0\t{"tool_input":{"command":"git clone x $HOME/../outside-home/wt"}}\ta parent segment that leaves HOME is not over-blocked\n' # The target may be HOME itself. End-of-command and whitespace terminate the # token just as a slash does; punctuation that can extend a path does not. printf '2\t{"tool_input":{"command":"git clone x $HOME"}}\tthe unbraced variable may name HOME exactly\n' @@ -100,6 +106,57 @@ FIXTURES="$TMP/fixtures.tsv" printf '0\t{"tool_input":{"command":"git clone --template $HOME/t https://example.invalid/x /src/wt"}}\ta space-separated HOME template remains a source\n' printf '2\t{"tool_input":{"command":"git clone 2>/dev/null https://example.invalid/x $HOME/wt"}}\ta redirection before clone arguments does not become the destination\n' printf '2\t{"tool_input":{"command":"git clone --reference $HOME https://example.invalid/x $HOME/wt"}}\ta source option does not hide a later HOME destination\n' + # Round eleven: Git accepts boolean options as a rule-generated family, + # including --no-* negations. Each command below was checked with Git itself: + # `git clone