mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-11 13:24:55 +01:00
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 <noreply@paperclip.ing>
This commit is contained in:
1 parent
72d065a535
commit
02fc853af2
2 files changed
+44
-11
No files matched your search
+40
-7
@@ -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 <app-repo> HEAD <corpus> /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=<app-repo> make-harness.sh <app-repo> HEAD <corpus>
|
||||
make-harness: TMPDIR is inside the App checkout: <app-repo> exit 2
|
||||
|
||||
$ TMPDIR=<corpus> make-harness.sh <app-repo> HEAD <corpus>
|
||||
make-harness: TMPDIR is inside the corpus: <corpus> exit 2
|
||||
|
||||
$ TMPDIR=/tmp/repo-link make-harness.sh <app-repo> HEAD <corpus> # → <app-repo>
|
||||
make-harness: TMPDIR is inside the App checkout: <app-repo> exit 2
|
||||
|
||||
$ TMPDIR=/tmp/does-not-exist make-harness.sh <app-repo> HEAD <corpus>
|
||||
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:
|
||||
|
||||
|
||||
@@ -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",
|
||||
|
||||
Reference in new issue
Block a user