mirror of
https://github.com/instructkr/claude-code.git
synced 2026-05-22 13:46:44 +00:00
docs(roadmap): add delayed binary read gap
This commit is contained in:
@@ -6691,3 +6691,5 @@ Original filing (2026-04-18): the session emitted `SessionStart hook (completed)
|
||||
568. **Read-only bash validation classifies `git fetch` as safe even though fetch mutates `.git/FETCH_HEAD`, remote-tracking refs, packfiles, and optional tags** — dogfooded 2026-05-22 from the `#clawcode-building-in-public` 04:30 UTC nudge on `/home/bellman/Workspace/claw-code-pr2967` with branch/origin `docs/roadmap-workdir-provenance@1882c95`. Active tmux sessions at probe time: `gajae-pr-323-bootstrap-context-readiness-review`, `omx-issue-2451-madmax-detached-lock-timeout`; no active claw-code implementation session. Code inspection: `rust/crates/runtime/src/bash_validation.rs::validate_read_only` delegates `git` commands to `validate_git_read_only`, and `GIT_READ_ONLY_SUBCOMMANDS` includes `fetch` alongside pure inspection commands like `status`, `log`, and `show`. But `git fetch` is not read-only: even without checkout it writes `.git/FETCH_HEAD`, may update remote-tracking refs, downloads pack/object data, updates tags depending on flags/config, and can run hooks/config-driven network behavior. There is no mode distinction for `git fetch --dry-run` versus mutating fetch forms, and no test asserts that read-only mode blocks repository/network mutation. This creates a stale-branch/confusion footgun in the opposite direction: agents can claim read-only validation while changing the local base/ref state used by later freshness checks. **Required fix shape:** (a) remove plain `fetch` from `GIT_READ_ONLY_SUBCOMMANDS` or split it into a separate `NetworkMetadataMutation`/workspace-write classification; (b) allow only explicitly non-mutating forms if Git actually guarantees them for the chosen flags, otherwise require WorkspaceWrite/Prompt; (c) add read-only regressions proving `git fetch`, `git fetch origin main`, and `git fetch --tags` are blocked/warned, while `git status`, `git log`, and `git diff` remain allowed; (d) include a clear reason mentioning `.git` metadata/ref mutation and network access; (e) align stale-branch preflight code so any fetch it performs is an explicit trusted preflight action, not hidden inside user-command read-only permission. **Why this matters:** read-only mode is used as safety evidence. Treating `git fetch` as read-only lets commands mutate repository metadata and remote refs under a permission label that promises observation only, making later branch-freshness and audit results harder to trust. Source: gaebal-gajae dogfood response to Clawhip message `1507239113550725231` on 2026-05-22.
|
||||
|
||||
569. **Read-only bash validation does not unwrap `/usr/bin/env` command prefixes, so `env FOO=bar rm file` is allowed even though the executed command is write/destructive** — dogfooded 2026-05-22 from the `#clawcode-building-in-public` 05:00 UTC nudge on `/home/bellman/Workspace/claw-code-pr2967` with branch/origin `docs/roadmap-workdir-provenance@2bb5d49`. Active tmux sessions at probe time: `gajae-pr-323-fix-windows-host-path-safety2`, `omx-pr-2452-madmax-lock-timeout-review`; no active claw-code implementation session. Channel context also had new PR claw-code#3056, but this pinpoint is from local permission-validation inspection. Code inspection: `rust/crates/runtime/src/bash_validation.rs::extract_first_command` strips leading `KEY=value` assignments and `validate_read_only` has a special recursive path for `sudo`, but there is no equivalent handling for the common `env KEY=value <cmd>` launcher form. `env` itself appears in `SEMANTIC_READ_ONLY_COMMANDS`, and in read-only mode `validate_read_only("env FOO=bar rm -rf target", PermissionMode::ReadOnly)` sees first command `env`, does not recurse into the inner `rm`, finds no redirection/git state path, and returns `Allow`. The same bypass applies to `env PATH=/tmp cp a b`, `env -i sh -c 'rm file'`, and likely `/usr/bin/env` shebang-style invocations if the first token is a path. Existing tests cover bare env assignments like `FOO=bar ls -la`, and sudo-wrapped writes, but not `env`-wrapped writes. **Required fix shape:** (a) teach first-command extraction or read-only validation to recognize `env`/`/usr/bin/env` prefixes, skip env flags/assignments, and validate the inner command recursively; (b) block or warn if the inner command cannot be parsed safely (for example `env -i sh -c ...`) under read-only mode; (c) add regressions for `env FOO=bar rm -rf target`, `env PATH=/tmp cp a b`, `env -i npm install`, and safe `env FOO=bar grep x file`; (d) keep the existing `KEY=value cmd` behavior but cover quoted assignment values; (e) report the unwrapped inner command in the block reason so operators can see which payload violated read-only mode. **Why this matters:** permission validation must classify the command that will actually execute, not the wrapper binary. `env` is a routine launcher in scripts; allowing it as read-only lets agents hide write/package/destructive commands behind environment setup and produces false safety evidence. Source: gaebal-gajae dogfood response to Clawhip message `1507246662630903910` on 2026-05-22.
|
||||
|
||||
570. **`read_file` binary detection scans only the first 8192 bytes for NUL, so text-prefixed binaries can pass the binary gate and fail later as generic UTF-8 read errors** — dogfooded 2026-05-22 from the `#clawcode-building-in-public` 05:30 UTC nudge on `/home/bellman/Workspace/claw-code-pr2967` with branch/origin `docs/roadmap-workdir-provenance@a49d8e8`. Active tmux session at probe time: `gajae-issue-324-review-gate-degradation`; no active claw-code implementation session. Channel context from Jobdori pointed at `is_binary_file` line 30. Code inspection confirmed `rust/crates/runtime/src/file_ops.rs::is_binary_file` opens the path, reads exactly one 8192-byte chunk, and returns true only if that first chunk contains NUL. `read_file` then calls `fs::read_to_string` over the whole file. A file with an 8KB text header followed by NUL/non-UTF8 binary payload is therefore not rejected by the explicit binary gate; it reaches `read_to_string` and surfaces as a generic invalid UTF-8/io error instead of the stable `InvalidData: file appears to be binary` contract. Existing coverage writes NUL at byte 0 (`rejects_binary_files`) and does not cover delayed-NUL or delayed-non-UTF8 payloads. **Required fix shape:** (a) make binary/text validation cover the entire allowed read range (bounded by `MAX_READ_SIZE`) or stream decode as UTF-8 while detecting NUL/non-text bytes; (b) map any delayed NUL/non-UTF8 failure to the same stable binary-file error kind/message used by the early gate; (c) add regressions with NUL at byte 8192+, non-UTF8 after an ASCII prefix, and valid large UTF-8 text controls; (d) avoid loading oversized files by preserving the metadata size gate before full scan/decode; (e) include byte offset/evidence in diagnostics only if it does not leak file contents. **Why this matters:** `read_file` should fail predictably on binary artifacts. A small text header is common in mixed/generated files, and letting those bypass the binary gate creates inconsistent errors that look like brittle UTF-8/tool failures rather than an intentional binary-read refusal. Source: gaebal-gajae dogfood response to Clawhip message `1507254212403138672` on 2026-05-22.
|
||||
|
||||
Reference in New Issue
Block a user