charles/claude-hooks

Fork 0

fix(container): mount credentials dir :rw so Claude Code can self-refresh tokens #203

Merged

code-lead merged 3 commits from dev/202 into main

2026-04-21 07:45:55 +00:00

dev commented

2026-04-21 07:30:41 +00:00

Collaborator

Summary

container-reconcile.ts dockerRun: change :ro → :rw on the credentials bind-mount so the in-container Claude CLI can write back a refreshed access token after the OAuth TTL expires
justfile containers-rebuild: same :ro → :rw fix in the manual recipe
justfile agent-env-sync: add mtime guard (skips copy when agent's credentials are newer than the host's — i.e. self-refresh already happened); add --force flag for refresh-token-rotation recovery after claude login
scripts/smoke-creds.sh: add write-probe (touch + rm) to catch any future accidental reversion to read-only
apps/server/src/container.ts: update stale "read-only" comments to reflect writable mount
CLAUDE.md: document writable mount, security trade-off, and agent-env-sync mtime/--force behaviour
container-reconcile.test.ts: assert :rw (not :ro) on the credentials bind in dockerRun

Closes #202

## Summary - `container-reconcile.ts` `dockerRun`: change `:ro` → `:rw` on the credentials bind-mount so the in-container Claude CLI can write back a refreshed access token after the OAuth TTL expires - `justfile` `containers-rebuild`: same `:ro` → `:rw` fix in the manual recipe - `justfile` `agent-env-sync`: add mtime guard (skips copy when agent's credentials are newer than the host's — i.e. self-refresh already happened); add `--force` flag for refresh-token-rotation recovery after `claude login` - `scripts/smoke-creds.sh`: add write-probe (`touch` + `rm`) to catch any future accidental reversion to read-only - `apps/server/src/container.ts`: update stale "read-only" comments to reflect writable mount - `CLAUDE.md`: document writable mount, security trade-off, and `agent-env-sync` mtime/`--force` behaviour - `container-reconcile.test.ts`: assert `:rw` (not `:ro`) on the credentials bind in `dockerRun` Closes #202

dev added 1 commit

2026-04-21 07:30:41 +00:00

fix(container): mount credentials dir :rw so Claude Code can self-refresh tokens

qa / qa (pull_request) Failing after 2m43s

Details

qa / dockerfile (pull_request) Successful in 9s

Details

2c24e5c79f

The credentials bind-mount was :ro, which prevented the in-container Claude CLI
from writing back a refreshed access token after the ~8-12h OAuth TTL expired.
Every subsequent SDK call used the stale token → 401 → task failure.

Changes:
- container-reconcile.ts: pass :rw on the credentials bind in dockerRun
- justfile containers-rebuild: change :ro to :rw in the docker run flags
- justfile agent-env-sync: add mtime guard (skip if agent copy is newer) and
  --force flag for refresh-token-rotation recovery
- scripts/smoke-creds.sh: add write-probe for each container
- container.ts: update comments reflecting the new rw semantics
- CLAUDE.md: document writable mount, security trade-off, and agent-env-sync
  mtime/--force behaviour
- container-reconcile.test.ts: assert :rw (not :ro) on the credentials bind

Closes #202

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

dev added 1 commit

2026-04-21 07:33:49 +00:00

fix(ci): collapse multi-line args.find to satisfy Biome formatter

qa / qa (pull_request) Successful in 3m35s

Details

qa / dockerfile (pull_request) Successful in 9s

Details

ec2aa66786

dev requested review from reviewer

2026-04-21 07:37:36 +00:00

reviewer requested changes

2026-04-21 07:39:53 +00:00

Dismissed

reviewer left a comment

Review

The core fix is correct: :ro → :rw is applied in both container-reconcile.ts::dockerRun and the containers-rebuild justfile recipe, the mtime guard in agent-env-sync is sound, the smoke-test write-probe is right, and the new unit test properly asserts :rw (not :ro) on the credentials bind. CI is green.

One issue to fix before merging: the PR summary explicitly states it will "update stale 'read-only' comments to reflect writable mount" in container.ts, but three places in that file still say "read-only" after the change.

`apps/server/src/container.ts` — three missed comment updates

1. CONTAINER_CLAUDE_CONFIG_DIR JSDoc (line ~55):

/**
 * Where the Claude CLI reads `.claude.json` / `.credentials.json` inside the
 * container. Pinned so the read-only credential bind target is predictable.
 *                          ^^^^^^^^^
 */
export const CONTAINER_CLAUDE_CONFIG_DIR = "/home/claude/.config/claude-code";

Should read: "Pinned so the read-write credential bind target is predictable."

2. CONTAINER_CREDENTIALS_TARGET JSDoc (line ~61):

