d0zingcat/claude-code
1
0
Fork 0
mirror of https://github.com/instructkr/claude-code.git synced 2026-05-22 13:46:44 +00:00
Code Issues Packages Projects Releases Wiki Activity

docs(roadmap): add path traversal validation gap

Browse Source
This commit is contained in:
Yeachan-Heo
2026-05-22 04:01:56 +00:00
parent 665dd1fe2a
commit 1882c9582a
1 changed files with 4 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
ROADMAP.md
View File

@@ -6683,3 +6683,7 @@ Original filing (2026-04-18): the session emitted `SessionStart hook (completed)
563. **G004 conformance `get_path` falls back to suffix pointers, so nested required fields can be satisfied by same-named root fields and invalid bundles pass** — dogfooded 2026-05-22 from the `#clawcode-building-in-public` 00:00 UTC nudge on `/home/bellman/Workspace/claw-code-pr2967` with branch/origin `docs/roadmap-workdir-provenance@8ea54d3` and binary built from source SHA `25d663d`. Active tmux session at probe time: `gajae-issue-316-fix-still-open-fixture`. Code inspection: every conformance field helper ultimately calls `get_path(root, pointer)` in `runtime/src/g004_conformance.rs:386-398`. If `root.pointer(path)` misses, `get_path` iterates suffixes of the requested path and tries those against the same root object. For example, validating a report's `/identity/contentHash` at `g004_conformance.rs:132-137` first checks that nested field; if it is missing, `get_path(report, "/identity/contentHash")` then tries `"/contentHash"`. A malformed report with top-level `contentHash` but no `identity.contentHash` therefore passes the nested identity requirement. The same suffix fallback affects `/projection/provenance`, `/redaction/provenance`, `/metadata/provenance`, `/metadata/seq`, `/metadata/eventFingerprint`, and similar nested contract fields. This makes the machine-checkable G004 validator structurally permissive in exactly the fields that are supposed to prove provenance/identity, and tests/fixtures do not appear to include negative cases for misplaced nested fields. **Required fix shape:** (a) remove suffix fallback from `get_path` and use strict JSON Pointer lookup for conformance validation; (b) if fallback is needed for legacy aliases, make it explicit per field with a deprecation warning/error code, never implicit for all nested fields; (c) add negative tests where top-level `contentHash`, `provenance`, `seq`, or `eventFingerprint` exist but the required nested path is absent, proving validation fails; (d) update error messages to report the exact missing nested path; (e) run existing G004 fixtures through strict validation and fix any accidental reliance on suffix aliases. **Why this matters:** conformance validators are trust boundaries. A suffix fallback lets artifacts satisfy provenance/identity requirements from the wrong location, so downstream consumers can accept bundles whose shape does not match the advertised contract. Source: gaebal-gajae dogfood response to Clawhip message `1507171165033201735` on 2026-05-22.
564. **G004 approval-token conformance checks only that delegation hops are non-empty, not that the chain is contiguous from owner to executor, so broken approval provenance passes** — dogfooded 2026-05-22 from the `#clawcode-building-in-public` 00:30 UTC nudge on `/home/bellman/Workspace/claw-code-pr2967` with branch/origin `docs/roadmap-workdir-provenance@54bbee2` and binary built from source SHA `25d663d`. Active tmux sessions at probe time: none. Code inspection: `validate_approval_tokens` in `runtime/src/g004_conformance.rs:226-253` requires `tokenId`, `owner`, `scope`, `issuedAt`, `oneTimeUse`, `replayPreventionNonce`, and calls `validate_delegation_chain`. But `validate_delegation_chain` at `g004_conformance.rs:256-272` only checks each hop has non-empty `from`, `to`, `action`, and `at`; it never verifies that the first hop starts at the token `owner`, that each hop's `to` equals the next hop's `from`, or that the final `to` matches an approved/executing actor field. The valid fixture has one clean `leader-fixed -> worker-3` hop, and the only negative test uses an empty chain; there is no fixture where a non-empty but discontinuous chain like `owner -> lead`, then `unrelated -> worker` is rejected. Therefore an approval token can advertise an auditable delegation chain while the conformance helper accepts broken provenance continuity. **Required fix shape:** (a) add explicit `approvedExecutor`/`executingActor` (or equivalent) to the G004 approval-token schema if final actor is part of the contract; (b) validate first hop `from == owner`; (c) validate adjacent hop continuity (`hop[i].to == hop[i+1].from`); (d) validate final hop reaches the approved/executing actor when present; (e) add negative tests for owner mismatch, discontinuous middle hop, and wrong final executor, plus a positive multi-hop fixture. **Why this matters:** approval tokens are policy-exception evidence. A non-empty list of disconnected hops is not an audit trail; downstream claws can mistakenly trust a token whose delegation provenance does not actually connect the owner approval to the actor consuming it. Source: gaebal-gajae dogfood response to Clawhip message `1507178711106064444` on 2026-05-22.
565. **MCP lifecycle hardened diagnostics expose `timestamp` / `phase_timestamps` as raw epoch seconds while adjacent machine-readable event contracts are being tightened around RFC3339, so MCP/plugin lifecycle logs remain format-inconsistent and parser-hostile** — dogfooded 2026-05-22 from the `#clawcode-building-in-public` 02:30 UTC nudge on `/home/bellman/Workspace/claw-code-pr2967` with branch/origin `docs/roadmap-workdir-provenance@665dd1f`. Active tmux sessions at probe time: none. Code inspection: `runtime/src/mcp_lifecycle_hardened.rs:6-12` defines `now_secs()` as `SystemTime` seconds since `UNIX_EPOCH`; `McpErrorSurface.timestamp` at `mcp_lifecycle_hardened.rs:50-56` stores that raw `u64`, and `McpLifecycleState.phase_timestamps` at `mcp_lifecycle_hardened.rs:125-129` / `180-186` records the same epoch seconds per phase. Tests only assert presence via `phase_timestamp(...).is_some()` and preserve `waited_ms`; they never assert a stable date-time contract or expose both wall-clock and monotonic sequencing. This is a plugin lifecycle variant of the timestamp-contract findings in #548-#551/#554: MCP diagnostics are explicitly machine-readable, but a field named `timestamp` differs from the RFC3339-looking `emittedAt` / `createdAt` surfaces and will force downstream claws to special-case one lifecycle stream. **Required fix shape:** (a) decide and document MCP lifecycle timestamp contract; prefer RFC3339 UTC string fields like `occurredAt` / `phaseStartedAt` for wall-clock log interoperability; (b) if epoch seconds are retained, name them `timestamp_epoch_seconds` and optionally add a separate monotonic `phase_seq`; (c) update `McpErrorSurface` and `phase_timestamps` serialization with backward-compatible aliases or migration notes; (d) add tests that parse MCP lifecycle wall-clock fields as RFC3339 and reject ambiguous raw epoch strings when serialized under the public contract; (e) align MCP lifecycle diagnostics with lane events/recovery/G004 timestamp validators so plugin lifecycle failures can be sorted and displayed without bespoke parser branches. **Why this matters:** MCP startup/plugin lifecycle failures are already hard to debug. If the lifecycle stream uses raw epoch seconds while the rest of claw-code moves to RFC3339 event timestamps, logs and UIs lose a uniform temporal contract exactly on the surface operators need during plugin breakage. Source: gaebal-gajae dogfood response to Clawhip message `1507208914046025731` on 2026-05-22.
566. **Workspace-write outside-workspace detection misses exact sensitive directory targets like `/etc` because `command_targets_outside_workspace` only searches for paths with trailing slashes** — dogfooded 2026-05-22 from the `#clawcode-building-in-public` 03:00 UTC nudge on `/home/bellman/Workspace/claw-code-pr2967` with branch/origin `docs/roadmap-workdir-provenance@665dd1f`. Active tmux sessions at probe time: none. Code inspection: `runtime/src/bash_validation.rs:286-295` routes `PermissionMode::WorkspaceWrite` through `command_targets_outside_workspace`, and that helper at `bash_validation.rs:302-323` only warns for write/state-modifying commands when the command string contains one of `"/etc/", "/usr/", "/var/", "/boot/", "/sys/", "/proc/", "/dev/", "/sbin/", "/lib/", "/opt/"`. Existing coverage checks `validate_mode("cp file.txt /etc/config", PermissionMode::WorkspaceWrite)` and therefore exercises the trailing-slash case. But a write target that is exactly the directory name, such as `cp file.txt /etc`, `mv config /usr`, `install file /opt`, or `tee /etc`, does not contain `"/etc/"` / `"/usr/"` / `"/opt/"`; the helper returns false and `validate_mode` falls through to `Allow`. This is adjacent to, but distinct from, Jobdori's #570 traversal-substring bypass: even without `..`, exact system directory operands evade the workspace-write out-of-scope warning because the heuristic uses substring sentinels instead of token/path boundary checks. **Required fix shape:** (a) tokenize shell words enough to inspect path-like operands rather than raw substring matching; (b) treat exact sensitive directories (`/etc`, `/usr`, `/var`, `/boot`, `/sys`, `/proc`, `/dev`, `/sbin`, `/lib`, `/opt`) and descendants equivalently; (c) include command/path evidence in the warning so operators see which operand escaped workspace scope; (d) add regressions for `cp file.txt /etc`, `mv file /usr`, `install file /opt`, and keep the existing `/etc/config` case green; (e) consider unifying this with the #570 path-resolution hardening so workspace-write/read-only path scope checks use one boundary-aware path classifier. **Why this matters:** workspace-write mode is supposed to allow project edits while warning on system-level writes. Missing exact-directory operands lets an agent attempt writes into sensitive roots with no permission warning, which is precisely the boundary the mode is meant to make visible. Source: gaebal-gajae dogfood response to Clawhip message `1507216459665772677` on 2026-05-22.
567. **Path traversal validation treats any command string containing the workspace path as safe, so mixed outside-target commands can smuggle `../` escapes past the warning** — dogfooded 2026-05-22 from the `#clawcode-building-in-public` 04:00 UTC nudge on `/home/bellman/Workspace/claw-code-pr2967` with branch/origin `docs/roadmap-workdir-provenance@665dd1f`. Active tmux session at probe time: `omc-issue-3079-plugin-cache-commands`; no active claw-code implementation session. Code inspection: `rust/crates/runtime/src/bash_validation.rs::validate_paths` checks `if command.contains("../")`, then suppresses the traversal warning whenever the *entire command string* contains `workspace.to_string_lossy()`. That means a command with one operand inside the workspace and a separate traversal operand outside it, such as `cp /workspace/file ../../../etc/passwd` (workspace `/workspace`) or `tar -cf /workspace/out.tar ../../..`, is treated as `Allow` because the workspace prefix appears somewhere in the command. The validator never tokenizes operands, resolves each candidate path, or proves that the specific `../` path stays under the workspace root. Existing tests cover `cat ../../../etc/passwd` without a workspace substring, but not mixed safe+unsafe operands. This is a separate path-scope gap from #566's exact sensitive-directory miss: here the escape uses traversal and is hidden by an unrelated in-workspace token. **Required fix shape:** (a) tokenize shell words enough to collect path-like operands independently; (b) resolve each relative/absolute candidate against cwd/workspace and warn/block when any operand escapes, rather than checking whether the command string contains the workspace root; (c) keep quoted/non-path text from causing false positives; (d) add regressions for `cp /workspace/file ../../../etc/passwd`, `tar -cf /workspace/out.tar ../../..`, and a positive `cp /workspace/file /workspace/sub/../safe`; (e) align this with the boundary-aware classifier required by #566 so workspace-write/read-only path validation shares one per-operand path-scope primitive. **Why this matters:** path validation is supposed to make workspace escapes visible. A global substring exemption lets one benign workspace path launder a different `../` operand, producing false `Allow` evidence for commands that still target outside the workspace. Source: gaebal-gajae dogfood response to Clawhip message `1507231563484500048` on 2026-05-22.