Ensure app-home ownership before the privilege drop, not only on remap (#10530)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The Docker image persists all instance state (project checkouts, worktrees, run logs, uploads) under `PAPERCLIP_HOME`, and deployments mount a volume there for durability > - The entrypoint starts as root and drops privileges to the `node` user, but it fixes `PAPERCLIP_HOME` ownership only when it remaps the user's UID/GID > - A freshly mounted volume arrives root-owned and shadows the image's build-time `chown`, so a default-UID boot drops privileges onto an unwritable home and the server crashes on its first `mkdir` > - This pull request makes the entrypoint probe the home's ownership and chown whenever it does not match the runtime user, before the privilege drop > - The benefit is that the image works out of the box on any platform-managed volume, with the common already-correct boot staying chown-free ## Linked Issues or Issue Description No public issue exists — describing the bug inline (per the bug report template). **What happened?** Running the image with a freshly created volume mounted at `/paperclip` (a Docker named volume, a Kubernetes PV, or any platform-managed volume) and the default `USER_UID`/`USER_GID` crashes on boot: `Error: EACCES: permission denied, mkdir '/paperclip/instances/default/logs'`. **Expected behavior** The container boots and initializes its instance tree on the mounted volume, exactly as it does when `/paperclip` is the image's own (build-time chowned) directory. **Steps to reproduce** 1. `docker volume create paperclip-data` 2. `docker run -v paperclip-data:/paperclip ghcr.io/paperclipai/paperclip:<any current tag>` 3. Observe the EACCES crash on the first `mkdir` under `/paperclip`. **Root cause** `scripts/docker-entrypoint.sh` chowns `/paperclip` only inside its UID/GID remap branch (`changed=1`). A fresh volume mount is root-owned and shadows the image's build-time `chown node:node /paperclip`; with the default 1000:1000 no remap happens, so no chown happens, and `gosu node` drops onto an unwritable home. **Paperclip version or commit:** reproduces on `master` and any published image. **Deployment mode:** any; observed on managed-cloud volume mounts and reproducible with plain Docker named volumes. **Installation method:** Docker image (`ghcr.io/paperclipai/paperclip`). **Related PRs (dedup search):** no open or merged PR touches the entrypoint ownership logic; the entrypoint's privilege-handling tests were added previously and this extends them. No duplicate found. ## What Changed - `scripts/docker-entrypoint.sh`: the remap-conditional `chown` is replaced by an ownership probe — after any UID/GID remap, the entrypoint stats `PAPERCLIP_HOME` (default `/paperclip`) and runs `chown -R node:node` only when the owner does not match the runtime user, before `exec gosu node`. Covers fresh root-owned mounts and trees written under a previous UID mapping; the already-correct boot performs no chown. The unprivileged (non-root start) branch is unchanged. - `server/src/__tests__/docker-entrypoint.test.ts`: `stat` stub added to the harness; new cases for the fresh root-owned mount with default UID/GID and for `PAPERCLIP_HOME`-relative probing; the remap case now models the post-remap ownership mismatch. ## Verification - `pnpm vitest run server/src/__tests__/docker-entrypoint.test.ts` — 7 passed (5 existing behaviors unchanged, 2 new). - Live on a managed deployment: a container that crash-looped with the EACCES above boots cleanly once the home is chowned before the drop (the same effect this entrypoint change produces; forced there by a UID remap as an interim workaround). ## Risks - Low. Behavior changes only for boots where `PAPERCLIP_HOME` exists with mismatched ownership — exactly the boots that crash today. `chown -R` on a large previously-mismatched tree adds one-time boot latency; correctly-owned homes skip it entirely. Kubernetes restricted / OpenShift non-root starts keep the existing exec-directly path untouched. ## Model Used Claude Fable 5 (`claude-fable-5`, Anthropic; Claude Code CLI with extended thinking and tool use; tests executed locally via Vitest). ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above (no duplicates; extends the existing entrypoint privilege tests) - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge
This commit is contained in:
parent
075951f6bd
commit
dd1a7f5290
|
|
@ -19,24 +19,30 @@ if [ "$(id -u)" -ne 0 ]; then
|
|||
fi
|
||||
|
||||
# Adjust the node user's UID/GID if they differ from the runtime request
|
||||
# and fix volume ownership only when a remap is needed
|
||||
changed=0
|
||||
|
||||
if [ "$(id -u node)" -ne "$PUID" ]; then
|
||||
echo "Updating node UID to $PUID"
|
||||
usermod -o -u "$PUID" node
|
||||
changed=1
|
||||
fi
|
||||
|
||||
if [ "$(id -g node)" -ne "$PGID" ]; then
|
||||
echo "Updating node GID to $PGID"
|
||||
groupmod -o -g "$PGID" node
|
||||
usermod -g "$PGID" node
|
||||
changed=1
|
||||
fi
|
||||
|
||||
if [ "$changed" = "1" ]; then
|
||||
chown -R node:node /paperclip
|
||||
# Ensure the app home is owned by the runtime user BEFORE dropping
|
||||
# privileges -- not only after a UID/GID remap. A freshly mounted volume
|
||||
# (Docker named volume, Railway volume, Kubernetes PV) arrives root-owned
|
||||
# and shadows the image's build-time chown, so with the default UID the old
|
||||
# remap-only condition dropped privileges onto an unwritable home and the
|
||||
# server crashed on its first mkdir. The probe is a first-mismatch find
|
||||
# over the WHOLE tree (uid and gid): a root-owned mount or descendant
|
||||
# (init containers, backup restores, files written before a remap) is
|
||||
# found immediately and repaired recursively, a GID-only remap is caught,
|
||||
# and a fully-correct tree costs one metadata-only walk with no chown.
|
||||
home_dir="${PAPERCLIP_HOME:-/paperclip}"
|
||||
if [ -d "$home_dir" ] && [ -n "$(find "$home_dir" \( ! -user node -o ! -group node \) -print -quit 2>/dev/null)" ]; then
|
||||
chown -R node:node "$home_dir"
|
||||
fi
|
||||
|
||||
exec gosu node "$@"
|
||||
|
|
|
|||
|
|
@ -32,7 +32,7 @@ function writeStub(name: string, body: string) {
|
|||
writeFileSync(path, `#!/bin/sh\n${body}\n`, { mode: 0o755 });
|
||||
}
|
||||
|
||||
function installStubs(ids: { uid: number; gid: number; nodeUid?: number; nodeGid?: number }) {
|
||||
function installStubs(ids: { uid: number; gid: number; nodeUid?: number; nodeGid?: number; homeMismatch?: boolean }) {
|
||||
writeStub(
|
||||
"id",
|
||||
[
|
||||
|
|
@ -46,6 +46,12 @@ function installStubs(ids: { uid: number; gid: number; nodeUid?: number; nodeGid
|
|||
for (const cmd of ["usermod", "groupmod", "chown"]) {
|
||||
writeStub(cmd, `echo "${cmd} $*" >> "${logFile}"`);
|
||||
}
|
||||
// The entrypoint's ownership probe is a first-mismatch find over the app
|
||||
// home. An empty result models a fully node-owned tree (image-baked dir,
|
||||
// healthy volume); a path models any uid OR gid mismatch anywhere in the
|
||||
// tree (fresh root-owned mount, root-owned descendant, stale group after
|
||||
// a GID-only remap).
|
||||
writeStub("find", ids.homeMismatch ? `echo "$1/mismatched-entry"` : `true`);
|
||||
writeStub("gosu", `echo "gosu $*" >> "${logFile}"\nshift\nexec "$@"`);
|
||||
}
|
||||
|
||||
|
|
@ -79,17 +85,60 @@ describe("docker-entrypoint.sh", () => {
|
|||
});
|
||||
|
||||
it("remaps the node user and chowns /paperclip before gosu when root requests a different UID/GID", async () => {
|
||||
installStubs({ uid: 0, gid: 0 });
|
||||
// The stubbed node uid stays 1000 while the stat probe reports the old
|
||||
// ownership, modelling the post-remap mismatch that must trigger chown.
|
||||
installStubs({ uid: 0, gid: 0, homeMismatch: true });
|
||||
|
||||
const { stdout, calls } = await runEntrypoint({ USER_UID: "1001", USER_GID: "1001" });
|
||||
const { stdout, calls } = await runEntrypoint({ USER_UID: "1001", USER_GID: "1001", PAPERCLIP_HOME: stubDir });
|
||||
|
||||
expect(stdout).toContain("ENTRYPOINT-CMD-RAN");
|
||||
expect(calls).toContain("usermod -o -u 1001 node");
|
||||
expect(calls).toContain("groupmod -o -g 1001 node");
|
||||
expect(calls).toContain("chown -R node:node /paperclip");
|
||||
expect(calls).toContain(`chown -R node:node ${stubDir}`);
|
||||
expect(calls).toContain("gosu node echo ENTRYPOINT-CMD-RAN");
|
||||
});
|
||||
|
||||
it("chowns a root-owned home before gosu even with the default UID/GID (fresh volume mount)", async () => {
|
||||
// A freshly mounted volume arrives root-owned and shadows the image's
|
||||
// build-time chown; with no remap requested the old entrypoint dropped
|
||||
// privileges onto an unwritable home and the server crashed on its
|
||||
// first mkdir.
|
||||
installStubs({ uid: 0, gid: 0, homeMismatch: true });
|
||||
|
||||
const { stdout, calls } = await runEntrypoint({ PAPERCLIP_HOME: stubDir });
|
||||
|
||||
expect(stdout).toContain("ENTRYPOINT-CMD-RAN");
|
||||
expect(calls).toContain(`chown -R node:node ${stubDir}`);
|
||||
expect(calls).not.toContain("usermod");
|
||||
expect(calls).toContain("gosu node echo ENTRYPOINT-CMD-RAN");
|
||||
});
|
||||
|
||||
it("repairs ownership on a GID-only remap (stale group on persisted descendants)", async () => {
|
||||
installStubs({ uid: 0, gid: 0, homeMismatch: true });
|
||||
|
||||
const { calls } = await runEntrypoint({ USER_GID: "1001", PAPERCLIP_HOME: stubDir });
|
||||
|
||||
expect(calls).toContain("groupmod -o -g 1001 node");
|
||||
expect(calls).toContain(`chown -R node:node ${stubDir}`);
|
||||
});
|
||||
|
||||
it("keeps a fully node-owned tree chown-free (no per-boot recursive chown)", async () => {
|
||||
installStubs({ uid: 0, gid: 0, homeMismatch: false });
|
||||
|
||||
const { calls } = await runEntrypoint({ PAPERCLIP_HOME: stubDir });
|
||||
|
||||
expect(calls).not.toContain("chown");
|
||||
expect(calls).toContain("gosu node echo ENTRYPOINT-CMD-RAN");
|
||||
});
|
||||
|
||||
it("honours PAPERCLIP_HOME for the ownership probe", async () => {
|
||||
installStubs({ uid: 0, gid: 0, homeMismatch: true });
|
||||
|
||||
const { calls } = await runEntrypoint({ PAPERCLIP_HOME: stubDir });
|
||||
|
||||
expect(calls).toContain(`chown -R node:node ${stubDir}`);
|
||||
});
|
||||
|
||||
it("execs directly and silently when already running as the requested user (restricted PodSecurity)", async () => {
|
||||
installStubs({ uid: 1000, gid: 1000 });
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue