Skip to content

fix(docker): bind-mount ownership, Compose override discovery, and the default runtime account - #377

Open
opticon454 wants to merge 8 commits into
Ark0N:masterfrom
opticon454:bugfix-docker-user-perms
Open

fix(docker): bind-mount ownership, Compose override discovery, and the default runtime account#377
opticon454 wants to merge 8 commits into
Ark0N:masterfrom
opticon454:bugfix-docker-user-perms

Conversation

@opticon454

@opticon454 opticon454 commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Problem

Two independent bugs in the Docker deployment.

1. The container crash-loops whenever a bind source does not already exist

docker-compose.yaml binds CODEMAN_APPDATA_PATH and CODEMAN_CASES_PATH from the host. When either path does not exist yet - a first run, a cleared application-data directory, a restored backup - the Docker daemon creates it owned by root:root. The server runs unprivileged as CODEMAN_RUNTIME_USER, so it cannot create its own state directory, and the container restarts forever on:

Failed to start web server: EACCES: permission denied, mkdir '/home/<user>/.codeman'

Start-Codeman.sh works around this by preparing the directory on the host first, so the failure only appears when Compose is run directly - which docker/README.md documents as a supported path.

2. Start-Codeman.sh silently discards docker-compose.override.yml

The script passes -f "$compose_file", and naming a Compose file explicitly disables Compose's automatic discovery of the override file. Any customisation placed in the conventional override file is dropped without warning, and the only way to notice is to inspect the running container.

3. The default runtime account is named opencode

CODEMAN_RUNTIME_USER defaults to opencode, which no longer matches the project and is confusing in a deployment whose every other identifier is codeman. The example application-data path also carries a Coding/ component that means nothing outside the original author's host.

Changes

  • docker/entrypoint.sh (new) - starts as root, corrects the ownership of both bind mounts, then drops to PUID:PGID with setpriv.
  • docker/server.Dockerfile - the USER instruction is replaced by that entrypoint; CMD is unchanged. PUID/PGID are also exported as runtime ENV defaults so the image behaves correctly when run without Compose, rather than depending on build args alone.
  • docker/docker-compose.yaml - adds back only the four capabilities the chown and the privilege drop require, so cap_drop: ALL continues to remove everything else.
  • docker/Start-Codeman.sh - collects the -f arguments into an array, appends the override file when present, and reuses that array for the final launch so the two cannot drift apart again. Both .yml and .yaml are checked, in Compose's own precedence order, and the chosen file is reported on startup.
  • docker/.env.example, docker/server.Dockerfile - rename the default runtime account to codeman, in both the example file and the ARG that mirrors it, and correct the comment that referred to /home/opencode/codeman-cases. Simplify the example paths to /mnt/user/appdata/codeman and its codeman-cases child. The npm package opencode-ai and the references to the OpenCode CLI are deliberately untouched, as those name a different tool.
  • docker/README.md, .gitignore - document the override file, including the two things that are easy to get wrong (it is ignored when -f is passed without naming it, and it cannot remove a key such as ports, which Compose concatenates), and ignore it in Git.

4. CODEMAN_ALLOWED_HOSTS is documented nowhere in the Docker deployment docs

The variable is a real, working application setting (the Host-header allowlist in network-auth-policy.ts), but nothing in docker/README.md mentions it, and docker-compose.yaml does not forward it from .env into the container - Compose only passes through variables explicitly listed under environment:. A reverse-proxied deployment fails with 403 Forbidden: host not allowed with no pointer back to the fix.

5. Build-artefact volumes go stale after an externally-triggered rebuild

codeman-node-modules and codeman-dist (docker-compose.yaml) are seeded from the image only while empty. The in-app self-updater never hits this - it rebuilds INSIDE the running container, into the very volume already in use - but a docker compose build triggered from outside it (Start-Codeman.sh, after a manual git pull) does: the container comes back up looking unchanged, serving stale compiled routes against current source. This bit a real deployment during this work: a source fix landed, docker compose build && up ran cleanly, and the old behaviour persisted until the volume was cleared by hand.

Compatibility

