From 4f399cec9f84ea85a62231c054fa084c03dcc416 Mon Sep 17 00:00:00 2001 From: Devin Foley Date: Mon, 12 Oct 2026 10:22:43 -0700 Subject: [PATCH] fix(docker): skip the full-tree ownership walk on routine boots (#15992) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The Docker image keeps all instance state under `PAPERCLIP_HOME`, and deployments mount a volume there > - Before it drops privileges, the entrypoint makes sure that the `node` user owns the home (#10530) > - The ownership probe is a first-mismatch `find` over the whole tree. When ownership is correct, `-quit` never fires, so every boot stats every file on the volume > - On homes with many agent worktrees and `node_modules` trees, this walk takes minutes before Node starts. That is close to common 300s healthcheck windows > - This pull request keeps a cheap shallow probe on every boot. It runs the full-tree probe only once per volume and node UID/GID, and records the result in a marker file > - The benefit is that routine boots no longer scale with the file count, and the original root-owned-volume fix stays in place ## Linked Issues or Issue Description Refs #10431 (the open follow-up about the remaining cost of the ownership walk). Related: #10530 (the change that added the full-tree probe). **What happened?** On a hosted multi-instance deployment, we measured the time from container start to the first app log line. Instances with many files on the volume (agent worktrees with `node_modules`) took about 190-235s. Small instances took 5-22s. One 30 GB volume with few files took 24s, so the cost follows the file count, not the volume size. Almost all of this time is the `find "$home_dir" \( ! -user node -o ! -group node \) -print -quit` walk in `scripts/docker-entrypoint.sh`. When ownership is already correct, `-quit` never fires, so the walk stats every inode on every boot. **Expected behavior** When ownership is already correct, the entrypoint pre-start phase is short and does not depend on the number of files in `PAPERCLIP_HOME`. A fresh root-owned volume, a UID/GID remap, and an upgrade from an older image are still repaired before the privilege drop. **Steps to reproduce** 1. Run the image with a volume on `/paperclip` that holds a few hundred thousand files, all owned by `node:node`. 2. Restart the container. 3. Compare the container start time with the first server log line. The gap grows with the file count, even though no `chown` runs. **Paperclip version or commit** `master` at a9830414c7e **Deployment mode** Docker production image (any platform that mounts a volume on `PAPERCLIP_HOME`) ## What Changed - `scripts/docker-entrypoint.sh`: the ownership probe is now tiered. - Every boot runs a shallow probe (`find -maxdepth 3`) over the home and its first three levels. A fresh root-owned mount and a UID/GID remap both change the owner of the home directory, so this probe catches them. - The full-tree probe runs only when `PAPERCLIP_HOME/.paperclip-ownership-verified` is missing or records a different node UID/GID (format `v1 :`). This catches deep mismatches, including the first boot after an upgrade from an older image. - Any mismatch repairs the whole tree with `chown -R node:node`, as before. After a clean probe or a repair, the entrypoint writes the marker as the `node` user (through `gosu`), so the marker never trips the shallow probe. A failed marker write is ignored: the only cost is a full probe on the next boot. If `find` exits non-zero without a mismatch (for example, an I/O error), the walk is incomplete, so the entrypoint does not write the marker and the next boot probes again. - `scripts/docker-entrypoint.sh`: timestamped log lines at entrypoint start, around the full probe and the repair (with durations), and right before the `exec` of the server. You can now measure the pre-start phase from the container log. - `server/src/__tests__/docker-entrypoint.test.ts`: the `find` stub now tells the shallow probe from the full probe. New tests cover each path (see Verification). - `doc/DOCKER.md`: documents the marker, how to force a full check, and the new log lines. ## Verification - `cd server && npx vitest run src/__tests__/docker-entrypoint.test.ts src/__tests__/container-init-reaping.test.ts src/__tests__/docker-build-stamp.test.ts`: 40 passed. New cases: - correct ownership + current marker → shallow probe only, no full walk, no `chown` - current marker + deep-only mismatch → full walk skipped (this is the documented trade-off) - no marker (upgrade) → one full walk, no `chown`, marker written as `v1 1000:1000` - no marker + deep mismatch → full walk finds it, `chown -R`, marker written - root-owned top level with a current marker → `chown -R` without a full walk - full walk fails without a mismatch → no `chown`, no marker, retry logged - marker for another UID/GID → full walk runs again, marker rewritten as `v1 1001:1001` - timestamped `starting`, `ownership repair finished in Ns`, and `starting app as node` lines, in order, before the command runs - The new tests fail against the old script. The 11 existing tests pass unchanged. - `cd server && npx tsc --noEmit`: passes. - `shellcheck -s sh scripts/docker-entrypoint.sh`: clean. `dash -n`: syntax OK. - Manual run with the real `find` (BSD find, script run under `dash`) against a 200k-file tree. Only `id`/`usermod`/`groupmod`/`chown`/`gosu` were stubbed, and `node` was mapped to the local user and group: - boot 1, no marker, one deep group mismatch → full probe, `chown -R`, marker written - boot 2, marker present → no full probe, no `chown` - boot 3, no marker, clean tree → full probe, no `chown`, marker written - boot 4, marker present, clean → no full probe, no `chown` - boot 5, top-level group mismatch with marker → `chown -R` - I did not build the image or measure boot time in a container. Docker was not available on the machine I used. On a warm local disk the full walk is sub-second, so the local timings say nothing about cold network block storage. ## Risks - Once the marker is current, a mismatch deeper than three levels is not detected automatically. Examples: files written as root by `docker exec` or a restore run as root, after the first verified boot. Before this change, those files were repaired on the next boot. The workaround is in `doc/DOCKER.md`: delete the marker and restart. A remap, a fresh volume, and a root-owned top level are still detected on every boot. - On the first boot after an upgrade, each existing volume pays one full walk (the same cost as today). Later boots skip it. - The marker is a new dotfile in `PAPERCLIP_HOME`. Backups and copies that include the home carry it along. This is safe, because the marker records the UID/GID that it verified, and a different UID/GID forces a new full probe. - The non-root path (restricted PodSecurity, OpenShift) does not change, apart from two new stdout log lines. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`, 1M context) via Claude Code, with tool use (shell, file editing). Default reasoning mode. ## 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 - [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 - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --- doc/DOCKER.md | 2 + scripts/docker-entrypoint.sh | 79 ++++++++++- .../src/__tests__/docker-entrypoint.test.ts | 132 +++++++++++++++++- 3 files changed, 199 insertions(+), 14 deletions(-) diff --git a/doc/DOCKER.md b/doc/DOCKER.md index d59ddc102bd..507814c483a 100644 --- a/doc/DOCKER.md +++ b/doc/DOCKER.md @@ -340,6 +340,8 @@ Notes: ## General Notes - The `docker-entrypoint.sh` adjusts the container `node` user UID/GID at startup to match the values passed via `USER_UID`/`USER_GID`, avoiding permission issues on bind-mounted volumes. +- Before it drops privileges, the entrypoint makes sure `PAPERCLIP_HOME` is owned by the `node` user. Each boot checks only the home directory and its first three levels. The first boot (and any boot after a UID/GID change) also checks the full tree, and then writes `PAPERCLIP_HOME/.paperclip-ownership-verified`. While that marker is current, later boots skip the full-tree walk, which can take minutes on homes with many files. A mismatch triggers a recursive `chown`. If files deep in the tree get the wrong owner (for example, after a restore as root), delete the marker and restart to force a full check. If a check fails (for example, with an I/O error), the entrypoint does not write the marker, so the next boot checks again. +- The entrypoint writes timestamped lines when it starts, around the full ownership check or any repair (with the duration), and just before it starts the server, so you can measure the pre-start time from the container log. - Paperclip data persists via Docker volumes/bind mounts (compose) or at `~/.local/share/paperclip` (quadlet). ## Native Runner build cache diff --git a/scripts/docker-entrypoint.sh b/scripts/docker-entrypoint.sh index f97b4f76338..1e723f849dc 100644 --- a/scripts/docker-entrypoint.sh +++ b/scripts/docker-entrypoint.sh @@ -1,6 +1,14 @@ #!/bin/sh set -e +# Timestamped progress lines make the pre-start phase (container start to +# the first app log line) measurable from the container log alone. +log() { + echo "$(date -u +%Y-%m-%dT%H:%M:%SZ) docker-entrypoint.sh: $*" +} + +log "starting" + # Capture runtime UID/GID from environment variables, defaulting to 1000 PUID=${USER_UID:-1000} PGID=${USER_GID:-1000} @@ -15,6 +23,7 @@ if [ "$(id -u)" -ne 0 ]; then if [ "$(id -u)" -ne "$PUID" ] || [ "$(id -g)" -ne "$PGID" ]; then echo "docker-entrypoint.sh: running unprivileged as $(id -u):$(id -g); cannot remap to requested ${PUID}:${PGID}" >&2 fi + log "starting app as $(id -u):$(id -g)" exec "$@" fi @@ -35,14 +44,70 @@ fi # (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. +# server crashed on its first mkdir. +# +# A full-tree probe costs one stat per file. On a home with many agent +# worktrees and node_modules trees that is minutes of boot time, so the +# probe is tiered: +# 1. A shallow probe (the home and its first few levels) runs on every +# boot. It catches a fresh root-owned mount and a UID/GID remap, because +# both change the owner of the home directory itself. +# 2. A full-tree probe runs only when the marker file below is missing or +# records a different node UID/GID. It catches a mismatch deeper in the +# tree (init containers, backup restores, files written before a remap, +# the first boot after an upgrade from an older image). +# Any mismatch repairs the whole tree. After a clean probe or a repair, the +# marker records the verified node UID/GID, so later boots skip the full +# walk. Delete the marker to force a full probe on the next boot. 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" +ownership_marker="$home_dir/.paperclip-ownership-verified" +ownership_shallow_depth=3 + +# Sets needs_repair=1 when any entry is not node:node, and probe_failed=1 +# when find exits non-zero (for example, an I/O error), so an incomplete +# walk is never recorded as verified. The arguments bound the walk (empty +# for the full tree); -quit stops at the first mismatch. +probe_ownership() { + if ! first_mismatch="$(find "$home_dir" "$@" \( ! -user node -o ! -group node \) -print -quit 2>/dev/null)"; then + probe_failed=1 + log "ownership probe of $home_dir failed; it will run again on the next boot" + fi + if [ -n "$first_mismatch" ]; then + needs_repair=1 + fi +} + +if [ -d "$home_dir" ]; then + expected_marker="v1 $(id -u node):$(id -g node)" + current_marker="$(cat "$ownership_marker" 2>/dev/null || true)" + needs_repair=0 + probe_failed=0 + probe_ownership -maxdepth "$ownership_shallow_depth" + if [ "$needs_repair" -eq 0 ] && [ "$current_marker" != "$expected_marker" ]; then + log "full ownership probe of $home_dir (no current marker)" + probe_started=$(date +%s) + probe_ownership + log "full ownership probe finished in $(( $(date +%s) - probe_started ))s" + fi + + if [ "$needs_repair" -eq 1 ]; then + log "repairing ownership of $home_dir to node:node" + repair_started=$(date +%s) + chown -R node:node "$home_dir" + log "ownership repair finished in $(( $(date +%s) - repair_started ))s" + fi + + # Record the marker only when the tree is known good: a successful + # recursive chown (set -e stops the script if it fails), or probes that + # all completed without a mismatch. + if [ "$current_marker" != "$expected_marker" ] && { [ "$needs_repair" -eq 1 ] || [ "$probe_failed" -eq 0 ]; }; then + # Write as node so the marker itself never trips the shallow probe + # (root-squashed network mounts included). Best effort: a read-only + # home only costs a full probe on the next boot. + # shellcheck disable=SC2016 # $1/$2 expand in the inner shell + gosu node sh -c 'printf "%s\n" "$1" > "$2"' sh "$expected_marker" "$ownership_marker" 2>/dev/null || true + fi fi +log "starting app as node" exec gosu node "$@" diff --git a/server/src/__tests__/docker-entrypoint.test.ts b/server/src/__tests__/docker-entrypoint.test.ts index 03d0e4acb93..f71586f8094 100644 --- a/server/src/__tests__/docker-entrypoint.test.ts +++ b/server/src/__tests__/docker-entrypoint.test.ts @@ -32,7 +32,20 @@ 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; homeMismatch?: boolean }) { +type StubOptions = { + uid: number; + gid: number; + nodeUid?: number; + nodeGid?: number; + /** Mismatch anywhere: both the shallow and the full-tree probe find it. */ + homeMismatch?: boolean; + /** Mismatch only below the shallow probe depth: just the full-tree probe finds it. */ + deepMismatch?: boolean; + /** The full-tree find exits non-zero without printing a mismatch (e.g. an I/O error). */ + fullProbeFails?: boolean; +}; + +function installStubs(ids: StubOptions) { writeStub( "id", [ @@ -46,12 +59,21 @@ 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`); + // The entrypoint's ownership probes are first-mismatch finds over the app + // home: a shallow one bounded by -maxdepth, and a full-tree one. An empty + // result models a node-owned tree (image-baked dir, healthy volume); a + // path models a uid OR gid mismatch (fresh root-owned mount, root-owned + // descendant, stale group after a GID-only remap). Each call is logged as + // "find-shallow" or "find-full" so tests can assert which walks ran. + writeStub( + "find", + [ + `case " $* " in`, + ` *" -maxdepth "*) echo "find-shallow $1" >> "${logFile}"; ${ids.homeMismatch ? `echo "$1/mismatched-entry"` : "true"} ;;`, + ` *) echo "find-full $1" >> "${logFile}"; ${ids.homeMismatch || ids.deepMismatch ? `echo "$1/deep/mismatched-entry"` : ids.fullProbeFails ? "exit 1" : "true"} ;;`, + `esac`, + ].join("\n"), + ); writeStub("gosu", `echo "gosu $*" >> "${logFile}"\nshift\nexec "$@"`); } @@ -151,6 +173,102 @@ describe("docker-entrypoint.sh", () => { expect(calls).toContain(`chown -R node:node ${stubDir}`); }); + describe("ownership probe cost", () => { + const marker = () => join(stubDir, ".paperclip-ownership-verified"); + + it("skips the full-tree walk when ownership is correct and the marker is current", async () => { + installStubs({ uid: 0, gid: 0 }); + writeFileSync(marker(), "v1 1000:1000\n"); + + const { stdout, calls } = await runEntrypoint({ PAPERCLIP_HOME: stubDir }); + + expect(calls).toContain(`find-shallow ${stubDir}`); + expect(calls).not.toContain("find-full"); + expect(calls).not.toContain("chown"); + expect(stdout).toContain("ENTRYPOINT-CMD-RAN"); + }); + + it("skips the full-tree walk when the marker is current, even with a deep mismatch", async () => { + // The marker is the trade-off that keeps boots O(1): once a tree has + // been verified, only the shallow levels are re-checked. + installStubs({ uid: 0, gid: 0, deepMismatch: true }); + writeFileSync(marker(), "v1 1000:1000\n"); + + const { calls } = await runEntrypoint({ PAPERCLIP_HOME: stubDir }); + + expect(calls).not.toContain("find-full"); + expect(calls).not.toContain("chown"); + }); + + it("runs one full-tree walk without a marker, then records the marker", async () => { + // First boot after upgrading from an image without the marker. + installStubs({ uid: 0, gid: 0 }); + + const { calls } = await runEntrypoint({ PAPERCLIP_HOME: stubDir }); + + expect(calls).toContain(`find-shallow ${stubDir}`); + expect(calls).toContain(`find-full ${stubDir}`); + expect(calls).not.toContain("chown"); + expect(readFileSync(marker(), "utf8")).toBe("v1 1000:1000\n"); + }); + + it("does not record the marker when the full-tree walk fails without finding a mismatch", async () => { + installStubs({ uid: 0, gid: 0, fullProbeFails: true }); + + const { stdout, calls } = await runEntrypoint({ PAPERCLIP_HOME: stubDir }); + + expect(calls).toContain(`find-full ${stubDir}`); + expect(calls).not.toContain("chown"); + expect(existsSync(marker())).toBe(false); + expect(stdout).toContain("failed; it will run again on the next boot"); + expect(stdout).toContain("ENTRYPOINT-CMD-RAN"); + }); + + it("repairs a deep mismatch found by the full-tree walk when there is no marker", async () => { + installStubs({ uid: 0, gid: 0, deepMismatch: true }); + + const { calls } = await runEntrypoint({ PAPERCLIP_HOME: stubDir }); + + expect(calls).toContain(`find-full ${stubDir}`); + expect(calls).toContain(`chown -R node:node ${stubDir}`); + expect(readFileSync(marker(), "utf8")).toBe("v1 1000:1000\n"); + }); + + it("repairs a root-owned top level without a full-tree probe, even with a current marker", async () => { + installStubs({ uid: 0, gid: 0, homeMismatch: true }); + writeFileSync(marker(), "v1 1000:1000\n"); + + const { calls } = await runEntrypoint({ PAPERCLIP_HOME: stubDir }); + + expect(calls).toContain(`find-shallow ${stubDir}`); + expect(calls).not.toContain("find-full"); + expect(calls).toContain(`chown -R node:node ${stubDir}`); + }); + + it("re-runs the full-tree walk when the marker was written for another UID/GID", async () => { + installStubs({ uid: 0, gid: 0, nodeUid: 1001, nodeGid: 1001, deepMismatch: true }); + writeFileSync(marker(), "v1 1000:1000\n"); + + const { calls } = await runEntrypoint({ USER_UID: "1001", USER_GID: "1001", PAPERCLIP_HOME: stubDir }); + + expect(calls).toContain(`find-full ${stubDir}`); + expect(calls).toContain(`chown -R node:node ${stubDir}`); + expect(readFileSync(marker(), "utf8")).toBe("v1 1001:1001\n"); + }); + + it("logs timestamped start, repair duration, and pre-exec lines", async () => { + installStubs({ uid: 0, gid: 0, homeMismatch: true }); + + const { stdout } = await runEntrypoint({ PAPERCLIP_HOME: stubDir }); + const stamp = "\\d{4}-\\d{2}-\\d{2}T\\d{2}:\\d{2}:\\d{2}Z docker-entrypoint\\.sh: "; + + expect(stdout).toMatch(new RegExp(`^${stamp}starting$`, "m")); + expect(stdout).toMatch(new RegExp(`^${stamp}ownership repair finished in \\d+s$`, "m")); + expect(stdout).toMatch(new RegExp(`^${stamp}starting app as node$`, "m")); + expect(stdout.indexOf("starting app as node")).toBeLessThan(stdout.indexOf("ENTRYPOINT-CMD-RAN")); + }); + }); + it("execs directly and silently when already running as the requested user (restricted PodSecurity)", async () => { installStubs({ uid: 1000, gid: 1000 });