From 02fc853af23b87e7903d25b1cceb691d86947176 Mon Sep 17 00:00:00 2001 From: Gardener Date: Sun, 20 Sep 2026 19:59:41 +0000 Subject: [PATCH] fix(skills-catalog): apply harness containment checks to the allocated path The default form allocated under TMPDIR without the containment checks the explicit form used, so a TMPDIR pointing into the App checkout or the corpus would build the harness tree inside a source tree. Both branches now call one `assert_outside` helper. The temp root is checked before `mktemp` runs, so a refusal leaves nothing behind, and `pwd -P` resolves it first so a symlinked TMPDIR cannot slip past. Equality with the checkout, not only containment inside it, is refused. Executed: TMPDIR set to the checkout, to the corpus, to a symlink pointing at the checkout, and to a path that does not exist. All four exit 2 and create nothing. The seven earlier refusals still refuse, and the normal flow still runs end to end in both forms. Co-Authored-By: Paperclip --- .../references/offline-verification.md | 47 ++++++++++++++++--- .../skills-catalog/generated/catalog.json | 8 ++-- 2 files changed, 44 insertions(+), 11 deletions(-) diff --git a/packages/skills-catalog/catalog/optional/software-development/author-connector-definition/references/offline-verification.md b/packages/skills-catalog/catalog/optional/software-development/author-connector-definition/references/offline-verification.md index 8d1083618f2..0c34b020344 100644 --- a/packages/skills-catalog/catalog/optional/software-development/author-connector-definition/references/offline-verification.md +++ b/packages/skills-catalog/catalog/optional/software-development/author-connector-definition/references/offline-verification.md @@ -43,13 +43,13 @@ The rules it follows: | Input | What happens | | --- | --- | -| No `out-dir` given | `mktemp -d` allocates a fresh unique directory. | +| No `out-dir` given | `mktemp -d` allocates a fresh unique directory under `TMPDIR`. The temp root is checked for containment *before* `mktemp` runs, so a refusal leaves nothing behind. | | A relative path | Refused. It resolves against the current directory, which is too easy to get wrong. | | `/` | Refused. | | A path that already exists — file or directory, empty or not | Refused. Pass a new path or omit the argument. | | A symlink | Refused before anything is read or written through it. | | A path whose parent does not exist | Refused. The script creates one level, never a tree. | -| A path inside the App checkout or the corpus | Refused after resolving the parent with `pwd -P`, so `..` and symlinked parents cannot slip past. | +| A path inside the App checkout or the corpus | Refused after resolving with `pwd -P`, so `..` and symlinked parents cannot slip past. The same check applies to `TMPDIR`, so the allocated form cannot land in a source tree either. | | Anything else | `mkdir` creates it — not `mkdir -p`, so a race that creates it first is an error, not a silent reuse. | Refusing every existing destination is what makes this safe, and it is why the @@ -96,11 +96,25 @@ die() { printf 'make-harness: %s\n' "$*" >&2; exit 2; } APP_REPO_REAL=$(cd "$APP_REPO" && pwd -P) CORPUS_REAL=$(cd "$CORPUS" && pwd -P) +# Applied to both branches below. An allocated TMPDIR can sit inside the +# checkout just as easily as a path somebody typed. +assert_outside() { + case $1/ in + "$APP_REPO_REAL"/|"$APP_REPO_REAL"/*) die "$2 is inside the App checkout: $1" ;; + "$CORPUS_REAL"/|"$CORPUS_REAL"/*) die "$2 is inside the corpus: $1" ;; + esac +} + # --- allocate the output directory ------------------------------------------- # Every branch below either creates a new directory or refuses. There is no # path through this block that removes anything. if [ -z "$OUT" ]; then - OUT=$(mktemp -d "${TMPDIR:-/tmp}/paperclip-harness.XXXXXXXX") + # Check the temp root *before* mktemp, so a refusal leaves nothing behind. + TMPROOT=${TMPDIR:-/tmp} + [ -d "$TMPROOT" ] || die "TMPDIR does not exist: $TMPROOT" + TMPROOT=$(cd "$TMPROOT" && pwd -P) + assert_outside "$TMPROOT" "TMPDIR" + OUT=$(mktemp -d "$TMPROOT/paperclip-harness.XXXXXXXX") echo "allocated a fresh harness directory: $OUT" else case $OUT in @@ -116,10 +130,7 @@ else [ -d "$PARENT" ] || die "parent directory does not exist: $PARENT" # Resolve the parent so `..` and symlinked parents cannot escape the checks. OUT="$(cd "$PARENT" && pwd -P)/$(basename "$OUT")" - case $OUT/ in - "$APP_REPO_REAL"/*) die "out-dir is inside the App checkout: $OUT" ;; - "$CORPUS_REAL"/*) die "out-dir is inside the corpus: $OUT" ;; - esac + assert_outside "$OUT" "out-dir" # mkdir, not `mkdir -p`: if something created the path in the meantime, stop. mkdir "$OUT" || die "could not create $OUT" fi @@ -254,6 +265,28 @@ $ make-harness.sh HEAD /tmp/no/such/parent/out make-harness: parent directory does not exist: /tmp/no/such/parent exit 2 ``` +The allocated form is checked too. Containment is not something only an explicit +destination needs — `TMPDIR` can point into a source tree just as easily. It is +checked *before* `mktemp` runs, so a refusal leaves nothing behind: + +```text +$ TMPDIR= make-harness.sh HEAD +make-harness: TMPDIR is inside the App checkout: exit 2 + +$ TMPDIR= make-harness.sh HEAD +make-harness: TMPDIR is inside the corpus: exit 2 + +$ TMPDIR=/tmp/repo-link make-harness.sh HEAD # → +make-harness: TMPDIR is inside the App checkout: exit 2 + +$ TMPDIR=/tmp/does-not-exist make-harness.sh HEAD +make-harness: TMPDIR does not exist: /tmp/does-not-exist exit 2 +``` + +The third case is the one worth noticing: `TMPDIR` was a **symlink** to the App +checkout, and `pwd -P` resolved it before the comparison. The checkout held the +same number of entries before and after all four runs. + The two helper guards were exercised the same way, by running each from the App checkout instead of its own harness root: diff --git a/packages/skills-catalog/generated/catalog.json b/packages/skills-catalog/generated/catalog.json index 1647651fee9..b27e04f5e6f 100644 --- a/packages/skills-catalog/generated/catalog.json +++ b/packages/skills-catalog/generated/catalog.json @@ -2,7 +2,7 @@ "schemaVersion": 1, "packageName": "@paperclipai/skills-catalog", "packageVersion": "0.3.1", - "generatedAt": "2026-09-20T19:50:57.718Z", + "generatedAt": "2026-09-20T19:59:29.226Z", "skills": [ { "id": "paperclipai:bundled:docs:doc-maintenance", @@ -1165,11 +1165,11 @@ { "path": "references/offline-verification.md", "kind": "reference", - "sizeBytes": 16768, - "sha256": "296f08e6249687255162eb3afee40e51992ae1f5a86677594e63e95f91ebdaf2" + "sizeBytes": 18496, + "sha256": "3b5a73bc50963bc5a1d7044817642022397eb42229f78a75df4d3b3c8a43b0f1" } ], - "contentHash": "sha256:26c08b6917c5bf80dc6b99b62f177746bfafed458905182782aa14652e0c3b84" + "contentHash": "sha256:830e6a1d3d1dfc06372e4bd17883b6f916d5425d7c45b6e53bec8a1c8aa3c2aa" }, { "id": "paperclipai:optional:software-development:connect-agent-tools",