fix: land remaining v0.6 blockers (codex passes 3-6, /code-review, #183 security, #184 docs)#185
Merged
Merged
Conversation
…ter valid
Codex 3rd-pass: two more fixes before the release gate clears.
[P1] CLI run_cli_restore's post-confirm re-read ignored the
read_all `skipped` count. A malformed line appended during the
confirm prompt would slip past the tail-id check (corrupt entries
don't shift the trailing ULID — they're invisible to it) and apply
the stale restore against a partial log. Same shape as the GUI
gate broadened in the previous commit; the CLI gate now mirrors it.
[P2] `archive_sort_key` returned `(stem, u32::MAX)` for unknown
archive shapes, but a tuple sorts on the first element first — so a
stray `audit-0000.jsonl` lex-sorted *before* `audit-2026-05.jsonl`
regardless of the MAX. Restructured to `(is_unrecognized, year_month,
suffix)`: the bool sorts parseable archives ahead of every unknown
shape, with year_month + suffix providing chronological order within
each group.
Tests:
- `cli_undo_refuses_when_audit_log_degrades_during_confirm_prompt`:
spawn `undo`, wait for prompt, inject a malformed line, send y,
assert non-zero exit + "degraded" / "unreadable" on stderr.
- `archive_sort_key_parks_unknown_shapes_after_valid` covers both
the direct key comparison and the actual sort.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… file Codex 4th-pass surfaced two real release-blockers exposed by the multi-key restore-plan dedupe fix: [P1] CLI `--project-dir` was stored verbatim in `ScopePaths`. A relative path landed in the audit log's `file_path` values, so a later `undo` / `redo` / `restore` run from a different cwd would resolve those paths against the new cwd — restoring or creating files under the wrong directory entirely. `resolve_paths` now absolutizes both `--project-dir` and `--home-dir` against the process cwd before deriving any scope paths. [P1] `apply_restore_plan` issued one save per `plan.targets[i]`. A restore-to-point window touching two top-level keys in the same settings.json (now possible thanks to the dedupe fix in #163 / #167) loaded the file twice with the same stamp, mutated each load for its own key, then saved twice. The second save would either fail with `ConcurrentModification` (the first save changed the file out from under it) or, on coarse-mtime filesystems, overwrite the first key's restoration with a doc that didn't include that change. Restructured to load once per unique file and accumulate every plan target's mutation into one `new_doc` + one save; phase 3 still returns one `audit::Side` per target so the wire format is unchanged. Tests: - `apply_restore_plan_coalesces_two_targets_in_one_file`: writes a file with two keys at their post-change state, builds a plan with two targets on that file, applies, asserts both keys are restored in a single save. - `resolve_paths_absolutizes_relative_project_dir`: pins the new behavior so a future refactor can't quietly drop it. - Existing `resolve_paths_*` tests updated to use tempdir paths (the literal `/tmp/...` strings broke on Windows where they're not absolute, and absolutize() would resolve them against the test cwd). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…--yes Codex 5th-pass: two more narrow but real edge cases. [P1] `resolve_paths`'s early-return branch (no `--project-dir`) passed the raw `--home-dir` straight into `scope::resolve_with_home` without absolutizing it. A relative `--home-dir` would persist relative `file_path` values for user / user-local scopes into the audit log, with the same wrong-cwd-on-restore failure mode the project-dir fix already addressed. Absolutizing now happens at the top of `resolve_paths` so both branches benefit. [P1] `run_cli_restore` skipped the post-confirm log re-read entirely under `--yes`. Even without a prompt window, there's a real (though narrower) window between the caller's initial `read_audit_log` and the apply call where a concurrent GUI/CLI append can land. Lifted the re-read out of the `!yes` branch so it runs immediately before `apply_restore_plan` regardless of prompt path. The tail-id + degraded-log refusals now cover every restore apply. Tests: existing coverage (the prompt-window staleness test and the degraded-log test) still exercises the relevant paths. The new no-prompt window is short enough that a deterministic test would require subprocess timing as fragile as the prompt-based ones we already have, and the read_audit_log gate at the cmd_* level already catches the broader case. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…111) Codex 6th-pass [P1]. The cross-project Move-to submenu (#111) let the user pick `OtherProject > Local` / `OtherProject > Project` for a rule, but the click handler dropped `project.root` and called `onPick(target)` with only the scope. `main.ts`'s `moveLeaf` then invoked `apply_move_leaf` with the *current* `state.projectDir`, so the rule landed in the currently-open project's settings file — not the project the user clicked. A user with two known projects could silently modify the wrong repository's Claude settings. Fix plumbs `project.root` through three layers: - `buildMoveToSubmenu`'s `onPick` callback now takes `(target, projectRoot?)`. Cross-project items pass `project.root`; same-project items keep the old single-arg shape. - The two `buildMoveToSubmenu` callers (`leafContextMenuItems`, `combinedChipContextMenuItems`) forward the optional projectRoot into `props.onMoveLeaf` via a new `MoveOptions.projectDir`. - `moveLeaf` in `main.ts` uses `opts?.projectDir ?? state.projectDir` so the override wins when set. Notes: - For cross-project moves from User scope (the intended use), this works fully: the resolver finds the source under `~/.claude/` and the destination under the override's project root. - For cross-project moves whose source is the current project's `Local`/`Project`, the backend's single-project_dir command can't express "read from project A, write to project B" — those will now error with "source not found in project B" instead of silently corrupting project A's settings, which is the meaningful half-fix. Full cross-project moves with non-user sources need a backend `project_dir_from` / `project_dir_to` split; that's a follow-up issue, not a v0.6 blocker. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…config-dir env hook Claude /code-review against the codex-cleared PR found two more real issues: [P1] `moveLeaf` called `await load(projectDir)` after the apply, using the cross-project override path when set. For a User-scope rule moved into OtherProject, the GUI would silently context-switch to OtherProject — scopes panel re-renders for the other project, file watcher reinstalls on the other paths, recent-projects LRU promotes the other project. The user expected to keep viewing the current project (the rule still lived on disk wherever the source scope did). Now `load(state.projectDir)` reloads the originally viewed project; the destination write still landed correctly. [doc] `CLAUDE_SCOPE_CONFIG_DIR` env hook was added for tests but the doc comment described it as "also a useful escape hatch for users." That's misleading — the override has no UI affordance, no sandbox banner, so a user who sets it can silently desync their real config from the one ClaudeScope reads. Rewrote the comment to clearly tag it as a developer/test hook and point at the right fix (route through RuntimeOverrides + sandbox banner) if we ever want to expose it for real. Tests: existing 268 lib + 20 CLI + 120 JS pass; no behavior the existing tests covered changed (the cross-project Move-to flow is exercised manually, no integration test for the cross-project case exists yet — that lives with the #179 backend follow-up). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…set (#183, #180) Closes #183 (security — path injection via audit log). Surfaced by `/security-review` on PR #178 at confidence 0.95. Bundles the #180 follow-up because the security gate exposed it as an immediate blocker. ## The vulnerability (#183) The undo/redo/restore-to-point pipeline trusted the `file_path: PathBuf` field of `audit::Side` records deserialized from `~/.claude/claude-scope/audit.jsonl` and wrote to that path directly via `io_atomic::save`. An attacker who could append a single well-formed JSON line to `audit.jsonl` (cloud-sync collision, a previously-compromised tool with user-scope write, a malicious package postinstall, etc.) could cause ClaudeScope to write attacker-controlled JSON to any path the user could write to. The parent-directory auto-creation in `atomic_write_json` (`fs:: create_dir_all`) extended the primitive to "create any missing dirs along the way." `claude-scope-cli undo --yes` bypassed the confirmation preview entirely, making the attack scriptable. The codex+code-review iteration loop on PR #178 had focused on audit-log integrity (rotation, locking, dedupe, atomicity). The path allowlist gap survived all six codex passes and the Claude code-review pass — it's the kind of issue the iterative review caught only when explicitly framed as a security review. ## Fix New `validate_audit_records(records, home_dir)` helper enforces a per-record allowlist: each Side's `file_path` must equal one of the four resolved `ScopePaths` slots (`local`, `project`, `user_local`, `user`) for the record's own `project_dir`, against the active home override. Canonicalization handles `/var → /private/var` (macOS) and `\?\` extended-path (Windows) drift. Basename allowlist (`settings.json` / `settings.local.json`) is defense in depth. Validation runs at every audit-log boundary: - GUI: `undo_redo_target` validates the resolved target record before returning. `restore_to_point_preview` and `apply_restore_to_point` validate the whole window `&records[target_idx..]` — each record carries its own `project_dir`, so the allowlist scopes per-record. - CLI: `cmd_undo` / `cmd_redo` / `cmd_restore` validate at the same boundary the audit log is read at. `--yes` no longer bypasses validation. `build_restore_plan` and `plan_restore_to` stay pure (no signature change) so the 17 existing test sites that call them with manually- constructed plans don't need updating. ## #180 fix (bundled) CLI `cmd_move` previously recorded `project_dir: None` on every audit record. The new validator correctly refused those records as "user-only scope only" — which broke `undo` / `redo` / `restore` against any CLI-initiated move. Fixed in the same commit: the record now carries `Some(paths.project_dir.clone())` to match the GUI's `apply_move_leaf` shape. Tests updated. ## Tests - `validate_audit_records_refuses_path_outside_scope_allowlist`: hostile Side pointing at `tmp.path()/malicious-target.json` rejected with "path injection refused" message. - `validate_audit_records_refuses_non_settings_basename`: even a path under the legitimate project directory but with the wrong basename is rejected. - `validate_audit_records_accepts_legitimate_record`: sanity that the gate doesn't fail-closed on real records. - `cli_undo_refuses_audit_record_pointing_outside_scope_allowlist` (integration test): writes a hostile audit record via the real `audit::` API, runs `claude-scope-cli undo --yes`, asserts non-zero exit + path-injection-refused message + the attacker's target file does NOT exist (write was prevented before any I/O). - Existing CLI tests `logged_move` helper updated to thread project_dir through so the new validator accepts the seed records. Total: 257 lib + 21 CLI integration + 120 JS tests pass. Closes #183, closes #180. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…eat model `/document-release` sweep over the v0.6 ship (PRs #178 + #184). The audit-log architecture doc, CLI reference, undo/redo user guide, and the security page all had drift relative to what shipped — the six codex passes + Claude code-review pass + security review added behaviors that the docs hadn't caught up to. Updates: - `docs/src/architecture/audit-log.md`: new "Security invariants" section documenting the path allowlist (#183), degraded-log refusal (#170), and tail-ID stalecheck (#165/#171). Also bumped the stale `claude_scope_version` example from 0.3.0 to 0.6.0 in the record-shape snippet. - `docs/src/security.md`: new "Threat model" section covering hostile audit-log entries (linking to the allowlist invariants) and hand-edited settings files (linking to atomic-writes), plus explicit out-of-scope items (multi-user systems, resource exhaustion, binary tampering). - `docs/src/user-guide/undo-redo.md`: documented the three new user-visible refusals — the "log degraded" tooltip / button disable, the "log changed since the preview" staleness error, and the "path injection refused" hostile-record message. - `docs/src/reference/cli.md`: new "Refusals" subsection covering the same three error states for `undo` / `redo` / `restore`, including the explicit note that `--yes` does not bypass any of them. README.md already had v0.6 feature coverage (audit log bullet + keybindings table entry) from earlier commits; no further edits. No source-code TODOs are stale — the audit/restore code is comment- dense but every comment names a live invariant. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
1 participant
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.
Summary
PR #178 landed only its first 5 cluster commits on `dev`. PR #184 then merged into `fix/v0.6-release-blockers` (the feature branch), not into `dev`. Net result: 7 v0.6 release-blocker commits never reached `dev` — including the audit-log path-injection fix (#183) and several codex-pass correctness fixes.
This PR cherry-picks those 7 commits onto a fresh branch off `dev`:
Cherry-picks were clean — no manual conflict resolution. Each commit's original message and authorship preserved.
Why this is its own PR
GitHub merged PR #184 into `fix/v0.6-release-blockers`, not into `dev`, because that was its base. The `Closes #N` keywords on `e901b51` / `81012ba` therefore never fired, leaving #183 + #180 open. This PR's merge to `dev` will close them.
Test plan
v0.6 release-gate impact
Once this lands, the v0.6 release-gate (#143) clears its final code-side checkbox. Then Actions → Release Please → Run workflow to cut the tag.
Closes #183, closes #180, closes #177.
🤖 Generated with Claude Code