docs(04): add code review fix report
This commit is contained in:
parent
0922b752a5
commit
390812625d
1 changed files with 59 additions and 0 deletions
59
.planning/phases/04-auth-passthrough/04-REVIEW-FIX.md
Normal file
59
.planning/phases/04-auth-passthrough/04-REVIEW-FIX.md
Normal file
|
|
@ -0,0 +1,59 @@
|
||||||
|
---
|
||||||
|
phase: 04-auth-passthrough
|
||||||
|
fixed_at: 2026-04-10T00:00:00Z
|
||||||
|
review_path: .planning/phases/04-auth-passthrough/04-REVIEW.md
|
||||||
|
iteration: 1
|
||||||
|
findings_in_scope: 4
|
||||||
|
fixed: 4
|
||||||
|
skipped: 0
|
||||||
|
status: all_fixed
|
||||||
|
---
|
||||||
|
|
||||||
|
# Phase 04: Code Review Fix Report
|
||||||
|
|
||||||
|
**Fixed at:** 2026-04-10
|
||||||
|
**Source review:** .planning/phases/04-auth-passthrough/04-REVIEW.md
|
||||||
|
**Iteration:** 1
|
||||||
|
|
||||||
|
**Summary:**
|
||||||
|
- Findings in scope: 4 (2 Critical, 2 Warning; Info excluded by fix_scope)
|
||||||
|
- Fixed: 4
|
||||||
|
- Skipped: 0
|
||||||
|
|
||||||
|
## Fixed Issues
|
||||||
|
|
||||||
|
### CR-01: CREDS_FILE path resolves against the wrong directory on the host
|
||||||
|
|
||||||
|
**Files modified:** `claudebox.sh`
|
||||||
|
**Commit:** adb9dd1
|
||||||
|
**Applied fix:** Changed `CREDS_FILE` from `$HOME/.claude/.credentials.json` to `$HOME/.claudebox/.credentials.json`. Added comments explaining that the `~/.claude -> ~/.claudebox` symlink only exists inside the sandbox at runtime, so host-side credential detection must use the real claudebox config directory path.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
### CR-02: Credentials bind target uses a symlink path — may silently fail
|
||||||
|
|
||||||
|
**Files modified:** `claudebox.sh`
|
||||||
|
**Commit:** adb9dd1
|
||||||
|
**Applied fix:** Changed the `BWRAP_ARGS` credential bind destination from `$HOME/.claude/.credentials.json` (symlink path, unresolvable by bwrap at bind time) to `$HOME/.claudebox/.credentials.json` (canonical real directory). Also updated the `--dry-run` output block to print the corrected destination path.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
### WR-01: Credentials file mounted read-write instead of read-only
|
||||||
|
|
||||||
|
**Files modified:** `claudebox.sh`
|
||||||
|
**Commit:** adb9dd1
|
||||||
|
**Applied fix:** Changed `--bind` to `--ro-bind` for the credentials bind in both the `BWRAP_ARGS` array (line ~366) and the `--dry-run` output block (line ~333). Also updated the `print_audit` mounts display to show `(read-only)` and display `$CREDS_FILE` (the actual host source path) instead of the hardcoded symlink path inside the sandbox.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
### WR-02: dry-run ENV_ARGS loop hard-codes stride of 3 — breaks if any non-setenv arg is added
|
||||||
|
|
||||||
|
**Files modified:** `claudebox.sh`
|
||||||
|
**Commit:** 0922b75
|
||||||
|
**Applied fix:** Added a guard assertion before the stride-3 loop that checks `${#ENV_ARGS[@]} % 3 != 0` and exits with a `BUG:` message if the invariant is violated. This catches any future breakage immediately rather than producing silently mangled output. Also changed `(( dry_run_i += 3 ))` to `dry_run_i=$(( dry_run_i + 3 ))` to use safe arithmetic assignment compatible with `set -euo pipefail` (which `writeShellApplication` enables).
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
_Fixed: 2026-04-10_
|
||||||
|
_Fixer: Claude (gsd-code-fixer)_
|
||||||
|
_Iteration: 1_
|
||||||
Loading…
Add table
Reference in a new issue