Two guards keep existing deployments working:

  • A container started with an explicit user: is left alone entirely. The entrypoint execs straight through, with no elevation and no chown.

  • A chown that fails is a warning, not an error. Bind mounts backed by NFS, CIFS or a rootless daemon can refuse chown while remaining perfectly writable, and those deployments must keep starting.

  • docker/README.md - documents CODEMAN_ALLOWED_HOSTS, why docker-compose.yaml does not forward it, and the override needed to do so, using the Local customisation mechanism already described earlier in the same file.

  • docker/Start-Codeman.sh, scripts/self-update.sh, docs/docker-self-update.md - Start-Codeman.sh compares the checkout's HEAD commit and package-lock.json hash against a recorded marker (docker-build-source.json) and clears just the affected volume(s) before its own --build when either moved. The in-place self-update path writes that same marker after a successful build, so the two mechanisms agree on what the volumes currently reflect - without it, the next plain Start-Codeman.sh run would see the HEAD self-update just checked out, not recognise it as already accounted for, and wipe the volumes self-update just correctly rebuilt right back to the older baked image.

Testing

Verified against a full rebuild on Docker 29 / Compose v5.5, with PUID=99 and PGID=100:

Case Result
Application-data directory deleted entirely Self-heals, logs corrected ownership of ... to 99:100, container healthy
Directory forced to root:root Self-heals, ownership restored
Already-correct tree No chown performed, verified on a fresh container so no stale log could mask it
Server process identity Uid 99/99/99/99, Gid 100/100/100/100, Groups: 281 - the socket group is kept and root's group is dropped
Docker socket as the app user Usable, Docker cases work
Container started with -u 1000:1000 uid=1000, straight exec, no elevation attempted
Read-only, unchownable mount Warns and continues, ends as uid=99 - does not abort
Start-Codeman.sh with an override present Both -f flags passed
Start-Codeman.sh with no override Only the base -f flag, no stray argument
Volume staleness (issue 5) Ran Start-Codeman.sh for real against the live Unraid deployment: correctly detected the source change on first run with no prior marker, resolved the real prefixed volume names via docker volume ls --filter label=com.docker.compose.volume=..., cleared both volumes, rebuilt, and came back up healthy; marker file confirmed written with the real HEAD commit and lockfile hash afterward

The five commits are independent; happy to split them into separate PRs if you would prefer that.

🤖 Generated with Claude Code


Folded in from #384 (opened, then closed as superseded by this consolidation once it turned out this identity has only read access to this repo and cannot merge PRs here directly).

@opticon454 opticon454 changed the title fix(docker): repair bind-mount ownership on start, and honour the Compose override file fix(docker): bind-mount ownership, Compose override discovery, and the default runtime account Sep 4, 2026
@opticon454
opticon454 force-pushed the bugfix-docker-user-perms branch from 57abc1a to b698b8e Compare September 4, 2026 11:37
opticon454 added a commit to opticon454/Codeman that referenced this pull request Sep 4, 2026
docker/agent.Dockerfile hardcoded the four npm-published CLIs it installs, one
of the several lists that had to be kept in step with the registry by hand.

It now takes them as `ARG CLI_NPM_PACKAGES`, supplied by
scripts/build-agent-image.mjs from config/clis.stock.json, with the default set
to today's list so a bare `docker build` still produces the same image. The arg
is expanded unquoted because word splitting is what turns the list into several
arguments, which is exactly why every token is validated against
^[@A-Za-z0-9][@A-Za-z0-9/._-]*$ on the producing side; a package name carrying a
space or a metacharacter is refused rather than reaching the RUN line. Verified
by building the layer: four packages in, four arguments out, and the default
still applies with no arg.

The list is filtered on each entry's `enabled` flag — the field whose absence
was the maintainer's §3 finding, where a CLI shipping disabled still got baked
into every image. No stock entry is disabled today, so that assertion would pass
vacuously; a unit test feeds the pure helper a fabricated disabled entry so the
fix is covered now rather than the first time someone ships one.

⚠️ It reads the STOCK catalogue, never the merged registry. A user's
~/.codeman/clis.json must not change what is inside an image tagged
codeman/agent:base, or two machines holding that tag hold different images.

