Merge pull request #1481 from open-gsd/fix/1460-integrity-confinement
fix(#1460): verify-or-reject capability --integrity per source; confine hook commands to the bundle
This commit is contained in:
Tom Boucher
6
.changeset/fix-1460-integrity-confinement.md
Normal file
6
.changeset/fix-1460-integrity-confinement.md
Normal file
@@ -0,0 +1,6 @@
|
||||
---
|
||||
type: Fixed
|
||||
pr: 1481
|
||||
---
|
||||
|
||||
**Capability `--integrity` is now verified or rejected per source, and hook commands are confined to the bundle** — a supplied `--integrity` pin was silently dropped for npm, git, and local capability sources (only the tarball source verified it), so a user could believe content was pinned when it was not. npm now verifies the pin over the `npm pack` `.tgz` bytes; git and local sources, which have no single hashable artifact, now reject a supplied `--integrity` with an actionable error instead of ignoring it. Separately, a capability hook's relative `script` was written verbatim as the hook command, so it resolved against the working directory (not the capability bundle) at hook-exec time and a crafted relative path could escape the bundle; the command is now resolved against the capability's own install dir and realpath-confined to it, then written as an absolute path. That absolute command is consumed by a shell, which exposed two further problems now fixed: (1) a manifest could ship a file literally named `run.sh; touch /tmp/pwn` (filenames may legally contain `;`, spaces, `$`, backtick, `|`, newline) and declare it as the hook `script`, so the emitted command injected a second shell command even though the file lived inside the bundle — the validator now rejects any hook script path outside a conservative `[A-Za-z0-9._/-]` allowlist (no whitespace, shell metacharacters, leading `-`, absolute path, or `..`), failing the install/load loudly, and the confinement helper mirrors the same rejection defensively; (2) the absolute path begins with the install-home directory, which commonly contains spaces (e.g. `/Users/Bob Smith/.claude/...`) and word-split or broke when written unquoted — the emitted command is now POSIX single-quoted so the install prefix can neither break nor inject. (#1460)
|
||||
@@ -51,7 +51,7 @@ Non-loop lifecycle hooks.
|
||||
| Sub-field | Type | Description |
|
||||
|---|---|---|
|
||||
| `event` | string | Hook event name (host-runtime specific). |
|
||||
| `script` | string | Path to the hook script, relative to the capability root. |
|
||||
| `script` | string | Path to the hook script, **relative** to the capability root. The hook `command` written into the host settings is the realpath-confined **absolute** path to this script (so it always runs the bundle's own file regardless of the working directory) and is POSIX single-quoted (so an install prefix containing spaces cannot break it). For shell safety the path must contain only `[A-Za-z0-9._/-]` — no whitespace, no shell metacharacters (`; \| & $ ` `` ` `` `( ) < > * ? [ ] { } ! ~ # ' " \` newline), no leading `-`, no absolute path, and no `..` segment. A script outside this allowlist fails validation and the capability is rejected. |
|
||||
|
||||
### `config` — federated config-key schema slice
|
||||
|
||||
|
||||
@@ -31,7 +31,7 @@ gsd capability install <spec> [--integrity sha512-<hash>] [--scope global|projec
|
||||
|
||||
| Flag | Type | Default | Description |
|
||||
|---|---|---|---|
|
||||
| `--integrity` | `sha512-<base64>` | — | SHA-512 bundle hash to verify before extraction. When supplied, a mismatch aborts the install. When the source registry or `capability.json` already carries an `integrity` field, both must agree. |
|
||||
| `--integrity` | `sha512-<base64>` | — | SHA-512 hash of the downloaded artifact, verified before extraction. When supplied, a mismatch aborts the install. **Per-source semantics (a supplied value is never silently ignored):** for **tarball** and **npm** sources it is verified over the downloaded artifact bytes (the fetched `.tgz` for tarball; the `npm pack` `.tgz` for npm — both the same SRI sha512 domain); for **git** and **local** sources there is no single downloadable artifact to hash, so a supplied `--integrity` is **rejected** with an actionable error (git: pin the commit with `#sha:<commit>` instead; local: not supported). When the source registry or `capability.json` already carries an `integrity` field, both must agree. |
|
||||
| `--scope` | `global` \| `project` | `global` | Installation root (see [Install layout](#install-layout)). |
|
||||
| `--yes` | flag | off | Grant consent for the capability's executable surfaces non-interactively. The disclosure is still printed. Without it, an install that declares executable surfaces is **aborted** after printing the disclosure (the CLI is non-interactive — there is no prompt to answer). |
|
||||
| `--shared-file` | path (repeatable) | — | A file, **relative to the scope root**, into which the capability's disclosed hooks / MCP servers should be spliced (e.g. a runtime's `settings.json`). Each fragment is marker-isolated so `remove` can strip exactly it. When omitted, the bundle still installs (declaratively); no shared-file edits are made. |
|
||||
@@ -241,13 +241,13 @@ These appear in ADR-1244's command surface but are **not implemented in 1.6.0**.
|
||||
|
||||
The `install` subcommand accepts the following source specification forms.
|
||||
|
||||
| Form | Example | Adapter |
|
||||
|---|---|---|
|
||||
| Registry name | `my-cap@gsd-registry` | Registry — fetches the capability bundle from the named registry; `integrity` is populated from the registry catalogue. |
|
||||
| Git URL with tag | `https://github.com/org/repo.git#v1.2.0` | Git — clones/fetches at the specified tag; `#sha:<40-hex>` pins a specific commit. |
|
||||
| npm package | `npm:@org/gsd-capability-foo@^1.0.0` | npm — resolves via `npm dist-tags` / semver range; installs with `--ignore-scripts`. |
|
||||
| Tarball URL | `https://host/path/cap-x.y.z.tgz` | Tarball — fetches over HTTPS, verifies SHA-512 when `--integrity` is supplied. |
|
||||
| Local path | `./local/path` (or an absolute path) | Local — copies from the filesystem path. Auto-update detection is not available for this form. |
|
||||
| Form | Example | Adapter | `--integrity` |
|
||||
|---|---|---|---|
|
||||
| Registry name | `my-cap@gsd-registry` | Registry — fetches the capability bundle from the named registry; `integrity` is populated from the registry catalogue. | Verified over the fetched bundle. |
|
||||
| Git URL with tag | `https://github.com/org/repo.git#v1.2.0` | Git — clones/fetches at the specified tag; `#sha:<40-hex>` pins a specific commit. | **Rejected** — a clone is a directory tree, not a single hashable artifact. Pin the commit with `#sha:<commit>` instead. |
|
||||
| npm package | `npm:@org/gsd-capability-foo@^1.0.0` | npm — resolves via `npm dist-tags` / semver range; installs with `--ignore-scripts`. | Verified over the `npm pack` `.tgz` bytes (same SRI sha512 domain as a tarball). |
|
||||
| Tarball URL | `https://host/path/cap-x.y.z.tgz` | Tarball — fetches over HTTPS. | Verified over the downloaded `.tgz` bytes. |
|
||||
| Local path | `./local/path` (or an absolute path) | Local — copies from the filesystem path. Auto-update detection is not available for this form. | **Rejected** — a local directory has no single hashable artifact; integrity pinning is not supported for local sources. |
|
||||
|
||||
Which source forms are *permitted* is governed by the `capabilities.strict_known_registries` policy (see [Configuration](../CONFIGURATION.md) and [the capability trust model](../explanation/capability-trust-model.md)): `null`/absent is permissive, `[]` is lockdown (no third-party sources), and a host allowlist permits only matching registries. This policy is **project-scoped** — it is read from the current project's `.planning/config.json` and applied to installs run in that project regardless of `--scope`; there is no machine-wide source allowlist. (A present-but-unparseable config fails **closed** — external installs are blocked until it is fixed.)
|
||||
|
||||
|
||||
@@ -195,6 +195,36 @@ const SEMVER_RANGE_RE = /^[0-9A-Za-z.\-+ |<>=~^*()]+$/;
|
||||
// chars + "==" padding). Exact length so malformed pins ("sha512-abc") fail.
|
||||
const SHA512_INTEGRITY_RE = /^sha512-[A-Za-z0-9+/]{86}==$/;
|
||||
|
||||
// #1460 (R) HIGH — shell-safe hook-script allowlist. A hook `script` is resolved to an
|
||||
// ABSOLUTE path and written verbatim as the hook `command` STRING in settings.json, which a
|
||||
// host runtime consumes through a shell (first-party hooks emit `node "${...}/hooks/x.js"`).
|
||||
// A manifest-controlled name like `run.sh; touch /tmp/pwn` (filenames may legally contain
|
||||
// `;`, spaces, `$`, backtick, `|`, newline on POSIX) would inject a second command — even
|
||||
// though the file genuinely exists inside the bundle and so passes path-confinement. We
|
||||
// therefore restrict the relative script path to a CONSERVATIVE allowlist: only
|
||||
// [A-Za-z0-9._/-], with no leading `-` on any segment (option-injection), no `..` segment,
|
||||
// and not absolute. Anything else (whitespace, any shell metacharacter, control/NUL) is a
|
||||
// hard validation error — fail closed so the capability install/load is rejected loudly.
|
||||
const SAFE_HOOK_SCRIPT_RE = /^[A-Za-z0-9._/-]+$/;
|
||||
|
||||
/**
|
||||
* #1460 (R): true when a relative hook-script path is shell-safe (see SAFE_HOOK_SCRIPT_RE).
|
||||
* Rejects absolute paths, `..` segments, a leading `-` on any path segment, and any char
|
||||
* outside the allowlist (whitespace / shell metacharacters / control / NUL).
|
||||
*/
|
||||
function isSafeHookScriptPath(script) {
|
||||
if (typeof script !== 'string' || script.length === 0) return false;
|
||||
if (!SAFE_HOOK_SCRIPT_RE.test(script)) return false;
|
||||
if (path.isAbsolute(script)) return false;
|
||||
const segments = script.split(/[/\\]/);
|
||||
if (segments.includes('..')) return false;
|
||||
// A leading '-' on any segment would be parsed as an option by the shell/`node`.
|
||||
for (const seg of segments) {
|
||||
if (seg.startsWith('-')) return false;
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
// A syntactically plausible semver range (shape-only — see SEMVER_RANGE_RE).
|
||||
// Requires a digit or a bare wildcard so pure-alpha garbage ("abcx", "()x") is
|
||||
// rejected; full range satisfaction is the runtime overlay's job (ADR-1244 D2).
|
||||
@@ -542,6 +572,15 @@ function validateFeatureBody(cap) {
|
||||
}
|
||||
if (typeof h.script !== 'string' || h.script.length === 0) {
|
||||
errors.push('hooks[' + i + '].script must be a non-empty string');
|
||||
} else if (!isSafeHookScriptPath(h.script)) {
|
||||
// #1460 (R) HIGH: the script becomes an absolute hook `command` consumed by a shell;
|
||||
// reject any unsafe character (shell metacharacters/whitespace/control), a leading `-`,
|
||||
// an absolute path, or a `..` segment so a manifest can never inject a second command.
|
||||
errors.push(
|
||||
'hooks[' + i + '].script must be a relative path containing only [A-Za-z0-9._/-] ' +
|
||||
'(no whitespace, shell metacharacters (e.g. ; | & $ ` ( ) < > * ? newline), leading "-", ' +
|
||||
'absolute path, or ".." segment) — it contains unsafe characters: ' + JSON.stringify(h.script),
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -375,6 +375,88 @@ function confinedSharedFile(runtimeDir: string, relFile: unknown): string | null
|
||||
return path.join(realParent, path.basename(target));
|
||||
}
|
||||
|
||||
// #1460 (R) HIGH — shell-safe hook-script allowlist (mirrors capability-validator.cjs
|
||||
// isSafeHookScriptPath; see confinedBundleScript for why). Only [A-Za-z0-9._/-], no leading
|
||||
// `-` segment, no `..`, not absolute.
|
||||
const SAFE_HOOK_SCRIPT_RE = /^[A-Za-z0-9._/-]+$/;
|
||||
function isSafeHookScriptPath(script: string): boolean {
|
||||
if (typeof script !== 'string' || script.length === 0) return false;
|
||||
if (!SAFE_HOOK_SCRIPT_RE.test(script)) return false;
|
||||
if (path.isAbsolute(script)) return false;
|
||||
const segments = script.split(/[/\\]/);
|
||||
if (segments.includes('..')) return false;
|
||||
for (const seg of segments) {
|
||||
if (seg.startsWith('-')) return false;
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
/**
|
||||
* #1460 (R) HIGH: POSIX single-quote an arbitrary string for safe inclusion in a shell command.
|
||||
* The emitted hook `command` is the ABSOLUTE confined script path, which begins with the
|
||||
* (non-manifest) install-prefix — commonly a home dir containing spaces/special chars (e.g.
|
||||
* "/Users/Bob Smith/.claude/..."). Written unquoted it would word-split (and, with a hostile
|
||||
* prefix, could inject). Wrapping in single quotes — with each embedded `'` escaped as `'\''` —
|
||||
* makes the whole path a single shell token that no metacharacter inside it can break.
|
||||
*/
|
||||
function shellSingleQuote(value: string): string {
|
||||
return "'" + value.replace(/'/g, "'\\''") + "'";
|
||||
}
|
||||
|
||||
/**
|
||||
* #1460 CONF-1: resolve a hook `script` (declared RELATIVE to the bundle) against the capability's
|
||||
* own install dir and CONFINE it via realpath, returning the ABSOLUTE confined path or null when it
|
||||
* escapes the bundle. Mirrors confinedSharedFile (realpath the FULL existing ancestor chain so an
|
||||
* ancestor symlink at any depth cannot escape) and capability-validator's materializeHookFragments
|
||||
* (resolve-against-capDir containment), but rooted at capDir rather than runtimeDir.
|
||||
*
|
||||
* Why this matters: the prior code wrote the RAW relative `script` as the hook command. At hook-exec
|
||||
* time a relative command resolves against the CWD, not the bundle — so it could execute an arbitrary
|
||||
* file, and a crafted relative path (or a symlinked subdir) could escape the bundle. Writing the
|
||||
* absolute confined path makes the hook always run the bundle's own file regardless of CWD.
|
||||
*/
|
||||
function confinedBundleScript(capDirPath: string, script: string): string | null {
|
||||
// Absolute paths and `..` segments are invalid script inputs (and rejected by the caller too).
|
||||
if (path.isAbsolute(script) || script.split(/[/\\]/).includes('..')) return null;
|
||||
|
||||
// #1460 (R) HIGH (defense-in-depth): the confined ABSOLUTE path is written verbatim as a hook
|
||||
// `command` string that a host runtime consumes through a shell. A manifest-controlled script
|
||||
// name containing a shell metacharacter / whitespace / control char / leading "-" would inject a
|
||||
// second command — even though the file genuinely exists inside the bundle and so passes the
|
||||
// realpath confinement below. The validator already rejects such scripts at install/load time
|
||||
// (capability-validator.cjs isSafeHookScriptPath); we MIRROR the same conservative allowlist here
|
||||
// so applyCapabilitySharedEdits skips an unsafe script even if validation were somehow bypassed.
|
||||
if (!isSafeHookScriptPath(script)) return null;
|
||||
|
||||
let realCapRoot: string;
|
||||
try {
|
||||
realCapRoot = fs.realpathSync(capDirPath);
|
||||
} catch {
|
||||
// capDir does not exist yet (e.g. applyCapabilitySharedEdits called before the bundle is on
|
||||
// disk): a non-existent root cannot be a symlink escaping itself, so confine lexically.
|
||||
realCapRoot = path.resolve(capDirPath);
|
||||
const targetLex = path.resolve(realCapRoot, script);
|
||||
if (targetLex !== realCapRoot && !targetLex.startsWith(realCapRoot + path.sep)) return null;
|
||||
return targetLex;
|
||||
}
|
||||
|
||||
const target = path.resolve(realCapRoot, script);
|
||||
const parentDir = path.dirname(target);
|
||||
let realParent: string;
|
||||
try {
|
||||
realParent = fs.realpathSync(parentDir);
|
||||
} catch {
|
||||
// Parent does not exist yet (created inside the bundle): lexical containment is sufficient
|
||||
// because a non-existent path cannot be a symlink escaping the root.
|
||||
if (parentDir !== realCapRoot && !parentDir.startsWith(realCapRoot + path.sep)) return null;
|
||||
return target;
|
||||
}
|
||||
// The realpath'd parent chain must remain inside the bundle — an ancestor symlink escaping the
|
||||
// bundle is refused here (the symlink is followed by realpathSync, so its real location is checked).
|
||||
if (realParent !== realCapRoot && !realParent.startsWith(realCapRoot + path.sep)) return null;
|
||||
return path.join(realParent, path.basename(target));
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Atomic directory promotion (stage -> swap, backup retained for the caller)
|
||||
// ---------------------------------------------------------------------------
|
||||
@@ -516,11 +598,21 @@ function applyCapabilitySharedEdits(args: {
|
||||
const event = typeof rec['event'] === 'string' ? rec['event'] : '';
|
||||
const script = typeof rec['script'] === 'string' ? rec['script'] : '';
|
||||
if (!event || !script || isUnsafeKey(event)) continue;
|
||||
// Never write a hook command pointing outside the capability's own bundle (the trust gate
|
||||
// already blocks such manifests at install; this is defense-in-depth for any other caller).
|
||||
if (path.isAbsolute(script) || script.split(/[/\\]/).includes('..')) continue;
|
||||
// #1460 CONF-1: resolve the declared (relative) script against the capability's OWN install
|
||||
// dir and CONFINE via realpath, then write the ABSOLUTE confined path as the hook command —
|
||||
// never the raw relative path (which would resolve against the CWD at hook-exec time and could
|
||||
// execute an arbitrary file). Absolute/`..` inputs and any script escaping the bundle (e.g.
|
||||
// through a symlinked subdir) return null and are SKIPPED, exactly as before.
|
||||
const absScript = confinedBundleScript(capDir(runtimeDir, capId), script);
|
||||
if (absScript === null) continue;
|
||||
// #1460 (R) HIGH: the hook `command` is consumed by a shell (first-party hooks emit
|
||||
// `node "${CLAUDE_PLUGIN_ROOT}/hooks/x.js"`). The absolute path begins with the
|
||||
// (non-manifest) install-prefix, which commonly contains spaces — emit it POSIX
|
||||
// single-quoted so the prefix cannot word-split or inject. The script BASENAME is
|
||||
// already restricted to a shell-safe allowlist by isSafeHookScriptPath above.
|
||||
const command = shellSingleQuote(absScript);
|
||||
const arr = Array.isArray(hooksObj[event]) ? (hooksObj[event] as unknown[]) : [];
|
||||
arr.push({ [CAP_MARKER]: capId, hooks: [{ type: 'command', command: script }] });
|
||||
arr.push({ [CAP_MARKER]: capId, hooks: [{ type: 'command', command }] });
|
||||
hooksObj[event] = arr;
|
||||
touched = true;
|
||||
}
|
||||
@@ -1545,6 +1637,11 @@ export = {
|
||||
reconcileCapabilities,
|
||||
applyCapabilitySharedEdits,
|
||||
stripCapabilitySharedEdits,
|
||||
// #1460 CONF-2: exported so the ancestor-symlink confinement is locked in by a regression test.
|
||||
confinedSharedFile,
|
||||
// #1460 (R) HIGH: exported so the shell-unsafe-script defense-in-depth (returns null for an
|
||||
// unsafe-char script even when the file exists in the bundle) is locked in by a regression test.
|
||||
confinedBundleScript,
|
||||
CAP_MARKER,
|
||||
// Exported for cross-process-lock unit tests (CONC-1/CONC-2/finding-1). Not part of the public CLI
|
||||
// surface. #1459 finding 4: the lock primitive now lives in the shared capability-lock module; these
|
||||
|
||||
@@ -344,6 +344,42 @@ function readManifestBounded(
|
||||
return cap as Record<string, unknown>;
|
||||
}
|
||||
|
||||
/**
|
||||
* #1460 CS-1: read a locally-produced `npm pack` `.tgz` as RAW BYTES via a bounded fd read so a
|
||||
* supplied `--integrity` can be verified over the tarball (same SRI sha512 domain as the tarball
|
||||
* adapter) before extraction/staging. `readSmallRegularFile` decodes utf8 (corrupting binary), so
|
||||
* this reads the Buffer directly while keeping the same fail-closed discipline: open → fstat →
|
||||
* require a regular file (a FIFO/device cannot BLOCK or be misread) → size-cap (MAX_RESPONSE_BYTES,
|
||||
* the same ceiling the HTTP fetch enforces) → read exactly fstat.size bytes.
|
||||
*/
|
||||
function readPackTarball(tgzPath: string): Buffer {
|
||||
let fd: number;
|
||||
try {
|
||||
fd = fs.openSync(tgzPath, 'r');
|
||||
} catch (err) {
|
||||
throw new Error(`Cannot read npm pack tarball: ${tgzPath}: ${(err as Error).message}`);
|
||||
}
|
||||
try {
|
||||
const st = fs.fstatSync(fd);
|
||||
if (!st.isFile()) {
|
||||
throw new Error(`Refusing to read non-regular npm pack tarball: ${tgzPath}`);
|
||||
}
|
||||
if (st.size > MAX_RESPONSE_BYTES) {
|
||||
throw new Error(`npm pack tarball exceeds ${MAX_RESPONSE_BYTES} bytes: ${tgzPath}`);
|
||||
}
|
||||
const buf = Buffer.allocUnsafe(st.size);
|
||||
let read = 0;
|
||||
while (read < st.size) {
|
||||
const n = fs.readSync(fd, buf, read, st.size - read, read);
|
||||
if (n === 0) break;
|
||||
read += n;
|
||||
}
|
||||
return read === st.size ? buf : buf.subarray(0, read);
|
||||
} finally {
|
||||
try { fs.closeSync(fd); } catch { /* best-effort */ }
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Reject spec/id values containing path separators or `..`.
|
||||
* Throws if the id is unsafe.
|
||||
@@ -813,6 +849,14 @@ function resolveLocal(
|
||||
gsdHome: string,
|
||||
hostVersion: string
|
||||
): ResolveResult {
|
||||
// #1460 CS-1: a local path is a directory tree, not a single downloadable artifact, so there is
|
||||
// no stable byte stream to verify a sha512 SRI pin against. A supplied `--integrity` is therefore
|
||||
// REJECTED with an actionable error rather than being silently dropped (the prior behaviour staged
|
||||
// with integrity:null, so the user believed content was pinned when it was not).
|
||||
if (opts.integrity) {
|
||||
throw new Error('integrity pinning is not supported for local sources');
|
||||
}
|
||||
|
||||
const absPath = path.resolve(parsed.target);
|
||||
if (!fs.existsSync(absPath)) {
|
||||
throw new Error(`Local capability path does not exist: ${absPath}`);
|
||||
@@ -838,6 +882,14 @@ function resolveGit(
|
||||
): ResolveResult {
|
||||
const execGit = opts.execOverrides?.git ?? shellSeam.execGit;
|
||||
|
||||
// #1460 CS-1: a git working tree has no single downloadable artifact to verify a sha512 SRI pin
|
||||
// against (a clone is a directory tree, and the digest would vary with pack/checkout details). A
|
||||
// supplied `--integrity` is therefore REJECTED with an actionable error rather than silently
|
||||
// dropped (the prior behaviour staged with integrity:null). Pin a git source by COMMIT instead.
|
||||
if (opts.integrity) {
|
||||
throw new Error('integrity pinning is not supported for git sources; pin the commit with #sha:<commit>');
|
||||
}
|
||||
|
||||
const cloneDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-cap-git-'));
|
||||
try {
|
||||
// Clone (copy only — no hooks execute on clone, no npm install).
|
||||
@@ -906,6 +958,17 @@ function resolveNpm(
|
||||
}
|
||||
const tgzPath = path.join(tmpPackDir, tarballs[0]);
|
||||
|
||||
// #1460 CS-1: a supplied `--integrity` is verified over the `.tgz` BYTES (same SRI sha512
|
||||
// domain as the tarball adapter) BEFORE anything is staged or promoted — never silently
|
||||
// dropped. The recorded integrity is always the computed digest of the produced tarball.
|
||||
// `npm pack --ignore-scripts` (above) ran no capability code, so reading these bytes is
|
||||
// copy-only. A mismatch throws here, before assertSafeTarMembers / extraction / staging.
|
||||
const tgzBytes = readPackTarball(tgzPath);
|
||||
const computedIntegrity = computeIntegrity(tgzBytes);
|
||||
if (opts.integrity) {
|
||||
verifyIntegrity(tgzBytes, opts.integrity);
|
||||
}
|
||||
|
||||
// Reject tar-slip member paths before extracting.
|
||||
assertSafeTarMembers(execTar, tgzPath);
|
||||
|
||||
@@ -929,7 +992,7 @@ function resolveNpm(
|
||||
const id = typeof cap['id'] === 'string' ? cap['id'] : '';
|
||||
if (!id) throw new Error('capability.json missing "id" field');
|
||||
|
||||
return stageValidated({ sourceDir, id, gsdHome, hostVersion, source: parsed.raw, integrity: null, promote: opts.promote, skipEnginesGate: opts.skipEnginesGate });
|
||||
return stageValidated({ sourceDir, id, gsdHome, hostVersion, source: parsed.raw, integrity: computedIntegrity, promote: opts.promote, skipEnginesGate: opts.skipEnginesGate });
|
||||
} finally {
|
||||
try { fs.rmSync(tmpPackDir, { recursive: true, force: true }); } catch { /* best-effort */ }
|
||||
try { fs.rmSync(extractDir, { recursive: true, force: true }); } catch { /* best-effort */ }
|
||||
|
||||
@@ -108,6 +108,18 @@ function capManifestVersion(dir, id) {
|
||||
return JSON.parse(fs.readFileSync(path.join(dir, '.gsd', 'capabilities', id, 'capability.json'), 'utf8')).version;
|
||||
} catch { return null; }
|
||||
}
|
||||
/**
|
||||
* #1460 CONF-1: the expected absolute hook command — `script` resolved against the
|
||||
* capability install dir and confined via realpath of the existing ancestor chain
|
||||
* (so an ancestor symlink cannot escape). Mirrors confinedBundleScript in the source.
|
||||
*/
|
||||
function expectedBundleCommand(dir, id, script) {
|
||||
const capDir = path.join(dir, '.gsd', 'capabilities', id);
|
||||
const target = path.resolve(capDir, script);
|
||||
let parent = path.dirname(target);
|
||||
try { parent = fs.realpathSync(parent); } catch { /* lexical fallback below */ }
|
||||
return path.join(parent, path.basename(target));
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Install
|
||||
@@ -294,7 +306,8 @@ test('upgrade: a changed executable set without consent aborts and leaves the OL
|
||||
assert.strictEqual(res.requiresConsent, true);
|
||||
assert.strictEqual(capManifestVersion(dir, 'e'), '1.0.0', 'old bundle untouched');
|
||||
assert.strictEqual(readLedgerEntry(dir, 'e').version, '1.0.0', 'old ledger untouched');
|
||||
assert.strictEqual(readSettings(dir).hooks.PostToolUse[0].hooks[0].command, 'hooks/a.js');
|
||||
// #1460 CONF-1/(R): command is the ABSOLUTE confined path inside the bundle (POSIX single-quoted), not the raw relative form.
|
||||
assert.strictEqual(readSettings(dir).hooks.PostToolUse[0].hooks[0].command, shQuote(expectedBundleCommand(dir, 'e', 'hooks/a.js')));
|
||||
});
|
||||
|
||||
test('upgrade: a changed executable set WITH consent upgrades and re-derives shared edits', async () => {
|
||||
@@ -310,7 +323,110 @@ test('upgrade: a changed executable set WITH consent upgrades and re-derives sha
|
||||
assert.strictEqual(res.status, 'upgraded');
|
||||
const hooks = readSettings(dir).hooks.PostToolUse;
|
||||
assert.strictEqual(hooks.length, 1);
|
||||
assert.strictEqual(hooks[0].hooks[0].command, 'hooks/b.js', 'old shared edit stripped, new applied');
|
||||
// #1460 CONF-1/(R): re-derived command is the ABSOLUTE confined path inside the bundle (POSIX single-quoted).
|
||||
assert.strictEqual(hooks[0].hooks[0].command, shQuote(expectedBundleCommand(dir, 'e', 'hooks/b.js')), 'old shared edit stripped, new applied (quoted absolute confined path)');
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// #1460 (R) HIGH: the emitted hook `command` must be shell-safe
|
||||
// ---------------------------------------------------------------------------
|
||||
// A hook `command` string is consumed by a shell (first-party hooks emit
|
||||
// `node "${CLAUDE_PLUGIN_ROOT}/hooks/x.js"`). The non-manifest install-prefix
|
||||
// (the home/runtime dir) commonly contains spaces (e.g. "/Users/Bob Smith/...")
|
||||
// — written unquoted it word-splits and breaks (or, with a hostile prefix,
|
||||
// could inject). The emitted absolute command must be POSIX single-quoted.
|
||||
|
||||
/** A runtime dir whose absolute path contains a SPACE (mirrors "/Users/Bob Smith/.claude"). */
|
||||
function runtimeWithSpace() {
|
||||
const base = fs.mkdtempSync(path.join(os.tmpdir(), 'cap life-'));
|
||||
cleanups.push(base);
|
||||
const dir = path.join(base, 'Bob Smith', '.claude');
|
||||
fs.mkdirSync(dir, { recursive: true });
|
||||
return dir;
|
||||
}
|
||||
|
||||
/** POSIX single-quote a string the way applyCapabilitySharedEdits must. */
|
||||
function shQuote(s) {
|
||||
return "'" + String(s).replace(/'/g, "'\\''") + "'";
|
||||
}
|
||||
|
||||
test('#1460 (R): emitted hook command is single-quoted when the install prefix contains a space', async () => {
|
||||
const dir = runtimeWithSpace();
|
||||
assert.ok(dir.includes(' '), 'precondition: install prefix contains a space');
|
||||
const res = await lifecycle.installCapability('./e', {
|
||||
runtimeDir: dir, hostVersion: '1.6.0', consentGranted: true, sharedFiles: ['settings.json'],
|
||||
_resolve: fakeResolve(execCap('e', '1.0.0', { script: 'hooks/format.sh' })),
|
||||
});
|
||||
assert.strictEqual(res.status, 'installed', JSON.stringify(res));
|
||||
const command = readSettings(dir).hooks.PostToolUse[0].hooks[0].command;
|
||||
const expectedAbs = expectedBundleCommand(dir, 'e', 'hooks/format.sh');
|
||||
// revert-fails: without quoting the command is the bare space-containing absolute path,
|
||||
// which a shell would word-split (the second token would be executed as a command).
|
||||
assert.strictEqual(command, shQuote(expectedAbs), 'command must be POSIX single-quoted');
|
||||
// Defense-in-depth: emulate POSIX word-splitting — single-quoted runs are atomic (whitespace
|
||||
// inside them does NOT split). The quoted command must collapse to exactly ONE word (the path).
|
||||
const words = command.match(/'[^']*'|[^\s']+/g) || [];
|
||||
assert.strictEqual(words.length, 1, 'quoting must keep the space-containing path as a single shell word');
|
||||
assert.strictEqual(words[0], command, 'the single word IS the entire quoted command');
|
||||
// Negative control: the UNQUOTED path would split into >1 word at the space (the bug this prevents).
|
||||
assert.ok((expectedAbs.match(/'[^']*'|[^\s']+/g) || []).length > 1, 'precondition: the bare path word-splits');
|
||||
});
|
||||
|
||||
test('#1460 (R): a normal script under a normal prefix emits the quoted absolute command and stays strippable by CAP_MARKER', async () => {
|
||||
const dir = runtime();
|
||||
await lifecycle.installCapability('./e', {
|
||||
runtimeDir: dir, hostVersion: '1.6.0', consentGranted: true, sharedFiles: ['settings.json'],
|
||||
_resolve: fakeResolve(execCap('e', '1.0.0', { script: 'hooks/run.js' })),
|
||||
});
|
||||
const before = readSettings(dir).hooks.PostToolUse;
|
||||
assert.strictEqual(before.length, 1);
|
||||
assert.strictEqual(before[0][CAP_MARKER], 'e');
|
||||
// revert-fails: without quoting the control command is the bare absolute path.
|
||||
assert.strictEqual(before[0].hooks[0].command, shQuote(expectedBundleCommand(dir, 'e', 'hooks/run.js')));
|
||||
// Strip is keyed on CAP_MARKER===capId, NOT the command string — quoting does not break it.
|
||||
const rem = await lifecycle.removeCapability('e', {
|
||||
runtimeDir: dir, hostVersion: '1.6.0', sharedFiles: ['settings.json'],
|
||||
});
|
||||
assert.strictEqual(rem.status, 'removed', JSON.stringify(rem));
|
||||
const after = readSettings(dir);
|
||||
assert.ok(!after || !after.hooks || !after.hooks.PostToolUse || after.hooks.PostToolUse.length === 0,
|
||||
'the stamped hook is stripped by CAP_MARKER regardless of quoting');
|
||||
});
|
||||
|
||||
test('#1460 (R): idempotent strip-then-reapply yields the identical quoted command', () => {
|
||||
const dir = runtime();
|
||||
const capId = 'e';
|
||||
const capDirPath = path.join(dir, '.gsd', 'capabilities', capId);
|
||||
fs.mkdirSync(path.join(capDirPath, 'hooks'), { recursive: true });
|
||||
fs.writeFileSync(path.join(capDirPath, 'hooks', 'run.js'), '// x', 'utf8');
|
||||
const manifest = execCap(capId, '1.0.0', { script: 'hooks/run.js' });
|
||||
const apply = () => lifecycle.applyCapabilitySharedEdits({
|
||||
runtimeDir: dir, capId, manifest, sharedFiles: ['settings.json'],
|
||||
});
|
||||
const edits = apply();
|
||||
const first = readSettings(dir).hooks.PostToolUse;
|
||||
// The real install/upgrade transition is strip-then-apply; re-running it must converge.
|
||||
lifecycle.stripCapabilitySharedEdits({ runtimeDir: dir, capId, sharedEdits: edits });
|
||||
apply();
|
||||
const second = readSettings(dir).hooks.PostToolUse;
|
||||
assert.strictEqual(second.length, 1, 'idempotent: still exactly one stamped hook after strip+reapply');
|
||||
assert.strictEqual(second[0].hooks[0].command, first[0].hooks[0].command, 'identical quoted command on re-apply');
|
||||
assert.strictEqual(second[0].hooks[0].command, shQuote(expectedBundleCommand(dir, capId, 'hooks/run.js')));
|
||||
});
|
||||
|
||||
test('#1460 (R): confinedBundleScript returns null for an unsafe-char script (defense-in-depth)', () => {
|
||||
const dir = runtime();
|
||||
const capDirPath = path.join(dir, '.gsd', 'capabilities', 'e');
|
||||
fs.mkdirSync(capDirPath, { recursive: true });
|
||||
// Even when the file literally exists on disk inside the bundle, an unsafe-char script
|
||||
// name must be refused — so applyCapabilitySharedEdits skips it even if validation were
|
||||
// bypassed. revert-fails: without the allowlist guard this returns the absolute path.
|
||||
fs.writeFileSync(path.join(capDirPath, 'run.sh; touch pwn'), '// x', 'utf8');
|
||||
assert.strictEqual(lifecycle.confinedBundleScript(capDirPath, 'run.sh; touch pwn'), null);
|
||||
// A normal script still resolves to its confined absolute path.
|
||||
fs.writeFileSync(path.join(capDirPath, 'ok.sh'), '// x', 'utf8');
|
||||
const ok = lifecycle.confinedBundleScript(capDirPath, 'ok.sh');
|
||||
assert.ok(typeof ok === 'string' && ok.endsWith(path.join('e', 'ok.sh')), 'normal script resolves: ' + ok);
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
@@ -813,6 +929,98 @@ test('applyCapabilitySharedEdits: __proto__ event/name is skipped (no pollution)
|
||||
assert.strictEqual({}.command, undefined);
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// #1460 CONF-1 — a hook command is written as the ABSOLUTE path inside the
|
||||
// capability's own install dir, confined via realpath; a script that resolves
|
||||
// OUTSIDE the bundle is NOT written.
|
||||
// revert-fails: with the raw `command: script` restored (pre-fix), the absolute
|
||||
// assertion fails and a bundle-escaping script would be written.
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
test('#1460 CONF-1: a relative hook script is emitted as the absolute path inside capDir', () => {
|
||||
const dir = runtime();
|
||||
const capId = 'conf1';
|
||||
// Make the install dir real so realpath confinement resolves a concrete chain.
|
||||
const capDir = path.join(dir, '.gsd', 'capabilities', capId);
|
||||
fs.mkdirSync(path.join(capDir, 'subdir'), { recursive: true });
|
||||
fs.writeFileSync(path.join(capDir, 'subdir', 'run.js'), '// hook', 'utf8');
|
||||
|
||||
lifecycle.applyCapabilitySharedEdits({
|
||||
runtimeDir: dir,
|
||||
capId,
|
||||
manifest: { hooks: [{ event: 'PostToolUse', script: 'subdir/run.js' }] },
|
||||
sharedFiles: ['settings.json'],
|
||||
});
|
||||
|
||||
const s = readSettings(dir);
|
||||
const command = s.hooks.PostToolUse[0].hooks[0].command;
|
||||
const expectedAbs = path.join(fs.realpathSync(path.join(capDir, 'subdir')), 'run.js');
|
||||
// #1460 (R): the emitted command is the absolute confined path, POSIX single-quoted.
|
||||
assert.strictEqual(command, shQuote(expectedAbs), 'command must be the quoted absolute confined path inside capDir');
|
||||
const unquoted = command.slice(1, -1); // strip the wrapping single quotes for the path-shape checks
|
||||
assert.ok(path.isAbsolute(unquoted), 'command must be absolute (CWD-independent)');
|
||||
assert.ok(unquoted.startsWith(fs.realpathSync(capDir) + path.sep), 'command must live inside the bundle');
|
||||
});
|
||||
|
||||
test('#1460 CONF-1: a script resolving OUTSIDE capDir via a symlinked subdir is NOT written (skipped)', (t) => {
|
||||
const dir = runtime();
|
||||
const capId = 'conf1-escape';
|
||||
const capDir = path.join(dir, '.gsd', 'capabilities', capId);
|
||||
fs.mkdirSync(capDir, { recursive: true });
|
||||
|
||||
// Plant a victim file outside the bundle and a symlinked subdir inside the bundle
|
||||
// that points at the victim's parent. A relative script "evil/run.js" would then
|
||||
// resolve to the victim through the symlink — confinement must refuse it.
|
||||
const outside = fs.mkdtempSync(path.join(os.tmpdir(), 'conf1-outside-'));
|
||||
cleanups.push(outside);
|
||||
fs.writeFileSync(path.join(outside, 'run.js'), '// victim', 'utf8');
|
||||
try {
|
||||
fs.symlinkSync(outside, path.join(capDir, 'evil'));
|
||||
} catch {
|
||||
t.skip('symlink not supported on this platform');
|
||||
return;
|
||||
}
|
||||
|
||||
lifecycle.applyCapabilitySharedEdits({
|
||||
runtimeDir: dir,
|
||||
capId,
|
||||
manifest: { hooks: [{ event: 'PostToolUse', script: 'evil/run.js' }] },
|
||||
sharedFiles: ['settings.json'],
|
||||
});
|
||||
|
||||
const s = readSettings(dir);
|
||||
// The escaping hook is skipped → no settings written at all (no touched edits).
|
||||
assert.strictEqual(s, null, 'no shared-config edit must be written for a bundle-escaping script');
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// #1460 CONF-2 — REGRESSION GUARD: confinedSharedFile realpaths the FULL ancestor
|
||||
// chain, so an ANCESTOR symlink (not just the final component) cannot escape.
|
||||
// revert-fails: if confinedSharedFile were changed to realpath only the final
|
||||
// component, a path through a symlinked ancestor would resolve OUTSIDE runtimeDir
|
||||
// and this test (asserting null) would fail.
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
test('#1460 CONF-2: confinedSharedFile refuses a path through a symlinked ANCESTOR directory', (t) => {
|
||||
const dir = runtime();
|
||||
// Build runtimeDir/inner where `inner` is a symlink to a directory OUTSIDE runtimeDir.
|
||||
const outside = fs.mkdtempSync(path.join(os.tmpdir(), 'conf2-outside-'));
|
||||
cleanups.push(outside);
|
||||
fs.mkdirSync(path.join(outside, 'deep'), { recursive: true });
|
||||
try {
|
||||
fs.symlinkSync(outside, path.join(dir, 'inner'));
|
||||
} catch {
|
||||
t.skip('symlink not supported on this platform');
|
||||
return;
|
||||
}
|
||||
|
||||
// A path whose ANCESTOR ("inner") is the escaping symlink — the final component
|
||||
// ("settings.json") is not itself a link, so a final-component-only realpath would
|
||||
// miss the escape. confinedSharedFile realpaths the parent chain and must return null.
|
||||
const result = lifecycle.confinedSharedFile(dir, path.join('inner', 'deep', 'settings.json'));
|
||||
assert.strictEqual(result, null, 'a path through a symlinked ancestor must be refused (null)');
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Site B: reconcileCapabilities on a corrupt-present ledger must surface a
|
||||
// warning in its report, not silently do nothing (#1462).
|
||||
|
||||
@@ -1819,6 +1819,62 @@ describe('C4: description and hooks validation', () => {
|
||||
assert.deepEqual(hookErrors, [], 'Expected no hook errors for valid hooks entry, got: ' + JSON.stringify(hookErrors));
|
||||
});
|
||||
|
||||
// ─── #1460 (R) HIGH: hook script path must be shell-safe ──────────────────
|
||||
// The hook `script` is resolved to an absolute path and written verbatim as the hook
|
||||
// `command` STRING in settings.json (consumed by a shell). A manifest-controlled script
|
||||
// name containing shell metacharacters (`;`, `|`, `$`, backtick, whitespace, …) would
|
||||
// inject a second command at hook-exec time. Fail closed at the validator: reject any
|
||||
// script path outside the conservative [A-Za-z0-9._/-] allowlist. revert-fails: without
|
||||
// the allowlist these all pass the non-empty-string check and validate OK.
|
||||
for (const [label, script] of [
|
||||
['command-injection via `;`', 'run.sh; touch /tmp/pwn'],
|
||||
['embedded space', 'my hook.sh'],
|
||||
['command substitution `$( )`', 'run-$(whoami).sh'],
|
||||
['backtick substitution', 'run-`id`.sh'],
|
||||
['pipe metacharacter', 'a.sh|b.sh'],
|
||||
['newline injection', 'a.sh\ntouch /tmp/pwn'],
|
||||
['ampersand background', 'a.sh & evil'],
|
||||
['shell glob', 'hooks/*.sh'],
|
||||
['redirect', 'a.sh > /tmp/pwn'],
|
||||
['leading dash (option injection)', '-rf'],
|
||||
['single quote', "a'.sh"],
|
||||
['double quote', 'a".sh'],
|
||||
['NUL/control char', 'a\u0000.sh'],
|
||||
]) {
|
||||
test(`hook script with unsafe chars is rejected (${label})`, () => {
|
||||
const cap = { ...UI_CAP, hooks: [{ event: 'PostToolUse', script }] };
|
||||
const errors = validateCapability(cap, 'ui');
|
||||
const hookErrors = errors.filter((e) => e.includes('hooks[0].script'));
|
||||
assert.ok(
|
||||
hookErrors.length > 0,
|
||||
`Expected a hooks[0].script rejection for ${label} (script=${JSON.stringify(script)}), got: ` + JSON.stringify(errors),
|
||||
);
|
||||
assert.ok(
|
||||
hookErrors.some((e) => /unsafe character/.test(e)),
|
||||
'Error should mention unsafe characters, got: ' + JSON.stringify(hookErrors),
|
||||
);
|
||||
});
|
||||
}
|
||||
|
||||
test('hook script with absolute path is rejected', () => {
|
||||
const cap = { ...UI_CAP, hooks: [{ event: 'PostToolUse', script: '/etc/evil.sh' }] };
|
||||
const errors = validateCapability(cap, 'ui');
|
||||
assert.ok(errors.some((e) => e.includes('hooks[0].script')), 'absolute script must be rejected: ' + JSON.stringify(errors));
|
||||
});
|
||||
|
||||
test('hook script with .. traversal is rejected', () => {
|
||||
const cap = { ...UI_CAP, hooks: [{ event: 'PostToolUse', script: '../../etc/evil.sh' }] };
|
||||
const errors = validateCapability(cap, 'ui');
|
||||
assert.ok(errors.some((e) => e.includes('hooks[0].script')), '.. script must be rejected: ' + JSON.stringify(errors));
|
||||
});
|
||||
|
||||
test('hook script with a normal nested relative path is still accepted', () => {
|
||||
const cap = { ...UI_CAP, hooks: [{ event: 'PostToolUse', script: 'hooks/sub-dir/format_v2.sh' }] };
|
||||
const errors = validateCapability(cap, 'ui');
|
||||
const hookErrors = errors.filter((e) => e.includes('hooks[0].script'));
|
||||
assert.deepEqual(hookErrors, [], 'Expected a normal nested relative script to be accepted, got: ' + JSON.stringify(hookErrors));
|
||||
});
|
||||
|
||||
test('description present in UI_CAP passes validation', () => {
|
||||
const errors = validateCapability(UI_CAP, 'ui');
|
||||
const descErrors = errors.filter((e) => e.includes('description'));
|
||||
|
||||
@@ -404,6 +404,7 @@ describe('tarball adapter — integrity via _setCapabilitySourceHttpGet', () =>
|
||||
|
||||
_setCapabilitySourceHttpGet(() => Promise.resolve({ statusCode: 200, body: tgzBuf }));
|
||||
|
||||
let tarCalls = 0;
|
||||
await assert.rejects(
|
||||
() =>
|
||||
resolveCapabilitySource('https://example.com/tarball-mismatch.tgz', {
|
||||
@@ -412,7 +413,8 @@ describe('tarball adapter — integrity via _setCapabilitySourceHttpGet', () =>
|
||||
integrity: badIntegrity,
|
||||
execOverrides: {
|
||||
tar: (_prog, args, _opts) => {
|
||||
// Should never be reached — integrity check fires first.
|
||||
// Must never be reached — integrity check fires before any tar invocation.
|
||||
tarCalls++;
|
||||
const extractDir = args[args.indexOf('-C') + 1];
|
||||
fs.writeFileSync(
|
||||
path.join(extractDir, 'capability.json'),
|
||||
@@ -426,12 +428,146 @@ describe('tarball adapter — integrity via _setCapabilitySourceHttpGet', () =>
|
||||
/integrity mismatch|mismatch/i
|
||||
);
|
||||
|
||||
// Integrity is verified over the raw .tgz bytes BEFORE any tar call — ordering invariant.
|
||||
assert.strictEqual(tarCalls, 0, 'tar must NOT be invoked — integrity verified over the .tgz bytes before extraction');
|
||||
|
||||
// No staged directory must exist.
|
||||
const finalDir = path.join(gsdHome, '.gsd', 'capabilities', 'tarball-mismatch');
|
||||
assert.ok(!fs.existsSync(finalDir), 'staged dir must NOT exist after integrity mismatch');
|
||||
});
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// #1460 CS-1 — a supplied --integrity is NEVER silently ignored.
|
||||
// - npm: verified over the produced .tgz bytes (same SRI sha512 domain as tarball).
|
||||
// - git / local: REJECTED with an actionable error (no single byte-SRI artifact).
|
||||
// revert-fails: with stageValidated's `integrity: null` restored on these adapters
|
||||
// (the pre-fix behaviour), the npm-mismatch case would silently resolve and the
|
||||
// git/local cases would silently resolve with integrity:null — every assert below
|
||||
// would then fail.
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
describe('#1460 CS-1 — supplied --integrity is verified or rejected per source (never silently dropped)', () => {
|
||||
let gsdHome = '';
|
||||
beforeEach(() => { gsdHome = createTempDir('gsd-home-'); });
|
||||
afterEach(() => { cleanup(gsdHome); });
|
||||
|
||||
/** Build a fake `npm pack` that writes a real .tgz of `tgzBytes` to the --pack-destination. */
|
||||
function fakeNpmPack(tgzBytes) {
|
||||
return (args) => {
|
||||
const destIdx = args.indexOf('--pack-destination');
|
||||
const dest = destIdx >= 0 ? args[destIdx + 1] : '';
|
||||
fs.writeFileSync(path.join(dest, 'cap.tgz'), tgzBytes);
|
||||
return { exitCode: 0, stdout: 'cap.tgz\n', stderr: '', signal: null, error: null };
|
||||
};
|
||||
}
|
||||
|
||||
/** A tar override that lists safe members and writes `cap`'s capability.json on extract. */
|
||||
function fakeTar(cap) {
|
||||
return (_prog, args) => {
|
||||
if (args[0] === '-tzf') {
|
||||
return { exitCode: 0, stdout: 'package/capability.json\n', stderr: '', signal: null, error: null };
|
||||
}
|
||||
if (args[0] === '-tvzf') {
|
||||
return { exitCode: 0, stdout: '-rw-r--r-- 0 user group 10 Jan 1 2020 package/capability.json\n', stderr: '', signal: null, error: null };
|
||||
}
|
||||
const extractDir = args[args.indexOf('-C') + 1];
|
||||
const pkgDir = path.join(extractDir, 'package');
|
||||
fs.mkdirSync(pkgDir, { recursive: true });
|
||||
fs.writeFileSync(path.join(pkgDir, 'capability.json'), JSON.stringify(cap), 'utf8');
|
||||
return { exitCode: 0, stdout: '', stderr: '', signal: null, error: null };
|
||||
};
|
||||
}
|
||||
|
||||
test('npm + matching --integrity (over the .tgz bytes) → resolves', async () => {
|
||||
const cap = featureCap('npm-int-ok');
|
||||
const tgzBytes = Buffer.from('a deterministic npm tarball payload for integrity', 'utf8');
|
||||
const integrity = sha512b64(tgzBytes);
|
||||
|
||||
const result = await resolveCapabilitySource('npm:@org/npm-int-ok@^1.0.0', {
|
||||
gsdHome,
|
||||
hostVersion: '1.5.0',
|
||||
integrity,
|
||||
execOverrides: { npm: fakeNpmPack(tgzBytes), tar: fakeTar(cap) },
|
||||
});
|
||||
|
||||
assert.strictEqual(result.id, 'npm-int-ok');
|
||||
assert.ok(result.integrity && result.integrity.startsWith('sha512-'), 'integrity must be recorded');
|
||||
assert.ok(fs.existsSync(result.stagedDir), 'staged dir must exist');
|
||||
});
|
||||
|
||||
test('npm + MISMATCHING --integrity → throws BEFORE promote/staging (no final dir)', async () => {
|
||||
const cap = featureCap('npm-int-bad');
|
||||
const tgzBytes = Buffer.from('the real npm tarball bytes', 'utf8');
|
||||
const badIntegrity = 'sha512-' + Buffer.from('not-the-real-hash').toString('base64');
|
||||
|
||||
let tarCalls = 0;
|
||||
const instrumentedTar = (_prog, args) => {
|
||||
// Must never be reached — integrity is verified over the .tgz bytes before any tar call.
|
||||
tarCalls++;
|
||||
return fakeTar(cap)(_prog, args);
|
||||
};
|
||||
|
||||
await assert.rejects(
|
||||
() => resolveCapabilitySource('npm:@org/npm-int-bad@^1.0.0', {
|
||||
gsdHome,
|
||||
hostVersion: '1.5.0',
|
||||
integrity: badIntegrity,
|
||||
execOverrides: { npm: fakeNpmPack(tgzBytes), tar: instrumentedTar },
|
||||
}),
|
||||
/integrity mismatch|mismatch/i,
|
||||
);
|
||||
|
||||
// Integrity is verified over the raw .tgz bytes BEFORE any tar call — ordering invariant.
|
||||
assert.strictEqual(tarCalls, 0, 'tar extraction must NOT run — integrity verified over the .tgz bytes before extraction');
|
||||
|
||||
const finalDir = path.join(gsdHome, '.gsd', 'capabilities', 'npm-int-bad');
|
||||
assert.ok(!fs.existsSync(finalDir), 'no final dir after integrity mismatch');
|
||||
});
|
||||
|
||||
test('git + any --integrity → throws an actionable error (no silent resolve)', async () => {
|
||||
// The integrity reject must fire BEFORE the clone — so execGit is never called.
|
||||
const gitCalls = [];
|
||||
const fakeGit = (...callArgs) => {
|
||||
gitCalls.push(callArgs);
|
||||
return { exitCode: 0, stdout: '', stderr: '', signal: null, error: null };
|
||||
};
|
||||
|
||||
await assert.rejects(
|
||||
() => resolveCapabilitySource('https://github.com/org/repo.git#v1.0.0', {
|
||||
gsdHome,
|
||||
hostVersion: '1.5.0',
|
||||
integrity: 'sha512-' + Buffer.from('anything').toString('base64'),
|
||||
execOverrides: { git: fakeGit },
|
||||
}),
|
||||
/integrity pinning is not supported for git sources|#sha:/i,
|
||||
);
|
||||
|
||||
assert.strictEqual(gitCalls.length, 0, 'execGit must NOT run — rejection fires before the clone');
|
||||
const finalDir = path.join(gsdHome, '.gsd', 'capabilities', 'repo');
|
||||
assert.ok(!fs.existsSync(finalDir), 'no final dir after git integrity rejection');
|
||||
});
|
||||
|
||||
test('local + --integrity → throws an actionable error (no silent resolve)', async () => {
|
||||
const capDir = makeLocalCap(featureCap('local-int-cap'));
|
||||
try {
|
||||
await assert.rejects(
|
||||
() => resolveCapabilitySource(capDir, {
|
||||
gsdHome,
|
||||
hostVersion: '1.5.0',
|
||||
integrity: 'sha512-' + Buffer.from('anything').toString('base64'),
|
||||
}),
|
||||
/integrity pinning is not supported for local sources/i,
|
||||
);
|
||||
} finally {
|
||||
cleanup(capDir);
|
||||
}
|
||||
|
||||
const finalDir = path.join(gsdHome, '.gsd', 'capabilities', 'local-int-cap');
|
||||
assert.ok(!fs.existsSync(finalDir), 'no final dir after local integrity rejection');
|
||||
});
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Security: shell metacharacters in spec / args → arrive as argv array
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
Reference in New Issue
Block a user