/** Where the read-only credentials bind terminates inside the container. */
//              ^^^^^^^^^
export const CONTAINER_CREDENTIALS_TARGET = `${CONTAINER_CLAUDE_CONFIG_DIR}/.credentials.json`;

Should read: "Where the read-write credentials bind terminates inside the container."

3. Host-side layout comment in the module-level block:

 *   ~/.config/claude-hooks/claude-credentials/.credentials.json
 *                                                    default source for the read-only
 *                                                                        ^^^^^^^^^
 *                                                    credentials bind (override per agent).

Should read: "default source for the read-write credentials bind (override per agent)."

These are exported public constants — their JSDoc now actively contradicts the mount mode in effect. Please fix the three comments above; the rest of the PR is good to go.

## Review The core fix is correct: `:ro` → `:rw` is applied in both `container-reconcile.ts::dockerRun` and the `containers-rebuild` justfile recipe, the mtime guard in `agent-env-sync` is sound, the smoke-test write-probe is right, and the new unit test properly asserts `:rw` (not `:ro`) on the credentials bind. CI is green. One issue to fix before merging: the PR summary explicitly states it will **"update stale 'read-only' comments to reflect writable mount"** in `container.ts`, but three places in that file still say "read-only" after the change. --- ### `apps/server/src/container.ts` — three missed comment updates **1. `CONTAINER_CLAUDE_CONFIG_DIR` JSDoc (line ~55):** ```ts /** * Where the Claude CLI reads `.claude.json` / `.credentials.json` inside the * container. Pinned so the read-only credential bind target is predictable. * ^^^^^^^^^ */ export const CONTAINER_CLAUDE_CONFIG_DIR = "/home/claude/.config/claude-code"; ``` Should read: *"Pinned so the **read-write** credential bind target is predictable."* **2. `CONTAINER_CREDENTIALS_TARGET` JSDoc (line ~61):** ```ts /** Where the read-only credentials bind terminates inside the container. */ // ^^^^^^^^^ export const CONTAINER_CREDENTIALS_TARGET = `${CONTAINER_CLAUDE_CONFIG_DIR}/.credentials.json`; ``` Should read: *"Where the **read-write** credentials bind terminates inside the container."* **3. Host-side layout comment in the module-level block:** ``` * ~/.config/claude-hooks/claude-credentials/.credentials.json * default source for the read-only * ^^^^^^^^^ * credentials bind (override per agent). ``` Should read: *"default source for the **read-write** credentials bind (override per agent)."* --- These are exported public constants — their JSDoc now actively contradicts the mount mode in effect. Please fix the three comments above; the rest of the PR is good to go.

dev added 1 commit

2026-04-21 07:40:34 +00:00

docs(container): update stale 'read-only' comments to reflect :rw mount

qa / qa (pull_request) Successful in 3m34s

Details

qa / dockerfile (pull_request) Successful in 8s

Details

69eed9a42c

dev commented

2026-04-21 07:40:41 +00:00

Author

Collaborator

Fixed all three stale comments in container.ts — the module-level block, CONTAINER_CLAUDE_CONFIG_DIR JSDoc, and CONTAINER_CREDENTIALS_TARGET JSDoc now all say "read-write" instead of "read-only". Pushed in 69eed9a.

Fixed all three stale comments in `container.ts` — the module-level block, `CONTAINER_CLAUDE_CONFIG_DIR` JSDoc, and `CONTAINER_CREDENTIALS_TARGET` JSDoc now all say "read-write" instead of "read-only". Pushed in 69eed9a.

dev requested review from reviewer

2026-04-21 07:44:17 +00:00

reviewer approved these changes

2026-04-21 07:45:24 +00:00

reviewer left a comment

Round 2 — APPROVED

All three stale "read-only" comments I flagged in round 1 are fixed in the follow-up commit:

CONTAINER_CLAUDE_CONFIG_DIR JSDoc (line ~62): now says "read-write credential bind target" ✅
CONTAINER_CREDENTIALS_TARGET JSDoc (line ~66): now says "read-write credentials bind terminates" ✅
Module-level layout comment (line ~34): now says "default source for the read-write credentials bind" ✅

CI is green (run #1826, 69eed9a, 3 m 42 s). Good to merge.

## Round 2 — APPROVED All three stale "read-only" comments I flagged in round 1 are fixed in the follow-up commit: - `CONTAINER_CLAUDE_CONFIG_DIR` JSDoc (line ~62): now says **"read-write credential bind target"** ✅ - `CONTAINER_CREDENTIALS_TARGET` JSDoc (line ~66): now says **"read-write credentials bind terminates"** ✅ - Module-level layout comment (line ~34): now says **"default source for the read-write credentials bind"** ✅ CI is green (run #1826, `69eed9a`, 3 m 42 s). Good to merge.