fix(#3514): deny internal fetch hosts; disclose unverified integrity (#3516)

* test(#3514): add failing-first denylist and integrity suites * fix(#3514): deny internal fetch hosts; disclose unverified integrity * docs(#3514): trust-model, glossary, and changeset entries * fix(#3514): scope v6 checks to literals; exact pin kinds in prompt * chore(#3514): backfill changeset pr number --------- Co-authored-by: sim <sim@local>
2026-08-14 21:34:29 -04:00
parent 268ca7e32d
commit 6badb839a0
18 changed files with 842 additions and 602 deletions
--- a/tests/capability-lifecycle.test.cjs
+++ b/tests/capability-lifecycle.test.cjs
@@ -3397,3 +3397,48 @@ test('#1463 outdated: npm EXACT-pinned source (@1.2.3) → status pinned (update
  const [rec] = lifecycle.outdatedCapabilities({ runtimeDir: dir, execOverrides: { npm: fakeNpm } });
  assert.strictEqual(rec.status, 'pinned');
 });
+
+// ---------------------------------------------------------------------------
+// #3514 (epic #1900 F21c) — the lifecycle threads integrityPinned from the
+// install opts (and a git #sha: pin) into the trust verdict's disclosure, so
+// the consent prompt the CLI renders distinguishes pinned from unverified.
+// ---------------------------------------------------------------------------
+
+test('#3514: an unpinned executable install aborts with an unverified disclosure', async () => {
+  const dir = runtime();
+  const res = await lifecycle.installCapability('./exec', {
+    runtimeDir: dir, hostVersion: '1.6.0', consentGranted: false,
+    _resolve: fakeResolve(execCap('exec', '1.0.0')),
+  });
+  assert.strictEqual(res.status, 'aborted');
+  assert.strictEqual(res.disclosure.integrityStatus, 'unverified');
+});
+
+test('#3514: an install with a supplied --integrity pin aborts with a pinned disclosure', async () => {
+  const dir = runtime();
+  const res = await lifecycle.installCapability('./exec', {
+    runtimeDir: dir, hostVersion: '1.6.0', consentGranted: false,
+    integrity: 'sha512-AAAA',
+    _resolve: fakeResolve(execCap('exec', '1.0.0'), { integrity: 'sha512-AAAA' }),
+  });
+  assert.strictEqual(res.status, 'aborted');
+  assert.strictEqual(res.disclosure.integrityStatus, 'pinned');
+});
+
+test('#3514: a git #sha:<hex-commit> source renders commit-pinned; a #sha:<mutable-ref> does not', async () => {
+  const dir = runtime();
+  const shaPin = await lifecycle.installCapability('https://example.com/repo.git#sha:0123456789abcdef0123456789abcdef01234567', {
+    runtimeDir: dir, hostVersion: '1.6.0', consentGranted: false,
+    _resolve: fakeResolve(execCap('exec', '1.0.0')),
+  });
+  assert.strictEqual(shaPin.status, 'aborted');
+  assert.strictEqual(shaPin.disclosure.integrityStatus, 'commit-pinned');
+
+  const mutable = await lifecycle.installCapability('https://example.com/repo.git#sha:main', {
+    runtimeDir: dir, hostVersion: '1.6.0', consentGranted: false,
+    _resolve: fakeResolve(execCap('exec', '1.0.0')),
+  });
+  assert.strictEqual(mutable.status, 'aborted');
+  assert.strictEqual(mutable.disclosure.integrityStatus, 'unverified',
+    'a #sha:<non-hex> ref is a moving ref and must not render as pinned');
+});
--- a/tests/capability-source.test.cjs
+++ b/tests/capability-source.test.cjs
@@ -1826,3 +1826,168 @@ describe('#1463 pickHighestNpmVersion (robust multi-line range parse)', () => {
    );
  });
 });
+
+// ---------------------------------------------------------------------------
+// #3514 (epic #1900 F21) — fetch-transport hardening in realHttpsGet:
+// a loopback/link-local/metadata host denylist (unconditional) and a distinct
+// named reason for non-https URLs (no parse-time hard reject — internal-mirror
+// classification flows stay intact; the https-only transport fails clearly).
+// ---------------------------------------------------------------------------
+
+describe('#3514 F21a — loopback/metadata denylist in realHttpsGet', () => {
+  let gsdHome = '';
+
+  beforeEach(() => { gsdHome = createTempDir('gsd-3514-home-'); });
+  afterEach(() => {
+    _setHttpsGetImpl(null); // restore the real https.get
+    cleanup(gsdHome);
+  });
+
+  /** Wrap makeFakeHttpsGet with a call counter so a test can prove the gate
+   *  rejects BEFORE the transport is invoked. */
+  function makeCountingFake(chunks, headers) {
+    const inner = makeFakeHttpsGet(chunks, headers);
+    const fn = (url, opts, cb) => {
+      fn.calls += 1;
+      return inner(url, opts, cb);
+    };
+    fn.calls = 0;
+    fn.state = inner.state;
+    return fn;
+  }
+
+  // Every entry: no legitimate install fetches these — loopback, link-local
+  // (which contains the cloud metadata IPs), unspecified, or a localhost-form
+  // name. REVERT-FAILS: pre-#3514 these reached the transport.
+  for (const [label, url] of [
+    ['loopback v4', 'https://127.0.0.1/cap.tgz'],
+    ['loopback v4 high octet', 'https://127.255.0.1/cap.tgz'],
+    ['cloud metadata v4', 'https://169.254.169.254/cap.tgz'],
+    ['link-local v4', 'https://169.254.0.9/cap.tgz'],
+    ['unspecified v4', 'https://0.0.0.0/cap.tgz'],
+    ['loopback v6', 'https://[::1]/cap.tgz'],
+    ['link-local v6', 'https://[fe80::1]/cap.tgz'],
+    ['link-local v6 high', 'https://[febf::1]/cap.tgz'],
+    ['unspecified v6', 'https://[::]/cap.tgz'],
+    ['v4-mapped loopback', 'https://[::ffff:127.0.0.1]/cap.tgz'],
+    ['v4-mapped loopback (hex-normalized)', 'https://[::ffff:7f00:1]/cap.tgz'],
+    ['localhost', 'https://localhost/cap.tgz'],
+    ['subdomain of localhost', 'https://svc.localhost/cap.tgz'],
+    ['localhost with trailing FQDN dot', 'https://localhost./cap.tgz'],
+  ]) {
+    test(`denied host (${label}) rejects before the transport is called`, async () => {
+      const fake = makeCountingFake([Buffer.from('x')]);
+      _setHttpsGetImpl(fake);
+      await assert.rejects(
+        () => resolveCapabilitySource(url, { gsdHome, hostVersion: '1.5.0' }),
+        /internal host|loopback|link-local|metadata|denylist/i,
+        `${label} must be refused with the named denylist reason`
+      );
+      assert.strictEqual(fake.calls, 0, `the transport must NEVER be called for ${label}`);
+    });
+  }
+
+  // Spec-review follow-up (#3514): alternate IPv4-literal spellings — decimal integer
+  // (2130706433 = 127.0.0.1), hex (0x7f000001), octal (0177.0.0.1), and short forms (127.1) —
+  // are host LITERALS, not DNS names, so they are inside the denylist's stated scope. The
+  // WHATWG URL parser normalizes every form it accepts to dotted-quad BEFORE `.hostname`
+  // (verified: new URL('https://2130706433/').hostname === '127.0.0.1'), so the gate's
+  // dotted-quad range check already denies them — these tests LOCK that invariant, because
+  // a future refactor to raw-string host matching would silently reopen the bypass.
+  for (const [label, url] of [
+    ['decimal integer form', 'https://2130706433/cap.tgz'],
+    ['hex form', 'https://0x7f000001/cap.tgz'],
+    ['hex per-octet form', 'https://0x7f.0.0.1/cap.tgz'],
+    ['octal form', 'https://0177.0.0.1/cap.tgz'],
+    ['short form', 'https://127.1/cap.tgz'],
+  ]) {
+    test(`alternate IP spelling (${label}) normalizes to a denied dotted-quad`, async () => {
+      const fake = makeCountingFake([Buffer.from('x')]);
+      _setHttpsGetImpl(fake);
+      await assert.rejects(
+        () => resolveCapabilitySource(url, { gsdHome, hostVersion: '1.5.0' }),
+        /internal host|loopback|denylist/i,
+        `${label} must be refused — it is the loopback literal in another spelling`
+      );
+      assert.strictEqual(fake.calls, 0);
+    });
+  }
+
+  test('CONTROL: a public host still fetches through the gate', async () => {
+    const cap = featureCap('gate-control-cap');
+    const tgzBuf = _fakeTarball(cap);
+    const fake = makeCountingFake([tgzBuf], { 'content-length': String(tgzBuf.length) });
+    _setHttpsGetImpl(fake);
+    const result = await resolveCapabilitySource('https://example.com/gate-control-cap.tgz', {
+      gsdHome,
+      hostVersion: '1.5.0',
+      execOverrides: {
+        tar: (_prog, args) => {
+          if (args[0] === '-tzf') return { exitCode: 0, stdout: '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 capability.json\n', stderr: '', signal: null, error: null };
+          const extractDir = args[args.indexOf('-C') + 1];
+          fs.writeFileSync(path.join(extractDir, 'capability.json'), JSON.stringify(cap), 'utf8');
+          return { exitCode: 0, stdout: '', stderr: '', signal: null, error: null };
+        },
+      },
+    });
+    assert.strictEqual(result.id, 'gate-control-cap');
+    assert.strictEqual(fake.calls, 1, 'the denylist is not an allowlist — public hosts fetch');
+  });
+
+  // Isolated-review Major 1: the fe80::/10 prefix test runs only on IPv6 literals — a
+  // registered domain never contains ':'. A domain-shaped host starting fe8/fe9/fea/feb
+  // (feather.internal, february.example.com) must NOT be denied.
+  test('CONTROL: fe-prefixed DOMAIN hosts are not denied (the v6 branch requires a colon)', async () => {
+    for (const host of ['feather.internal', 'february.example.com', 'fe9cdn.example.com']) {
+      const fake = makeCountingFake([Buffer.from('x')]);
+      _setHttpsGetImpl(fake);
+      await assert.rejects(
+        () => resolveCapabilitySource(`https://${host}/cap.tgz`, { gsdHome, hostVersion: '1.5.0' }),
+        (err) => !/internal host|denylist|loopback/i.test(String(err && err.message)),
+        `${host} is a domain, not an IPv6 literal — it must reach the transport`
+      );
+      assert.strictEqual(fake.calls, 1, `${host} must reach the transport`);
+    }
+  });
+
+  test('CONTROL: a private-range mirror host is NOT denied (internal mirrors are legitimate)', async () => {
+    const fake = makeCountingFake([Buffer.from('x')]);
+    _setHttpsGetImpl(fake);
+    // The flow proceeds PAST the gate into the transport (the body then fails
+    // downstream as a non-tarball — irrelevant here; the assertion is that the
+    // gate itself does not reject a private-range host).
+    await assert.rejects(
+      () => resolveCapabilitySource('https://192.168.1.10/cap.tgz', { gsdHome, hostVersion: '1.5.0' }),
+      (err) => !/internal host|denylist|loopback/i.test(String(err && err.message))
+    );
+    assert.strictEqual(fake.calls, 1, 'a private-range host must reach the transport');
+  });
+});
+
+describe('#3514 F21b — non-https tarball spec fails with a named reason', () => {
+  let gsdHome = '';
+
+  beforeEach(() => { gsdHome = createTempDir('gsd-3514-http-home-'); });
+  afterEach(() => {
+    _setHttpsGetImpl(null);
+    cleanup(gsdHome);
+  });
+
+  test('parseSpec STILL classifies an http:// tarball URL as tarball (no parse-time hard reject)', () => {
+    const p = parseSpec('http://mirror.internal/cap.tgz');
+    assert.strictEqual(p.kind, 'tarball', 'classification is unchanged — the gate lives in the fetcher');
+  });
+
+  test('an http:// tarball spec rejects with the https-only reason, not a raw protocol error', async () => {
+    let calls = 0;
+    const fake = (url, opts, cb) => { calls += 1; return makeFakeHttpsGet([])(url, opts, cb); };
+    _setHttpsGetImpl(fake);
+    await assert.rejects(
+      () => resolveCapabilitySource('http://mirror.internal/cap.tgz', { gsdHome, hostVersion: '1.5.0' }),
+      /requires https|plaintext|internal mirror/i,
+      'the failure must name the https-only transport and the mirror alternative'
+    );
+    assert.strictEqual(calls, 0, 'the transport is never called for a plaintext URL');
+  });
+});
--- a/tests/capability-trust.test.cjs
+++ b/tests/capability-trust.test.cjs
@@ -616,6 +616,88 @@ test('TV-09: signatureForManifest does NOT vary with missingArtifacts (MISSING a
  }
 });

+// ---------------------------------------------------------------------------
+// #3514 (epic #1900 F21c) — unverified-integrity disclosure. The verdict gains
+// integrityPinned; the Disclosure carries integrityStatus ('pinned' |
+// 'unverified'); summarizeDisclosure renders it. Deliberately NOT part of the
+// consent-binding signature — consent's content binding is bundleContentHash
+// (#1459), and a prompt line must never force a spurious re-consent.
+// ---------------------------------------------------------------------------
+
+const EXEC_MANIFEST_3514 = {
+  id: 'integrity-status-cap',
+  hooks: [{ event: 'PostToolUse', script: 'hooks/run.js' }],
+};
+
+test('#3514: integrityPinned:true verdict carries integrityStatus "pinned" and renders the pinned line', () => {
+  const v = trust.evaluateInstallTrust({
+    parsed: { kind: 'tarball', raw: 'https://example.com/cap.tgz', target: 'https://example.com/cap.tgz' },
+    manifest: EXEC_MANIFEST_3514,
+    hostVersion: '1.6.0',
+    integrityPin: 'sha512',
+  });
+  assert.strictEqual(v.disclosure.integrityStatus, 'pinned');
+  const joined = trust.summarizeDisclosure(v.disclosure).join('\n');
+  assert.match(joined, /pin supplied and verified/i);
+  assert.doesNotMatch(joined, /NO PINNED HASH/i);
+});
+
+test('#3514: integrityPinned:false verdict carries integrityStatus "unverified" and renders the unverified line', () => {
+  const v = trust.evaluateInstallTrust({
+    parsed: { kind: 'tarball', raw: 'https://example.com/cap.tgz', target: 'https://example.com/cap.tgz' },
+    manifest: EXEC_MANIFEST_3514,
+    hostVersion: '1.6.0',
+    integrityPin: 'none',
+  });
+  assert.strictEqual(v.disclosure.integrityStatus, 'unverified');
+  const joined = trust.summarizeDisclosure(v.disclosure).join('\n');
+  assert.match(joined, /NO PINNED HASH/i);
+  assert.match(joined, /staged unverified/i);
+});
+
+test('#3514: integrityPin git-commit renders the commit-pinned line, never a sha512 claim', () => {
+  const v = trust.evaluateInstallTrust({
+    parsed: { kind: 'git', raw: 'https://example.com/cap.git#sha:0123456789abcdef0123456789abcdef01234567', target: 'https://example.com/cap.git' },
+    manifest: EXEC_MANIFEST_3514,
+    hostVersion: '1.6.0',
+    integrityPin: 'git-commit',
+  });
+  assert.strictEqual(v.disclosure.integrityStatus, 'commit-pinned');
+  const joined = trust.summarizeDisclosure(v.disclosure).join('\n');
+  assert.match(joined, /pinned to a git commit/i);
+  assert.doesNotMatch(joined, /sha512 pin/i, 'a commit pin must not be reported as a sha512 pin');
+});
+
+test('#3514: legacy callers (no integrityPin) see no integrity line — byte-identical rendering', () => {
+  const base = {
+    parsed: { kind: 'tarball', raw: 'https://example.com/cap.tgz', target: 'https://example.com/cap.tgz' },
+    manifest: EXEC_MANIFEST_3514,
+    hostVersion: '1.6.0',
+  };
+  const v = trust.evaluateInstallTrust(base);
+  assert.strictEqual(v.disclosure.integrityStatus, undefined, 'absent arg ⇒ absent field');
+  const joined = trust.summarizeDisclosure(v.disclosure).join('\n');
+  assert.doesNotMatch(joined, /PINNED|unverified/i, 'no integrity line for legacy callers');
+});
+
+test('#3514: unverified line also renders on the declarative-only disclosure path', () => {
+  const d = trust.discloseExecutableSurfaces({ id: 'x', skills: ['s'] });
+  d.integrityStatus = 'unverified';
+  const joined = trust.summarizeDisclosure(d).join('\n');
+  assert.match(joined, /NO PINNED HASH/i);
+});
+
+test('#3514: integrityStatus NEVER enters the consent signature (executableSetChanged stable)', () => {
+  const d1 = trust.discloseExecutableSurfaces(EXEC_MANIFEST_3514);
+  const d2 = trust.discloseExecutableSurfaces(EXEC_MANIFEST_3514);
+  d2.integrityStatus = 'unverified';
+  assert.strictEqual(
+    trust.executableSetChanged(d1, d2),
+    false,
+    'a prompt-only integrity line must not read as a changed executable set'
+  );
+});
+
 // ---------------------------------------------------------------------------
 // #3515 (epic #1900 F20) — MCP server config is INTENTIONALLY unconfined
 // (unlike hooks); the consent disclosure must say so. Adopted decision: the
--- a/tests/decisions.test.cjs
+++ b/tests/decisions.test.cjs
@@ -29,7 +29,7 @@ const fs = require('fs');
 const path = require('path');

 const { parseDecisions, extractDecisions } = require('../gsd-core/bin/lib/decisions.cjs');
-const { runGsdTools, createTempProject, createTempDir, cleanup } = require('./helpers.cjs');
+const { runGsdTools, createTempProject, cleanup } = require('./helpers.cjs');

 // ─── Regression #1364: markdown-header fallback ───────────────────────────────

@@ -1300,119 +1300,3 @@ describe('check.decision-coverage-plan — empty contextPath argument fails clos
      `Missing contextPath argument must fail closed. Got: ${JSON.stringify(parsed)}`);
  });
 });
-
-// ─── #3484: modified-file truncation bound (MAX_MODIFIED_FILE_BYTES) ─────────
-//
-// readModifiedFilesContent harvests files_modified entries from phase summaries into
-// the decision-coverage-verify haystack, capping each file's content. The bound
-// survived the ADR-0174 SDK collapse as a bare `256 * 1024` literal duplicated in one
-// expression; #3484 restored it as MAX_MODIFIED_FILE_BYTES. These rows pin the bound
-// behaviorally — honored/not_honored flips at exactly 262,144 chars — so the value
-// cannot drift silently again. The constant counts String#length (UTF-16 code units),
-// not bytes; semantics preserved from the retired lineage.
-//
-// Fixture facts these rows depend on (verified against the compiled CLI):
-//   - files_modified entries resolve against the PROJECT root (cwd), not phaseDir;
-//   - decisionMentioned matches \bD-99\b, so the id needs a non-word char before it.
-
-describe('check.decision-coverage-verify — modified-file truncation bound (#3484)', () => {
-  const CAP = 256 * 1024;
-  let tmpDir;
-  let phaseDir;
-
-  beforeEach(() => {
-    tmpDir = createTempProject('gsd-3484-');
-    phaseDir = path.join(tmpDir, '.planning', 'phases', '01-init');
-    fs.mkdirSync(phaseDir, { recursive: true });
-  });
-
-  afterEach(() => cleanup(tmpDir));
-
-  function writeVerifyFixture(modifiedFileContent) {
-    writeGateFiles(['# Summary', '', 'files_modified:', '- big-modified.txt', '', 'Wrapped up the widget work.']);
-    fs.writeFileSync(path.join(tmpDir, 'big-modified.txt'), modifiedFileContent);
-    return path.join(phaseDir, 'CONTEXT.md');
-  }
-
-  function writeGateFiles(summaryLines) {
-    writeContextFile(phaseDir, [
-      '# Phase Context',
-      '',
-      '<decisions>',
-      '- **D-99:** keep the marker token unique to this fixture',
-      '</decisions>',
-    ].join('\n'));
-    writePlanFile(phaseDir, '01', '# Plan\n\n## Must Haves\n\n- deliver the widget\n');
-    fs.writeFileSync(path.join(phaseDir, '01-SUMMARY.md'), summaryLines.join('\n') + '\n');
-  }
-
-  function runVerify(contextPath) {
-    const result = runGsdTools(
-      ['query', 'check.decision-coverage-verify', phaseDir, contextPath],
-      tmpDir
-    );
-    return JSON.parse(result.output || '{}');
-  }
-
-  test('id ending at the last included char is kept (limit)', () => {
-    // Length exactly CAP: `raw.length > CAP` is false → content passes through whole,
-    // so a D-99 whose last char sits at index CAP-1 is honored. Pins > vs >=.
-    const contextPath = writeVerifyFixture('x'.repeat(CAP - 5) + ' D-99');
-    const parsed = runVerify(contextPath);
-    assert.strictEqual(parsed.honored, 1,
-      `D-99 ending at index CAP-1 must be honored. Got: ${JSON.stringify(parsed)}`);
-  });
-
-  test('id beyond the cap is truncated away (limit+1)', () => {
-    // Length CAP+4 with D-99 starting AT index CAP: slice(0, CAP) drops it entirely.
-    const contextPath = writeVerifyFixture('x'.repeat(CAP - 1) + ' D-99');
-    const parsed = runVerify(contextPath);
-    assert.strictEqual(parsed.honored, 0,
-      `D-99 starting at index CAP must be truncated away. Got: ${JSON.stringify(parsed)}`);
-    assert.ok(
-      (parsed.not_honored || []).some((item) => item.id === 'D-99'),
-      `D-99 must land in not_honored. Got: ${JSON.stringify(parsed)}`
-    );
-  });
-
-  test('id within a small modified file is honored (happy)', () => {
-    const contextPath = writeVerifyFixture('D-99 ' + 'y'.repeat(64));
-    const parsed = runVerify(contextPath);
-    assert.strictEqual(parsed.honored, 1,
-      `D-99 in a small modified file must be honored. Got: ${JSON.stringify(parsed)}`);
-  });
-
-  test('files_modified entry outside the root is skipped', (t) => {
-    // A sibling dir (outside the project root) holding a file that WOULD satisfy the
-    // decision — proving isInsideRoot skipped it, as distinct from a missing file.
-    const sibling = createTempDir('gsd-3484-escape-');
-    t.after(() => cleanup(sibling));
-    fs.writeFileSync(path.join(sibling, 'escape.txt'), 'D-99 '.repeat(10));
-
-    writeGateFiles(['# Summary', '', 'files_modified:', `- ../${path.basename(sibling)}/escape.txt`, '']);
-
-    const parsed = runVerify(path.join(phaseDir, 'CONTEXT.md'));
-    assert.strictEqual(parsed.honored, 0,
-      `An out-of-root files_modified entry must be skipped, not harvested. Got: ${JSON.stringify(parsed)}`);
-    assert.ok(
-      (parsed.not_honored || []).some((item) => item.id === 'D-99'),
-      'D-99 must be reported not honored — the only D-99 lives outside the root'
-    );
-  });
-
-  test('unreadable files_modified entry does not crash the gate', () => {
-    writeGateFiles(['# Summary', '', 'files_modified:', '- does-not-exist.txt', '']);
-
-    const parsed = runVerify(path.join(phaseDir, 'CONTEXT.md'));
-    assert.strictEqual(parsed.honored, 0,
-      `A missing file must harvest empty content, not crash. Got: ${JSON.stringify(parsed)}`);
-  });
-
-  test('summary without files_modified yields no haystack content', () => {
-    writeGateFiles(['# Summary', '', 'No file list here.']);
-
-    const parsed = runVerify(path.join(phaseDir, 'CONTEXT.md'));
-    assert.strictEqual(parsed.honored, 0,
-      `No files_modified block means D-99 cannot be honored. Got: ${JSON.stringify(parsed)}`);
-  });
-});