Four CLIs keep hand-written layers because the registry cannot describe what
makes them special: pi's --ignore-scripts, deepseek's pnpm companion and dsh-tui
profile, and the three standalone installers. Rather than extend the schema for
a Docker-only benefit, the coverage test requires each to carry a written reason
AND still be present, so an exclusion cannot quietly become an omission.

There are two producers of this command line and there have to be — the .mjs
cannot import TypeScript, and src/docker-hosts.ts builds the same argv for the
in-app auto-build — so a parity test pins them together, package list, arg pairs
and rendered argv. Their order is pinned too: a different order is a different
RUN string and so a needless cache miss between the two build paths.

docker/server.Dockerfile is deliberately NOT edited (PRs Ark0N#373 and Ark0N#377 both
modify it); its narrower list is asserted as a declared omission list instead, so
the divergence is reviewable without touching the file.

Also fixes the in-app hint at index.html, which the new coverage test caught
still omitting omp.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015EMxQreQUZX5ZyybxAGh12
@opticon454

opticon454 commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

GitHub's conflict flag traced down to two real conflicts, both caused by #373 (66eb01ba, merged after this PR's base) touching the same two files in overlapping places. The other two files this PR shares with #373 (docker/.env.example, docker/server.Dockerfile) and one more (docker/docker-compose.yaml) changed on both sides too but auto-merge cleanly — no markers there.

docker/README.md

Both PRs append a new ## section at the same spot, right after the ownership-fix paragraph:

They're unrelated sections that happen to land at the same insertion point — additive, not competing. Either order works; keep both.

docker/Start-Codeman.sh

Both touch the final docker compose ... up --build -d line:

  • master (feat(docker): restore in-app self-update in the Compose dep #373) resolves repo_path/CODEMAN_REPO_PATH beforehand and runs:
    exec docker compose --env-file "$env_file" -f "$compose_file" up --build -d
  • this PR builds compose_command earlier (bare -f "$compose_file" plus any discovered override file) and runs:
    exec "${compose_command[@]}" up --build -d

This one needs an actual merge of intent, not just picking a side: keep the repo_path resolution from master, but launch via "${compose_command[@]}" so the override file(s) this PR discovers still get threaded through — losing that would silently make the override support this PR adds dead on the self-update path.

Happy to help if useful, but since I don't have push access to bugfix-docker-user-perms a rebase onto current master is the actual fix here.

opticon454 and others added 4 commits September 5, 2026 07:31
Compose binds CODEMAN_APPDATA_PATH and CODEMAN_CASES_PATH from the host. When
either path does not exist yet - a first run, a cleared application-data
directory, a restored backup - the Docker daemon creates it owned by root. The
server runs unprivileged as CODEMAN_RUNTIME_USER, so it cannot create its own
state directory, and the container restarts forever on:

  Failed to start web server: EACCES: permission denied, mkdir '/home/<user>/.codeman'

Start-Codeman.sh already worked around this by preparing the directory on the
host, so the failure only appears when Compose is run directly, which the README
documents as a supported path.

Add docker/entrypoint.sh, which starts as root, corrects the ownership of both
bind mounts, then drops to PUID:PGID with setpriv. The Dockerfile's USER
instruction is replaced by that entrypoint and CMD is unchanged.
docker-compose.yaml adds back only the four capabilities the chown and the
privilege drop require, so cap_drop: ALL continues to remove everything else.

Two guards keep existing deployments working:

- A container started with an explicit `user:` is left alone. The entrypoint
  execs straight through, with no elevation and no chown.
- A chown that fails is a warning, not an error. Bind mounts backed by NFS,
  CIFS or a rootless daemon can refuse chown while remaining perfectly
  writable, and those deployments must keep starting.

PUID and PGID are also exported as runtime environment defaults so the image
behaves correctly when run without Compose, rather than depending on build args
alone.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Naming a Compose file with -f disables Compose's automatic discovery of the
override file, so Start-Codeman.sh silently ignored docker-compose.override.yml.
Any local customisation placed in the conventional override file was dropped
without warning, and the only way to notice was to inspect the running
container.

Collect the -f arguments into an array, append the override file when one is
present, and reuse that array for the final launch so the two cannot drift
apart again. Both .yml and .yaml are checked, in Compose's own precedence
order, and the chosen file is reported on startup.

Document the override file in docker/README.md, including the two things that
are easy to get wrong: it is ignored when -f is passed without naming it, and
it cannot remove a key such as ports, which Compose concatenates. Add the
override file to .gitignore.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
CODEMAN_RUNTIME_USER defaulted to `opencode`, which no longer matches the
project and is confusing in a deployment whose every other identifier is
codeman. Rename the default in .env.example and in the Dockerfile ARG that
mirrors it, and correct the example comment that referred to
/home/opencode/codeman-cases.

Also drop the `Coding/` component from the example application-data path.
CODEMAN_APPDATA_PATH and CODEMAN_CASES_PATH now suggest /mnt/user/appdata/codeman
and its codeman-cases child, matching the account name and removing a directory
level that meant nothing outside the original author's host. README.md is
updated to match, including the chown example.

The npm package `opencode-ai` and the references to the OpenCode CLI are
deliberately left alone: those name a different tool, not this account.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
CODEMAN_ALLOWED_HOSTS is a real, documented application setting (the Host-
header allowlist in network-auth-policy.ts), but docker-compose.yaml does not
forward it from .env into the container - Compose only passes through
variables explicitly listed under environment:, and this is not one of them.
Set without that passthrough, any request through a reverse proxy is rejected
with 403 Forbidden: host not allowed before it reaches any handler, and
nothing in the Docker deployment docs said why.

Document the variable and the override needed to forward it, using the
Local customisation mechanism already described above it.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@opticon454
opticon454 force-pushed the bugfix-docker-user-perms branch from 40d3bce to 0affc10 Compare September 4, 2026 23:33
@opticon454

opticon454 commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

Rebased this branch onto current master and resolved both conflicts:

  • docker/README.md — kept both new sections (master's "Updating", this branch's "Local customisation") one after the other.
  • docker/Start-Codeman.sh — kept master's repo_path resolution and self-update fingerprint block, but changed the final line to exec "${compose_command[@]}" up --build -d so the override-file discovery this branch adds still applies on that path.

CI is green on the rebased head and GitHub now reports this as cleanly mergeable.

🤖 Generated with Claude Code

opticon454 and others added 2 commits September 5, 2026 16:18
The four CLIs (claude, gemini, codex, opencode) are npm-installed
globally as root during the image build, before the unprivileged
runtime account exists. A session running as that account (e.g. a
codex-mode terminal) then hits EACCES the moment it tries to update
one in place, because npm renames the old package directory aside
before installing the new one, which needs write access to the
parent (/usr/local/lib/node_modules), not just the target package.

Chown that tree plus /usr/local/bin's CLI symlinks to PUID:PGID in
the same step that creates/renames the runtime account.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01R9ZSTEenc8soSu9bTi8Xru
codeman-node-modules and codeman-dist (docker-compose.yaml) are seeded
from the image only while empty, so a rebuilt image's fresh dist/
node_modules sat unused behind old volume content until something
cleared it. The in-app self-updater never hit this (it rebuilds INSIDE
the running container, into the very volume already in use), but a
`docker compose build` triggered from outside it — Start-Codeman.sh,
after a manual `git pull` — did: the container came back up looking
unchanged, serving stale compiled routes against current source.

Start-Codeman.sh now compares the checkout's HEAD commit and
package-lock.json hash against a recorded marker
(docker-build-source.json) and clears just the affected volume(s)
before its own --build when either moved.

The in-place self-update path writes that same marker after a
successful build, so the two mechanisms agree on what the volumes
currently reflect — without it, the next plain Start-Codeman.sh run
would see the HEAD self-update just checked out, not recognise it as
already accounted for, and wipe the volumes self-update just correctly
rebuilt right back to the older baked image.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01R9ZSTEenc8soSu9bTi8Xru
@Ark0N

Ark0N commented Sep 7, 2026

Copy link
Copy Markdown
Owner

Thanks for this, and for rebasing it yourself when #373 landed underneath you. The entrypoint measurement is right: a Compose long-form bind creates a missing host source root-owned, and asking every user to pre-create and chown it by hand is a bad first-run experience. Six commits is a lot to land at once though, and two of them need to change before I can take any of it.

1. The entrypoint chown fires on any owner mismatch, not just the root-owned directory the daemon created (docker/entrypoint.sh:28). The gate is stat -c '%u:%g' == PUID:PGID, so a directory owned by any other account gets recursively re-owned. I reproduced it with the compose caps: a host tree owned by 1000:1000 mounted as CODEMAN_CASES_PATH with PUID=1234 came back 1234:4321 down to the leaf files, with a single log line to say so. Two ordinary deployments hit this on their first start:

  • docker compose up run directly with the default PUID=1000 against a 99:100 Unraid appdata dir. The README tells people to set PUID in that case, and before this PR forgetting it failed loudly with EACCES. Now it silently rewrites the credentials tree to 1000:1000.
  • CODEMAN_CASES_PATH pointed at an existing projects directory owned by the host account, which the README explicitly allows ("unless you deliberately store workspaces elsewhere"), while appdata follows the 99:100 convention.

The case your PR describes (first run, cleared dir, restored backup) is a directory the daemon created as root:root. Please gate the chown on uid == 0 and turn any other mismatch into a clear error naming PUID, PGID and the path, which is exactly how Start-Codeman.sh already treats a root-owned appdata dir. Worth noting separately: Start-Codeman.sh never pre-creates or checks CODEMAN_CASES_PATH the way it does appdata. Adding that host-side would mirror the existing design and mean the script path never needs the in-container chown at all.

2. chown -R of /usr/local/bin hands the root-executed entrypoint and the node binary to the unprivileged runtime account (docker/server.Dockerfile:133). That directory holds node (a 124 MB binary, now writable by that user), npm/npx as symlinks into the also-chowned node_modules, the docker CLI copied at line 66, and after line 158 entrypoint.sh itself. The script stays root-owned, but owning the directory is enough to rename it away and drop a replacement, and line 161 runs that path as root with CHOWN/DAC_OVERRIDE/SETUID/SETGID on every container start. The writable layer survives exactly the restart the self-updater performs, so a session running as the runtime user could arrange for its own script to run as root at the next restart, which undoes the "the server itself never runs privileged" guarantee this PR is adding. In practice that user already holds the Docker socket, which is host-root-equivalent by design, so the marginal escalation is bounded, but chowning the system bin dir is the wrong shape for "let sessions update their CLIs". Install the four global CLIs into a dedicated prefix instead:

ENV NPM_CONFIG_PREFIX=/opt/codeman-cli
ENV PATH=/opt/codeman-cli/bin:$PATH
# npm install -g ... then:
RUN chown -R "${PUID}:${PGID}" /opt/codeman-cli

and leave /usr/local/bin and /usr/local/lib/node_modules root-owned. Commit 49c6353a is also unrelated to the PR title and is the cleanest one to split out.

Smaller, and I can apply these at merge time if you would rather not:

  • docker/Start-Codeman.sh:196: docker volume ls -q --filter label=com.docker.compose.volume=codeman-dist | head -n1 matches every Compose project on the host that declares a volume with that key, because the label is the key and not the project-qualified name. A beta stack started with -p alongside prod shares it and head -n1 picks whichever the daemon lists first, so the refresh can delete the other project's volume. Add --filter label=com.docker.compose.project=<name>, or drop the lookup and run down --volumes when either input moved. Both volumes re-seed from the image by a plain copy, and docs/docker-self-update.md already calls down -v the supported reset.
  • docker/Start-Codeman.sh:21: the override-file order is the reverse of Compose's, and the comment says it matches. Measured on Compose v5.5.0 with both files present: Compose warns and uses docker-compose.override.yml, the script picks .yaml first and says nothing. Swap the candidates, and consider warning when both exist.
  • docker/entrypoint.sh:48: after the drop the process shows CapPrm/CapEff 0 but CapBnd 0xc3. With no-new-privileges that is moot, but setpriv --bounding-set -all is free and makes the dropped state match the intent.
  • Docs that need to move with the code: CLAUDE.md:214 still says a missing bind source "has to be pre-created and chowned", which the entrypoint now handles, and that paragraph does not mention the container starting as root with four capabilities and dropping via setpriv. CLAUDE.md:379 lists docker-env-applied.json but not the new docker-build-source.json. The rename missed docs/docker-compose.md:18 and docker/.env.example:28, so the two example paths now disagree with each other.
  • git_head_commit() is a pure function and three fixtures would pin it (detached HEAD, symbolic ref, packed-refs after gc). I ran those by hand and they resolve correctly. A worktree checkout, where .git is a file containing gitdir:, returns nothing and silently disables the dist refresh, which is consistent with the script's existing -d .git convention but worth a comment.

You offered to split this up, and I think that is the fastest path: the ownership fix, override discovery, the volume refresh and the rename as one PR, with the CLI-chown commit either dropped or reworked to the dedicated prefix. The release note will need a line telling Compose users to run Start-Codeman.sh for this release, which I will write.

Two real bugs the review caught, both verified live against a real
build on the Unraid host:

1. entrypoint.sh's chown fired on ANY ownership mismatch, not just a
   directory the daemon itself created root-owned. A host tree
   legitimately owned by some other account - an existing
   CODEMAN_CASES_PATH the README already allows pointing at a normal
   projects directory, or appdata under a different PUID/PGID
   convention than the one in use - got silently recursively re-owned
   with one log line to explain it. Now gated on the target actually
   being root-owned; anything else is a clean refusal naming the
   directory, its owner, and PUID/PGID. Start-Codeman.sh also now
   pre-creates CODEMAN_CASES_PATH the same way it already did
   CODEMAN_APPDATA_PATH, so Compose never has to materialise a missing
   bind source as root in the first place - the in-container chown
   becomes a safety net, not the primary mechanism.

2. The CLI-update chown (chown -R .../node_modules /usr/local/bin)
   handed the runtime account write access to entrypoint.sh itself
   (root-owned, executed as root on every container start with
   CHOWN/DAC_OVERRIDE/SETUID/SETGID) and the node binary - owning the
   DIRECTORY is enough to rename it aside and drop a replacement, which
   would let a compromised session arrange for its own script to run
   as root at the next restart. The four CLIs now install into a
   dedicated /opt/codeman-cli prefix (NPM_CONFIG_PREFIX); only that
   directory is chowned, /usr/local stays root-owned throughout.

Smaller fixes from the same review:

- Start-Codeman.sh's volume-refresh label filter wasn't project-scoped:
  a second Compose stack on the same host sharing the `codeman-dist`
  volume KEY could have had ITS volume deleted. Added a
  com.docker.compose.project filter, resolved from this stack's own
  `compose config --format json`.
- Override-file precedence was backwards (checked .yaml before .yml;
  Compose actually prefers .yml) - swapped, plus a warning when both
  exist.
- entrypoint.sh's setpriv now also passes --bounding-set -all, so
  CapBnd actually clears post-drop rather than just CapPrm/CapEff.
- A comment on git_head_commit() noting it returns nothing for a
  worktree checkout (.git as a file), consistent with the script's
  existing -d .git convention elsewhere.
- Doc drift: CLAUDE.md's Docker Compose section still described the
  old pre-created-and-chowned-by-hand model and didn't mention the
  root-then-drop entrypoint; the state-files list was missing
  docker-build-source.json; docs/docker-compose.md and
  docker/.env.example still had the pre-rename `Coding/codeman` path
  in one place each.

Verified end to end against a real build on the Unraid host: a
root-owned bind source is corrected as before; a directory owned by
neither root nor PUID:PGID is refused rather than silently rewritten;
a correctly-owned directory is left alone entirely; the four CLIs
resolve via PATH from /opt/codeman-cli while /usr/local/bin,
/usr/local/lib/node_modules and entrypoint.sh itself stay root-owned;
CapBnd is fully cleared post-drop.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01R9ZSTEenc8soSu9bTi8Xru
@opticon454

Copy link
Copy Markdown
Contributor Author

Thanks for the thorough review — pushed a fix addressing all of it (63563def).

1. entrypoint.sh's chown gate. Now gated on uid == 0; anything else refuses with a clear error naming the directory, its actual owner, and PUID/PGID, rather than silently re-owning it. Also added CODEMAN_CASES_PATH pre-creation to Start-Codeman.sh mirroring the existing CODEMAN_APPDATA_PATH handling, per your suggestion — Compose should now never have to materialise either bind source as root in the first place.

2. The CLI-update chown. Reworked to the dedicated-prefix approach exactly as you sketched it: NPM_CONFIG_PREFIX=/opt/codeman-cli, only that directory gets chowned, /usr/local/bin//usr/local/lib/node_modules (and entrypoint.sh itself) stay root-owned throughout.

Smaller ones: volume-refresh lookup is now project-scoped (com.docker.compose.project filter), override-file precedence swapped to .yml-first with a warning when both exist, setpriv --bounding-set -all added, a comment on git_head_commit()'s worktree limitation, and the four doc-drift spots (CLAUDE.md x2, docs/docker-compose.md, docker/.env.example).

Verified all of it against a real build on my own deployment's Docker host rather than just by inspection:

Case Result
Root-owned bind source Auto-corrected as before, entrypoint: corrected ownership of /home/codeman to 99:100
Directory owned by neither root nor PUID:PGID (simulated 5000:5000) Refused cleanly, exit 1, ownership left untouched — this is the exact case you reproduced
Correctly-owned directory No chown attempted, straight through
CLI resolution which claude/codex/gemini/opencode all resolve to /opt/codeman-cli/bin/* via PATH
Ownership boundary /opt/codeman-cli is codeman:users (99:100); /usr/local/bin, /usr/local/lib/node_modules, and entrypoint.sh itself are all still root:root
Capabilities post-drop CapBnd is now 0000000000000000

🤖 Generated with Claude Code

…st at build

/opt/codeman-cli is chowned to PUID:PGID once, at image build time, from
the PUID/PGID build args. That bake only happens when the image is
actually rebuilt (`docker compose up --build`, which Start-Codeman.sh
always does) — a deployment that runs the compose file directly instead
(Unraid's Compose Manager, a native systemd unit, any plain
`docker compose up`/`restart`) can change PUID/PGID in .env and restart
without ever rebuilding. The container then runs as the NEW uid via
entrypoint's setpriv (Linux needs no /etc/passwd entry to setuid to an
arbitrary number) while the CLI directory is still owned by the OLD one
baked into the image layer — silently breaking the self-update-a-CLI-
in-place fix that directory exists for.

Unlike HOME/CODEMAN_CASES_PATH, this one is pure image content Codeman
itself populated, never host data that might legitimately belong to
someone else, so there is no ownership to be careful about — it is
always correct for it to be owned by whoever the container is about to
run as. Re-assert it unconditionally on every start.

Verified live: built an image with PUID=99/PGID=100, ran it with
PUID=1234/PGID=4321 (no rebuild, simulating a changed .env restarted
directly), confirmed /opt/codeman-cli ends up 1234:4321-owned and is
genuinely writable by the running process.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01R9ZSTEenc8soSu9bTi8Xru
@opticon454

Copy link
Copy Markdown
Contributor Author

One gap in 63563def's fix, caught by a user on my own deployment who runs the compose file directly (Unraid's Compose Manager) rather than through Start-Codeman.sh: /opt/codeman-cli was only ever chowned to PUID:PGID at image build time (from the build args), never re-verified at container start the way HOME/CODEMAN_CASES_PATH are. Anyone changing PUID/PGID in .env and restarting without a rebuild — which Start-Codeman.sh always does via --build, but a plain docker compose up/restart does not — would end up running as the new UID while the CLI directory stayed owned by the old, baked-in one, silently breaking the self-update-a-CLI-in-place fix that directory exists for.

Fixed in d7f6b843: entrypoint.sh now re-asserts /opt/codeman-cli's ownership on every start, unconditionally (unlike the host bind mounts, this is pure image content with no "someone else's directory" ambiguity to be careful about). Verified live: built with PUID=99/PGID=100, ran with PUID=1234/PGID=4321 and no rebuild, confirmed the directory ends up 1234:4321-owned and genuinely writable.

🤖 Generated with Claude Code

opticon454 added a commit to opticon454/Codeman that referenced this pull request Sep 8, 2026
docker/agent.Dockerfile hardcoded the four npm-published CLIs it installs, one
of the several lists that had to be kept in step with the registry by hand.

It now takes them as `ARG CLI_NPM_PACKAGES`, supplied by
scripts/build-agent-image.mjs from config/clis.stock.json, with the default set
to today's list so a bare `docker build` still produces the same image. The arg
is expanded unquoted because word splitting is what turns the list into several
arguments, which is exactly why every token is validated against
^[@A-Za-z0-9][@A-Za-z0-9/._-]*$ on the producing side; a package name carrying a
space or a metacharacter is refused rather than reaching the RUN line. Verified
by building the layer: four packages in, four arguments out, and the default
still applies with no arg.

The list is filtered on each entry's `enabled` flag — the field whose absence
was the maintainer's §3 finding, where a CLI shipping disabled still got baked
into every image. No stock entry is disabled today, so that assertion would pass
vacuously; a unit test feeds the pure helper a fabricated disabled entry so the
fix is covered now rather than the first time someone ships one.

⚠️ It reads the STOCK catalogue, never the merged registry. A user's
~/.codeman/clis.json must not change what is inside an image tagged
codeman/agent:base, or two machines holding that tag hold different images.

Four CLIs keep hand-written layers because the registry cannot describe what
makes them special: pi's --ignore-scripts, deepseek's pnpm companion and dsh-tui
profile, and the three standalone installers. Rather than extend the schema for
a Docker-only benefit, the coverage test requires each to carry a written reason
AND still be present, so an exclusion cannot quietly become an omission.

There are two producers of this command line and there have to be — the .mjs
cannot import TypeScript, and src/docker-hosts.ts builds the same argv for the
in-app auto-build — so a parity test pins them together, package list, arg pairs
and rendered argv. Their order is pinned too: a different order is a different
RUN string and so a needless cache miss between the two build paths.

docker/server.Dockerfile is deliberately NOT edited (PRs Ark0N#373 and Ark0N#377 both
modify it); its narrower list is asserted as a declared omission list instead, so
the divergence is reviewable without touching the file.

Also fixes the in-app hint at index.html, which the new coverage test caught
still omitting omp.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015EMxQreQUZX5ZyybxAGh12
Ark0N pushed a commit that referenced this pull request Sep 8, 2026
Claude Code answers an exhausted model budget INSIDE the turn ("You've
reached your Fable limit. Run /usage-credits to continue or switch models
with /model.") and then sits there with nothing to write. The reviewer never
produces a report, so `runTurn` waited out its full 40-minute deadline and
reported a bare "timed out after 40 min without a report", which reads as a
hung reviewer rather than an account that needs attention.

Measured on 2026-09-08: #388, #393, #394 and #377 each lost 40 minutes this
way, and because every attempt counted, all four reached MAX_AUTO_RETRIES and
would NOT have been picked up again once the budget returned. One spent
afternoon quietly took the whole queue out of service.

`findModelLimitNotice()` reads the notice off the pane and `runTurn` returns
a new `limit` outcome instead of waiting. It is consulted in exactly two
places, both of which mean "the turn produced nothing": on a stop where
`isDone()` is still false, and on each timed-out wait slice. A review that
merely discusses usage limits in its own findings therefore cannot be
mistaken for one that hit the wall, and the pattern matches neither the model
name nor a straight apostrophe, since the pane renders a typographic one and
every model prints the same sentence.

A spent budget is an account condition, not a bad PR, so it no longer spends
the per-head retry budget: the queue resumes by itself when the budget does.
Telegram now names the cause and the file to change.

Tests use the pane captured verbatim off the run that lost the 40 minutes.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants