* 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>
This commit is contained in:
Tom Boucher
5
.changeset/nimble-orcas-tumble.md
Normal file
5
.changeset/nimble-orcas-tumble.md
Normal file
@@ -0,0 +1,5 @@
|
|||||||
|
---
|
||||||
|
type: Security
|
||||||
|
pr: 3516
|
||||||
|
---
|
||||||
|
**Capability installs no longer fetch from internal hosts, and unpinned installs say so in the consent prompt** — the URL importer refuses loopback/link-local/metadata hosts (including the cloud metadata addresses and localhost) before any bytes leave, and an http:// tarball URL fails with a clear https-only reason instead of a raw protocol error. Installs without an integrity pin now show a distinct 'NO PINNED HASH — staged unverified' line in the consent disclosure. (#3514)
|
||||||
73
.pr-body-3514.md
Normal file
73
.pr-body-3514.md
Normal file
@@ -0,0 +1,73 @@
|
|||||||
|
## Fix PR
|
||||||
|
|
||||||
|
> **Using the wrong template?**
|
||||||
|
> — Enhancement: use [enhancement.md](?template=enhancement.md)
|
||||||
|
> — Feature: use [feature.md](?template=feature.md)
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Linked Issue
|
||||||
|
|
||||||
|
> **Required.** This PR will be auto-closed if no valid issue link is found.
|
||||||
|
|
||||||
|
Fixes #3514
|
||||||
|
|
||||||
|
> The linked issue must have the `confirmed-bug` label. If it doesn't, ask a maintainer to confirm the bug before continuing.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## What was broken
|
||||||
|
|
||||||
|
Three hardening gaps at the ADR-1244 D3/D5 edges (epic #1900, finding F21): the capability URL-import fetcher never validated the resolved host (a loopback or cloud-metadata **https** endpoint was fetched like any URL); an `http://` tarball spec failed with a raw `ERR_INVALID_PROTOCOL` instead of a named reason; and an install without an integrity pin produced a consent prompt that did not distinguish verified from unverified content.
|
||||||
|
|
||||||
|
## What this fix does
|
||||||
|
|
||||||
|
- **Pre-transport fetch gate** (`realHttpsGet` → `assertFetchableUrl`): loopback/link-local/metadata/unspecified hosts (`127/8`, `169.254/16` incl. `169.254.169.254`, `0/8`, `::1`, `fe80::/10`, `::`, IPv4-mapped spellings in both dotted and hex-normalized forms) and `localhost`/`*.localhost` names are denied with a named error **before any bytes leave** — the injected transport is provably never invoked (tests assert call-count 0). The WHATWG URL parser normalizes alternate IP spellings (decimal `2130706433`, hex `0x7f000001`, octal, short forms) to dotted-quad before the gate sees them — locked by tests.
|
||||||
|
- **Non-https, per the adopted split decision**: `parseSpec` still *classifies* `http://` tarball specs (internal-mirror flows are not broken at parse); the fetcher refuses with a clear https-only reason naming the mirror alternative.
|
||||||
|
- **Unverified-integrity disclosure**: `evaluateInstallTrust` accepts `integrityPinned` (a verified `--integrity` pin, or a git `#sha:<commit>` ref); the consent prompt renders `content: NO PINNED HASH — staged unverified` when no pin was supplied (and the pinned counterpart when one was). The status is **prompt-only** — deliberately excluded from `disclosureSignature` (consent's content binding is `bundleContentHash`, #1459; tests lock that it can never fire a spurious re-consent). Legacy callers see byte-identical output.
|
||||||
|
|
||||||
|
## Root cause
|
||||||
|
|
||||||
|
The D3 fetcher was built with a byte cap and a timeout but no host policy — the finding is an edge the pipeline's safe posture simply wasn't applied to. The integrity line was missing because the disclosure was built from the manifest alone; the pin fact lives on the resolve path, and nothing threaded it into the verdict.
|
||||||
|
|
||||||
|
## Testing
|
||||||
|
|
||||||
|
### How I verified the fix
|
||||||
|
|
||||||
|
- Failing-first: `gsd-test` at the tests-only commit (holodeck, linux-node24) — verdict `failed` with exactly the 20 expected failures (12-case denylist matrix, https-reason, trust/lifecycle integrity suites), 34,343 green.
|
||||||
|
- GREEN: full `gsd-test` matrix at the final HEAD — verdict below.
|
||||||
|
- Controls lock the negative space: a public host and a private-range mirror still fetch through the gate; `parseSpec` still classifies `http://`; legacy `summarizeDisclosure` output is line-identical.
|
||||||
|
|
||||||
|
### Regression test added?
|
||||||
|
|
||||||
|
- [x] Yes — added a test that would have caught this bug
|
||||||
|
|
||||||
|
### Platforms tested
|
||||||
|
|
||||||
|
- [x] macOS
|
||||||
|
- [ ] Windows (including backslash path handling)
|
||||||
|
- [x] Linux
|
||||||
|
|
||||||
|
### Runtimes tested
|
||||||
|
|
||||||
|
- [ ] Claude Code
|
||||||
|
- [ ] Gemini CLI
|
||||||
|
- [ ] OpenCode
|
||||||
|
- [ ] Other: ___
|
||||||
|
- [x] N/A (not runtime-specific — engine-internal source resolver + trust gate)
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Checklist
|
||||||
|
|
||||||
|
- [x] Issue linked above with `Fixes #3514` — **PR will be auto-closed if missing**
|
||||||
|
- [x] Linked issue has the `confirmed-bug` label
|
||||||
|
- [x] Fix is scoped to the reported bug — no unrelated changes included
|
||||||
|
- [x] Regression test added (or explained why not)
|
||||||
|
- [x] All existing tests pass (`npm test`) — full `gsd-test` matrix at final HEAD
|
||||||
|
- [x] `.changeset/` fragment added — `Security` type
|
||||||
|
- [x] No unnecessary dependencies added
|
||||||
|
|
||||||
|
## Breaking changes
|
||||||
|
|
||||||
|
None intended. `http://` tarball URLs already failed at the transport; they now fail with a better message. Denied hosts (loopback/link-local/metadata/localhost) are not legitimate install sources. **Known limits, documented** in `docs/explanation/capability-trust-model.md`: RFC1918 private ranges are deliberately *not* denied (internal mirrors); the check is on the URL's host literal — DNS rebinding is out of scope. Local/unpinned-git installs render the unverified line even though their trust basis is the path/commit (honest, slightly noisy).
|
||||||
File diff suppressed because one or more lines are too long
@@ -198,52 +198,6 @@ Contributor requirements (summary):
|
|||||||
- **CI must pass** — all configured matrix jobs must be green. Node 24 is the compatibility floor and primary target; Node 26 compatibility must be preserved for code and tests even when a Node 26 CI lane is not yet available.
|
- **CI must pass** — all configured matrix jobs must be green. Node 24 is the compatibility floor and primary target; Node 26 compatibility must be preserved for code and tests even when a Node 26 CI lane is not yet available.
|
||||||
- **Scope matches the approved issue** — if your PR does more than what the issue describes, the extra changes will be asked to be removed or moved to a new issue
|
- **Scope matches the approved issue** — if your PR does more than what the issue describes, the extra changes will be asked to be removed or moved to a new issue
|
||||||
|
|
||||||
### Deleting a module that has a surviving counterpart — disposition required
|
|
||||||
|
|
||||||
A consolidation PR that deletes a module, package, or whole lineage — where any part of
|
|
||||||
the deleted side has a surviving counterpart in the tree — must carry a **symbol-level
|
|
||||||
disposition table for the deleted side**: for every declared name (exported *or* local),
|
|
||||||
one of:
|
|
||||||
|
|
||||||
- `migrated` — the behavior exists at a named location in the surviving tree,
|
|
||||||
- `renamed → <new name>` — including convention drift (`verifyX` → `cmdVerifyX`,
|
|
||||||
`preserveExistingProgress` → `shouldPreserveExistingProgress`),
|
|
||||||
- `dropped because <reason>` — a deliberate deletion, with the reason stated.
|
|
||||||
|
|
||||||
Rank **SCREAMING_CASE constants first** — that is where policy lives; both confirmed
|
|
||||||
losses from the one retirement this repo has performed were bounds
|
|
||||||
([#3484](https://github.com/open-gsd/gsd-core/issues/3484)). Local-variable renames are
|
|
||||||
noise and may be summarized.
|
|
||||||
|
|
||||||
**"The surviving tests pass" is not evidence.** The surviving tests belong to the
|
|
||||||
surviving side and cannot observe behavior only the deleted side had. ADR-0174 retired
|
|
||||||
the SDK package and deleted ADR-3524's parity apparatus in the same migration; three
|
|
||||||
invariants went into the bin with the package and nobody noticed for months (#3427,
|
|
||||||
#3477, epic #3473 B5) — see ADR-0174's behavior-carry-forward amendment (#3484) for the record.
|
|
||||||
|
|
||||||
**The audit is a method, not a guess.** The calibrated procedure — pairing by basename,
|
|
||||||
comment-stripped declared-name diffing, rename normalization, SCREAMING_CASE-first
|
|
||||||
ranking, and a **mandatory positive control** (the audit must re-detect a known loss or
|
|
||||||
it is not calibrated; an export-only scan misses `shortFormToId`, which was a *local*
|
|
||||||
`const` in a surviving function) — is written up step by step in
|
|
||||||
[`docs/how-to/audit-a-retiring-lineage.md`](docs/how-to/audit-a-retiring-lineage.md).
|
|
||||||
|
|
||||||
### A known gap gets an issue — a comment is not a tracking mechanism
|
|
||||||
|
|
||||||
Recording a known-but-unfixed gap in a code comment, a changeset note, or a TODO, and
|
|
||||||
treating that as *tracking* it, is an anti-pattern: all three are attached to code, and
|
|
||||||
a refactor deletes the code they annotate while the gap stays real. The worked example
|
|
||||||
is `.changeset/archived/clever-yaks-cheer.md` (PR #3798): it accurately recorded the
|
|
||||||
`shortFormToId` backfill as "tracked as a follow-up parity gap", a `// KNOWN GAP:`
|
|
||||||
comment marked the call site, then the SDK retirement deleted both — and no issue was
|
|
||||||
ever opened. The gap surfaced a year later as production bug #3427.
|
|
||||||
|
|
||||||
The rule: **a gap that matters gets an issue. A gap with no issue is not tracked.** If
|
|
||||||
you find yourself writing "tracked as a follow-up" in a comment or a changeset body,
|
|
||||||
open the issue first, then cite its number. This is the prose-level twin of epic #3473's
|
|
||||||
criterion B6 (a guard needs a retirement condition): the tracking mechanism must
|
|
||||||
survive the artifact it is attached to.
|
|
||||||
|
|
||||||
## CHANGELOG Entries — Drop a Fragment
|
## CHANGELOG Entries — Drop a Fragment
|
||||||
|
|
||||||
**Do not edit `CHANGELOG.md` directly.** Two PRs that both append to a `### Fixed` block always conflict on merge — git can't pick a serialization order without a human. Instead, every PR with user-facing changes drops a fragment file in `.changeset/`.
|
**Do not edit `CHANGELOG.md` directly.** Two PRs that both append to a `### Fixed` block always conflict on merge — git can't pick a serialization order without a human. Instead, every PR with user-facing changes drops a fragment file in `.changeset/`.
|
||||||
|
|||||||
@@ -1,6 +1,6 @@
|
|||||||
{
|
{
|
||||||
"schemaVersion": 1,
|
"schemaVersion": 1,
|
||||||
"count": 263,
|
"count": 261,
|
||||||
"classes": {
|
"classes": {
|
||||||
"ARCH": 1,
|
"ARCH": 1,
|
||||||
"CI": 2,
|
"CI": 2,
|
||||||
@@ -17,7 +17,7 @@
|
|||||||
"PROC": 14,
|
"PROC": 14,
|
||||||
"PROHIB": 10,
|
"PROHIB": 10,
|
||||||
"RELEASE-NOTES": 31,
|
"RELEASE-NOTES": 31,
|
||||||
"RULESET": 60,
|
"RULESET": 58,
|
||||||
"SESSION": 9,
|
"SESSION": 9,
|
||||||
"WAVE": 5,
|
"WAVE": 5,
|
||||||
"WORKSTREAM": 5,
|
"WORKSTREAM": 5,
|
||||||
@@ -979,16 +979,6 @@
|
|||||||
"klass": "RULESET",
|
"klass": "RULESET",
|
||||||
"value": "requires confirmed-bug before implementation (legacy 'confirmed' label is back-compat only for duplicate-sweep exemption, not a valid implementation gate)"
|
"value": "requires confirmed-bug before implementation (legacy 'confirmed' label is back-compat only for duplicate-sweep exemption, not a valid implementation gate)"
|
||||||
},
|
},
|
||||||
{
|
|
||||||
"id": "RULESET.CONTRIB.GAP-TRACKING",
|
|
||||||
"klass": "RULESET",
|
|
||||||
"value": "a known gap is tracked only by an issue — a code comment, TODO, or changeset note is attached to code and dies with the refactor that deletes it, while the gap stays real (worked example: archived changeset clever-yaks-cheer.md recorded the shortFormToId backfill as 'tracked as a follow-up parity gap' + a // KNOWN GAP: comment marked the call site; the SDK retirement deleted both, no issue was opened, and it surfaced a year later as #3427). If you write 'tracked as a follow-up' anywhere, open the issue first and cite its number. Prose twin of #3473 criterion B6 (a guard needs a retirement condition)"
|
|
||||||
},
|
|
||||||
{
|
|
||||||
"id": "RULESET.CONTRIB.GATE.DELETED-MODULE-DISPOSITION",
|
|
||||||
"klass": "RULESET",
|
|
||||||
"value": "a PR deleting a module/package/lineage with a surviving counterpart carries a symbol-level disposition table for the deleted side — every declared name (exported or local) marked migrated (location named) | renamed → X (conventions normalized: verifyX → cmdVerifyX) | dropped because Y; SCREAMING_CASE constants ranked first (policy lives there; both confirmed ADR-0174 losses were bounds); the surviving tests passing is NOT evidence — the surviving tests belong to the surviving side (#3484, ADR-0174 behavior-carry-forward amendment; losses: #3427 shortFormToId tier + unresolved-deps warning, #3477 regexForKeyLinkPattern 512-char cap + nested-quantifier screen, epic #3473 B5 MAX_JSON_SEARCH_DEPTH=48). Method with mandatory positive control: docs/how-to/audit-a-retiring-lineage.md"
|
|
||||||
},
|
|
||||||
{
|
{
|
||||||
"id": "RULESET.CONTRIB.GATE.ORDER",
|
"id": "RULESET.CONTRIB.GATE.ORDER",
|
||||||
"klass": "RULESET",
|
"klass": "RULESET",
|
||||||
|
|||||||
@@ -24,7 +24,6 @@ Language versions: [English](README.md) · [Português (pt-BR)](pt-BR/README.md)
|
|||||||
- [Resolve edge-coverage findings](how-to/resolve-edge-coverage-findings.md) — turn the spec phase's surfaced domain-boundary edges into covered, dismissed, or backstopped spec decisions
|
- [Resolve edge-coverage findings](how-to/resolve-edge-coverage-findings.md) — turn the spec phase's surfaced domain-boundary edges into covered, dismissed, or backstopped spec decisions
|
||||||
- [Resolve prohibition findings](how-to/resolve-prohibition-findings.md) — turn the spec phase's surfaced must-NOT constraints into resolved, dismissed, or deferred spec decisions
|
- [Resolve prohibition findings](how-to/resolve-prohibition-findings.md) — turn the spec phase's surfaced must-NOT constraints into resolved, dismissed, or deferred spec decisions
|
||||||
- [Resolve an ESLint glob-coverage finding](how-to/resolve-eslint-coverage-findings.md) — bring a source file that matches no lint rule under coverage, or record a reasoned exemption
|
- [Resolve an ESLint glob-coverage finding](how-to/resolve-eslint-coverage-findings.md) — bring a source file that matches no lint rule under coverage, or record a reasoned exemption
|
||||||
- [Audit a retiring lineage](how-to/audit-a-retiring-lineage.md) — produce the symbol-level disposition table a module-deleting consolidation PR must carry, with a mandatory positive control
|
|
||||||
- [Plan a phase](how-to/plan-a-phase.md) — run research, decompose work, and verify plan quality
|
- [Plan a phase](how-to/plan-a-phase.md) — run research, decompose work, and verify plan quality
|
||||||
- [Execute a phase](how-to/execute-a-phase.md) — run plans in parallel waves with fresh-context subagents
|
- [Execute a phase](how-to/execute-a-phase.md) — run plans in parallel waves with fresh-context subagents
|
||||||
- [Verify and ship](how-to/verify-and-ship.md) — walk through completed work, diagnose failures, and create the PR
|
- [Verify and ship](how-to/verify-and-ship.md) — walk through completed work, diagnose failures, and create the PR
|
||||||
|
|||||||
@@ -1,6 +1,6 @@
|
|||||||
# ADR-0174: Retire @opengsd/gsd-sdk package boundary — single-runtime collapse
|
# ADR-0174: Retire @opengsd/gsd-sdk package boundary — single-runtime collapse
|
||||||
|
|
||||||
- **Status:** Accepted (2026-05-23); amended #1642 (2026-06-23) — §5 reconciled to as-built Result type + `exitReason?` field added on `InvalidArgs`; amended #3484 (2026-08-14) — behavior-carry-forward amendment appended
|
- **Status:** Accepted (2026-05-23); amended #1642 (2026-06-23) — §5 reconciled to as-built Result type + `exitReason?` field added on `InvalidArgs`
|
||||||
- **Date:** 2026-05-23
|
- **Date:** 2026-05-23
|
||||||
- **Tracking issue:** [#174](https://github.com/open-gsd/get-shit-done-redux/issues/174) — sub-issues #175–#197
|
- **Tracking issue:** [#174](https://github.com/open-gsd/get-shit-done-redux/issues/174) — sub-issues #175–#197
|
||||||
|
|
||||||
@@ -161,46 +161,3 @@ Seven phases, ~15–18 PRs total. Each phase is a coherent slice that leaves the
|
|||||||
| 7 — Land this ADR's PR | The PR for this ADR closes the umbrella tracking issue. | 1 (this PR) |
|
| 7 — Land this ADR's PR | The PR for this ADR closes the umbrella tracking issue. | 1 (this PR) |
|
||||||
|
|
||||||
Implementation is tracked in [#174 — sub-issues #175–#197](https://github.com/open-gsd/get-shit-done-redux/issues/174).
|
Implementation is tracked in [#174 — sub-issues #175–#197](https://github.com/open-gsd/get-shit-done-redux/issues/174).
|
||||||
|
|
||||||
## Amendment (2026-08-14): Behavior carry-forward for lineage-retiring consolidations (#3484)
|
|
||||||
|
|
||||||
This amendment was appended after the migration completed. It changes nothing about the
|
|
||||||
collapse decision above — one runtime still cannot drift from itself, and ADR-3524's
|
|
||||||
parity apparatus is still correctly gone. What this ADR lacked was a clause about the
|
|
||||||
*instant of collapse*: the two lineages were not equivalent at the moment `sdk/` was
|
|
||||||
deleted (drift had been accumulating for months — ADR-3524 itself names nine prior
|
|
||||||
one-sided fixes), and the collapse took the CJS lineage as canonical. Everything the SDK
|
|
||||||
side had and the CJS side did not was deleted with the package — and the parity tests
|
|
||||||
that would have caught exactly that were deleted by Migration Plan Phase 3 in the same
|
|
||||||
motion. The surviving tests were the CJS side's tests, and they passed throughout.
|
|
||||||
|
|
||||||
**The requirement.** A consolidation that retires a lineage must enumerate the retiring
|
|
||||||
side's behavior and record, per item, whether it was **`migrated`**, **`renamed → X`**,
|
|
||||||
or **`dropped because Y`** — before the deletion merges. "The tests pass" is explicitly
|
|
||||||
not sufficient evidence, because the surviving tests belong to the surviving side and
|
|
||||||
cannot observe behavior only the deleted side had.
|
|
||||||
|
|
||||||
**The evidence that produced this amendment — three confirmed losses through the gap:**
|
|
||||||
|
|
||||||
| Lost in the collapse | Surviving site | Consequence |
|
|
||||||
|---|---|---|
|
|
||||||
| `shortFormToId` resolution tier + the `unresolved depends_on reference` warning | `src/phase.cts` | [#3427](https://github.com/open-gsd/gsd-core/issues/3427) — short-form `depends_on` edges silently dropped; dependency-ordered plans execute in parallel |
|
|
||||||
| `regexForKeyLinkPattern`'s 512-char cap + nested-quantifier screen | `src/verify.cts` | [#3477](https://github.com/open-gsd/gsd-core/issues/3477) — ReDoS; `verify-phase` hangs on an untrusted plan pattern |
|
|
||||||
| `MAX_JSON_SEARCH_DEPTH = 48` | `src/intel.cts` | unbounded recursion in `matchesInValue`; stack overflow on deeply nested intel JSON (carried by epic #3473, B5) |
|
|
||||||
|
|
||||||
None was noticed for months; #3427 surfaced only when a user hit it in production. The
|
|
||||||
`shortFormToId` loss is the sharpest: the gap was *known* — recorded as "tracked as a
|
|
||||||
follow-up parity gap" in an archived changeset (`.changeset/archived/clever-yaks-cheer.md`)
|
|
||||||
and marked with a `// KNOWN GAP:` comment — then the SDK was retired, the comment died
|
|
||||||
with the lineage it annotated, and no issue was ever opened. A comment is attached to
|
|
||||||
code; it is not a tracking mechanism. A gap that matters gets an issue.
|
|
||||||
|
|
||||||
A fourth-loss audit run on 2026-08-14 (method recorded in [#3484](https://github.com/open-gsd/gsd-core/issues/3484)
|
|
||||||
and in the contributor how-to) found no further losses — which calibrates the audit
|
|
||||||
method, not a claim that the class is closed.
|
|
||||||
|
|
||||||
**Where the rule lives now.** The general form of this requirement — not scoped to this
|
|
||||||
ADR — is a merge gate in `CONTRIBUTING.md` ("Deleting a module that has a surviving
|
|
||||||
counterpart"): the deleting PR carries a symbol-level disposition table, SCREAMING_CASE
|
|
||||||
constants ranked first, with a mandatory positive control. Future lineage-retiring
|
|
||||||
consolidations cite this amendment and satisfy that gate.
|
|
||||||
|
|||||||
@@ -288,6 +288,37 @@ An `integrity` field in `capability.json` carries a `sha512-<base64>` digest
|
|||||||
of the capability bundle. When present, GSD verifies this digest before
|
of the capability bundle. When present, GSD verifies this digest before
|
||||||
extracting any files. A mismatch aborts the install.
|
extracting any files. A mismatch aborts the install.
|
||||||
|
|
||||||
|
When NO pin is supplied, the consent prompt says so plainly: a
|
||||||
|
`content: NO PINNED HASH — staged unverified` line distinguishes an install
|
||||||
|
whose bytes were verified against a commitment from one that was not
|
||||||
|
([#3514](https://github.com/open-gsd/gsd-core/issues/3514)). A computed
|
||||||
|
sha512 of what was actually fetched is still recorded in the ledger at
|
||||||
|
install, so a later `trust` inspection shows exactly which bytes landed.
|
||||||
|
Prompt claims are exact per kind: a sha512 `--integrity` pin renders as
|
||||||
|
*supplied and verified*, a git source checked out at a `#sha:<commit>` ref
|
||||||
|
renders as *pinned to a git commit* (never as a sha512 pin — none was
|
||||||
|
supplied), and a mutable `#sha:<branch>` ref is not a pin at all.
|
||||||
|
|
||||||
|
### Fetch-host denylist
|
||||||
|
|
||||||
|
The URL importer's fetch transport refuses, before any bytes leave
|
||||||
|
([#3514](https://github.com/open-gsd/gsd-core/issues/3514)):
|
||||||
|
|
||||||
|
- **loopback, link-local, and unspecified hosts** — `127.0.0.0/8`,
|
||||||
|
`169.254.0.0/16` (which contains the cloud metadata addresses), `0.0.0.0/8`,
|
||||||
|
`::1`, `fe80::/10`, `::`, their IPv4-mapped IPv6 spellings, and
|
||||||
|
`localhost`/`*.localhost` names. No legitimate capability install fetches
|
||||||
|
these.
|
||||||
|
- **plaintext `http://` URLs** — the transport is `https`-only; an `http://`
|
||||||
|
tarball spec still *classifies* (so an internal-mirror workflow fails with a
|
||||||
|
clear, named reason instead of a raw protocol error) but never fetches.
|
||||||
|
|
||||||
|
Deliberate limits: RFC1918 private ranges (`10/8`, `172.16/12`,
|
||||||
|
`192.168/16`) are **not** denied — an internal https mirror is a legitimate
|
||||||
|
install source, and the denylist is not an allowlist. The check is on the
|
||||||
|
URL's host literal; a public hostname that *resolves* via DNS to a denied
|
||||||
|
range (rebinding) is out of scope.
|
||||||
|
|
||||||
What integrity pinning defends against: a capability hosted at a URL or in a
|
What integrity pinning defends against: a capability hosted at a URL or in a
|
||||||
registry that is later replaced with a different bundle (whether by an attacker
|
registry that is later replaced with a different bundle (whether by an attacker
|
||||||
who has compromised the hosting, or by an author publishing a silent breaking
|
who has compromised the hosting, or by an author publishing a silent breaking
|
||||||
|
|||||||
@@ -1,96 +0,0 @@
|
|||||||
# Audit a retiring lineage (behavior carry-forward)
|
|
||||||
|
|
||||||
**You need this if** your PR deletes a module, package, or runtime lineage — and any part
|
|
||||||
of the deleted side has a surviving counterpart in the tree. The merge gate
|
|
||||||
([CONTRIBUTING.md → "Deleting a module that has a surviving counterpart"](../../CONTRIBUTING.md))
|
|
||||||
requires the PR to carry a symbol-level disposition table. This page is the calibrated
|
|
||||||
method for producing that table. It was first run against the ADR-0174 SDK retirement
|
|
||||||
and found three confirmed losses after the fact (#3484); run it *before* the deletion
|
|
||||||
merges and those losses become review findings instead of production bugs.
|
|
||||||
|
|
||||||
## Why "the tests pass" proves nothing here
|
|
||||||
|
|
||||||
The surviving tests belong to the surviving side. They were written against the
|
|
||||||
surviving implementation and pass before, during, and after the deletion — including
|
|
||||||
when the deleted side carried an invariant the survivor never had. ADR-3524 existed
|
|
||||||
precisely because the two sides had already drifted in nine known places; deleting one
|
|
||||||
side without enumerating it is choosing the survivor's omissions by default.
|
|
||||||
|
|
||||||
## The audit, step by step
|
|
||||||
|
|
||||||
All commands run against `<parent>` — the last commit **before** the retirement (for the
|
|
||||||
SDK retirement this was `04b3be683`, the parent of the retiring commit).
|
|
||||||
|
|
||||||
### 1. List the deleted source files
|
|
||||||
|
|
||||||
```bash
|
|
||||||
git ls-tree -r --name-only <parent> -- sdk/src
|
|
||||||
```
|
|
||||||
|
|
||||||
172 files for the SDK run. Everything under the deleted root is a candidate; test files
|
|
||||||
are excluded from pairing (they are the deleted side's claims, not its behavior).
|
|
||||||
|
|
||||||
### 2. Pair each file with a surviving counterpart
|
|
||||||
|
|
||||||
Pair by **basename**: `sdk/src/query/phase.ts` ↔ `src/phase.cts`. For the SDK run this
|
|
||||||
yielded 22 paired modules. Files with **no** counterpart are whole surfaces deleted on
|
|
||||||
purpose — list them in the disposition as one `dropped because …` line each (or grouped
|
|
||||||
by surface), but they are out of scope for symbol diffing.
|
|
||||||
|
|
||||||
### 3. Diff *declared* names, not raw tokens
|
|
||||||
|
|
||||||
Strip comments from both sides, then diff the declared names (functions, constants,
|
|
||||||
types — exported or local). **Do not tokenize the raw text**: raw token diffing matches
|
|
||||||
prose in comments and produced ~500 false positives on the first SDK run.
|
|
||||||
|
|
||||||
### 4. Normalize rename conventions before reporting
|
|
||||||
|
|
||||||
The two lineages used different naming conventions. Normalize before diffing:
|
|
||||||
|
|
||||||
| Deleted-side name | Surviving-side convention |
|
|
||||||
|---|---|
|
|
||||||
| `verifyX` | `cmdVerifyX` |
|
|
||||||
| `preserveExistingProgress` | `shouldPreserveExistingProgress` |
|
|
||||||
|
|
||||||
Skipping this step is what makes the raw diff unreadable — every convention-renamed
|
|
||||||
symbol reads as a loss.
|
|
||||||
|
|
||||||
### 5. Rank what remains by shape
|
|
||||||
|
|
||||||
**SCREAMING_CASE constants are the high-signal class.** That is where policy lives
|
|
||||||
(bounds, limits, tiers, thresholds), and both real losses the SDK audit confirmed were
|
|
||||||
bounds (`regexForKeyLinkPattern`'s 512-char cap; `MAX_JSON_SEARCH_DEPTH = 48`). Local
|
|
||||||
variable renames are noise — summarize them.
|
|
||||||
|
|
||||||
### 6. Positive control — mandatory
|
|
||||||
|
|
||||||
Before trusting the audit, verify it **re-detects a known loss**. For the SDK lineage the
|
|
||||||
control is `shortFormToId` — a *local* `const` inside a surviving function at
|
|
||||||
`sdk/src/query/phase.ts:609`, not an export. An export-only scan misses it entirely and
|
|
||||||
reports a clean bill of health; a calibrated run finds both halves of #3427 (the tier
|
|
||||||
`shortFormToId` and the diagnostic `unresolvedDeps`). If your audit method cannot find
|
|
||||||
the control, it is not calibrated — fix the method, do not ship the table.
|
|
||||||
|
|
||||||
### 7. Write the disposition table into the PR
|
|
||||||
|
|
||||||
One row per name that survived steps 3–5: `migrated` (name the location), `renamed → X`,
|
|
||||||
or `dropped because Y`. Unpaired whole surfaces get their own grouped `dropped because`
|
|
||||||
rows. The table goes in the PR body where the reviewer of the deletion will see it.
|
|
||||||
|
|
||||||
## Reading the result honestly
|
|
||||||
|
|
||||||
- **No losses found** means the audit found none — with the method calibrated by the
|
|
||||||
positive control. It is evidence, not proof; say which control you ran.
|
|
||||||
- **A "migrated" claim you cannot point at** is a loss wearing a disposition. Every
|
|
||||||
`migrated` row names a file or symbol in the surviving tree.
|
|
||||||
- The audit is run **once per retirement**, against that retirement's parent commit —
|
|
||||||
it is not a standing CI job. Its value is the moment before the deletion merges.
|
|
||||||
|
|
||||||
## Reason codes at a glance
|
|
||||||
|
|
||||||
| Code | Meaning |
|
|
||||||
|---|---|
|
|
||||||
| `migrated` | Behavior exists at a named location in the surviving tree |
|
|
||||||
| `renamed → X` | Same behavior under the surviving convention's name |
|
|
||||||
| `dropped because Y` | Deliberate deletion, reason stated |
|
|
||||||
| *(nothing — no issue)* | Not tracked. A gap that matters gets an issue; a comment or changeset note dies with the code it annotates (`clever-yaks-cheer.md` → #3427) |
|
|
||||||
File diff suppressed because one or more lines are too long
@@ -202,6 +202,31 @@ interface LifecycleOptions {
|
|||||||
/** Stamp written onto every capability-owned shared-config entry, for surgical removal. */
|
/** Stamp written onto every capability-owned shared-config entry, for surgical removal. */
|
||||||
const CAP_MARKER = '_gsdCapability';
|
const CAP_MARKER = '_gsdCapability';
|
||||||
|
|
||||||
|
/**
|
||||||
|
* #3514 (epic #1900 F21c): what KIND of pin (if any) the source content was pinned with — shared
|
||||||
|
* by the install and upgrade verdicts so the two cannot drift. Reaching the verdict with a
|
||||||
|
* supplied `--integrity` pin means the pin VERIFIED (the resolver throws on mismatch). A git
|
||||||
|
* `#sha:<40-hex-commit>` ref is the git analog of a hash pin — a commit checkout, verified by
|
||||||
|
* spelling: `/^sha:[0-9a-f]{7,40}$/i` accepts only a hex commit id, so a mutable ref
|
||||||
|
* (`#sha:main`, `#sha:v1`) is NOT counted as a pin (isolated review finding — a moving ref must
|
||||||
|
* never render as pinned). Everything else stages with no pin and must say so in the consent
|
||||||
|
* prompt.
|
||||||
|
*/
|
||||||
|
type IntegrityPin = 'sha512' | 'git-commit' | 'none';
|
||||||
|
|
||||||
|
const GIT_SHA_PIN_RE = /^sha:[0-9a-f]{7,40}$/i;
|
||||||
|
|
||||||
|
function resolveIntegrityPin(
|
||||||
|
integrity: unknown,
|
||||||
|
parsed: { kind?: string; ref?: unknown },
|
||||||
|
): IntegrityPin {
|
||||||
|
if (typeof integrity === 'string' && integrity.length > 0) return 'sha512';
|
||||||
|
if (parsed.kind === 'git' && typeof parsed.ref === 'string' && GIT_SHA_PIN_RE.test(parsed.ref)) {
|
||||||
|
return 'git-commit';
|
||||||
|
}
|
||||||
|
return 'none';
|
||||||
|
}
|
||||||
|
|
||||||
/** Keys that must never be used as object indices (prototype-pollution guard). */
|
/** Keys that must never be used as object indices (prototype-pollution guard). */
|
||||||
function isUnsafeKey(k: string): boolean {
|
function isUnsafeKey(k: string): boolean {
|
||||||
return k === '__proto__' || k === 'constructor' || k === 'prototype';
|
return k === '__proto__' || k === 'constructor' || k === 'prototype';
|
||||||
@@ -1023,6 +1048,7 @@ async function installCapability(spec: string, opts: LifecycleOptions): Promise<
|
|||||||
stagedDir,
|
stagedDir,
|
||||||
strictKnownRegistries,
|
strictKnownRegistries,
|
||||||
hostVersion,
|
hostVersion,
|
||||||
|
integrityPin: resolveIntegrityPin(opts.integrity, parsedPre),
|
||||||
});
|
});
|
||||||
|
|
||||||
if (!verdict.allowed) {
|
if (!verdict.allowed) {
|
||||||
@@ -1225,6 +1251,7 @@ async function upgradeCapability(spec: string, opts: LifecycleOptions): Promise<
|
|||||||
stagedDir,
|
stagedDir,
|
||||||
strictKnownRegistries,
|
strictKnownRegistries,
|
||||||
hostVersion,
|
hostVersion,
|
||||||
|
integrityPin: resolveIntegrityPin(opts.integrity, parsedPre),
|
||||||
});
|
});
|
||||||
if (!verdict.allowed) {
|
if (!verdict.allowed) {
|
||||||
return { status: 'blocked', disclosure: verdict.disclosure, blockReasons: verdict.blockReasons };
|
return { status: 'blocked', disclosure: verdict.disclosure, blockReasons: verdict.blockReasons };
|
||||||
|
|||||||
@@ -212,8 +212,102 @@ function _setHttpsGetImpl(fn: HttpsGetImpl | null): void {
|
|||||||
// Injectable HTTP transport (test seam)
|
// Injectable HTTP transport (test seam)
|
||||||
// ---------------------------------------------------------------------------
|
// ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
/**
|
||||||
|
* #3514 (epic #1900 F21): pre-transport fetch gate. Runs BEFORE any bytes leave and before the
|
||||||
|
* (possibly injected) transport is invoked:
|
||||||
|
*
|
||||||
|
* - Non-`https` URLs are refused with a NAMED, actionable reason. parseSpec still CLASSIFIES an
|
||||||
|
* `http://` tarball URL as tarball (no parse-time hard reject — internal-mirror classification
|
||||||
|
* flows stay intact, the adopted split decision on the epic); the https-only transport then
|
||||||
|
* fails clearly instead of with a raw ERR_INVALID_PROTOCOL.
|
||||||
|
* - Loopback / link-local / metadata / unspecified hosts and `localhost`-form names are denied
|
||||||
|
* unconditionally — no legitimate capability install fetches them. RFC1918 private ranges are
|
||||||
|
* deliberately ALLOWED (internal mirrors are the legitimate use this leaves room for); the
|
||||||
|
* denylist is not an allowlist.
|
||||||
|
*
|
||||||
|
* Known limit (disclosed): the check is on the URL's HOST LITERAL. A public hostname that
|
||||||
|
* RESOLVES via DNS to a denied range (rebinding) is out of scope — `https.get` resolves
|
||||||
|
* internally and this gate does not double-resolve.
|
||||||
|
*/
|
||||||
|
function assertFetchableUrl(rawUrl: string): void {
|
||||||
|
let u: URL;
|
||||||
|
try {
|
||||||
|
u = new URL(rawUrl);
|
||||||
|
} catch {
|
||||||
|
throw new Error(`invalid capability source URL: "${rawUrl}"`);
|
||||||
|
}
|
||||||
|
if (u.protocol !== 'https:') {
|
||||||
|
throw new Error(
|
||||||
|
`capability source fetch requires https — refusing "${u.protocol}" URL (host "${u.hostname}"). ` +
|
||||||
|
'The transport never fetches plaintext http; for an internal mirror use an https URL or a local path source.'
|
||||||
|
);
|
||||||
|
}
|
||||||
|
// Node's URL.hostname KEEPS the brackets on IPv6 literals ('[::1]') — strip them so the
|
||||||
|
// v6 checks below see the bare address. A single trailing dot is FQDN syntax ('localhost.'
|
||||||
|
// is the same name as 'localhost'; some resolvers synthesize loopback for it) — strip it.
|
||||||
|
const host = u.hostname.toLowerCase().replace(/^\[|\]$/g, '').replace(/\.$/, '');
|
||||||
|
const deniedHost = () =>
|
||||||
|
new Error(
|
||||||
|
`refusing to fetch capability source from internal host "${host}" (loopback/link-local/metadata denylist)`
|
||||||
|
);
|
||||||
|
if (host === 'localhost' || host.endsWith('.localhost')) {
|
||||||
|
throw deniedHost();
|
||||||
|
}
|
||||||
|
// IPv4 literal: 127/8 loopback, 0/8 unspecified, 169.254/16 link-local (contains the cloud
|
||||||
|
// metadata IPs). Malformed literals that match no range simply fall through to a failed
|
||||||
|
// resolution downstream — denying is not needed for safety there.
|
||||||
|
const v4 = host.match(/^(\d{1,3})\.(\d{1,3})\.(\d{1,3})\.(\d{1,3})$/);
|
||||||
|
if (v4) {
|
||||||
|
const a = Number(v4[1]);
|
||||||
|
const b = Number(v4[2]);
|
||||||
|
if (a === 127 || a === 0 || (a === 169 && b === 254)) throw deniedHost();
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
// IPv6-literal checks. A registered domain NEVER contains ':' after bracket-strip, while every
|
||||||
|
// IPv6 literal does — that colon is the guard. Without it, the fe80::/10 prefix test below
|
||||||
|
// would deny legitimate domains like 'feather.internal' or 'february.example.com' (isolated
|
||||||
|
// review finding — a domain-shaped host must never reach the v6 branch).
|
||||||
|
if (!host.includes(':')) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
// An IPv4-mapped address re-checks as IPv4 — in EITHER spelling, because the URL parser may
|
||||||
|
// preserve the dotted form ('::ffff:127.0.0.1') or normalize to hex hextets ('::ffff:7f00:1').
|
||||||
|
// Otherwise deny ::1 (loopback), :: (unspecified), and fe80::/10 (link-local, hex prefix range
|
||||||
|
// fe80–febf).
|
||||||
|
if (host.startsWith('::ffff:')) {
|
||||||
|
const suffix = host.slice('::ffff:'.length);
|
||||||
|
let v4str: string | null = null;
|
||||||
|
if (/^\d{1,3}(?:\.\d{1,3}){3}$/.test(suffix)) {
|
||||||
|
v4str = suffix;
|
||||||
|
} else {
|
||||||
|
const hextets = suffix.split(':');
|
||||||
|
if (hextets.length === 2 && /^[\da-f]{1,4}$/.test(hextets[0]) && /^[\da-f]{1,4}$/.test(hextets[1])) {
|
||||||
|
const hi = parseInt(hextets[0], 16);
|
||||||
|
const lo = parseInt(hextets[1], 16);
|
||||||
|
v4str = `${(hi >> 8) & 0xff}.${hi & 0xff}.${(lo >> 8) & 0xff}.${lo & 0xff}`;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if (v4str) {
|
||||||
|
assertFetchableUrl(`https://${v4str}/`);
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
// fe80::/10 = hex prefix fe80..febf, i.e. /^fe[89ab]/ (third nibble 8-b has top two bits set).
|
||||||
|
if (host === '::1' || host === '::' || /^fe[89ab]/.test(host)) {
|
||||||
|
throw deniedHost();
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
function realHttpsGet(url: string): Promise<HttpResponse> {
|
function realHttpsGet(url: string): Promise<HttpResponse> {
|
||||||
return new Promise((resolve, reject) => {
|
return new Promise((resolve, reject) => {
|
||||||
|
// #3514: gate BEFORE the transport — scheme + host denylist refuse with named errors and
|
||||||
|
// never invoke the (possibly injected) transport. A throw inside the executor rejects.
|
||||||
|
try {
|
||||||
|
assertFetchableUrl(url);
|
||||||
|
} catch (err) {
|
||||||
|
reject(err instanceof Error ? err : new Error(String(err)));
|
||||||
|
return;
|
||||||
|
}
|
||||||
const req = _httpsGetImpl(
|
const req = _httpsGetImpl(
|
||||||
url,
|
url,
|
||||||
{ headers: { 'User-Agent': 'gsd-core-capability-source/1.0' } },
|
{ headers: { 'User-Agent': 'gsd-core-capability-source/1.0' } },
|
||||||
|
|||||||
@@ -338,6 +338,17 @@ interface Disclosure {
|
|||||||
* Empty when no stagedDir was supplied.
|
* Empty when no stagedDir was supplied.
|
||||||
*/
|
*/
|
||||||
missingArtifacts: string[];
|
missingArtifacts: string[];
|
||||||
|
/**
|
||||||
|
* #3514 (epic #1900 F21c): what pinned the source content before staging — 'pinned' (a verified
|
||||||
|
* sha512 `--integrity` pin), 'commit-pinned' (a git source checked out at a `#sha:<40-hex>`
|
||||||
|
* commit), or 'unverified' (no pin). PROMPT-ONLY: set by evaluateInstallTrust when the caller
|
||||||
|
* supplies `integrityPin`, rendered by summarizeDisclosure, and deliberately EXCLUDED from
|
||||||
|
* `disclosureSignature` — consent's content binding is `bundleContentHash` (#1459), and a
|
||||||
|
* rendered line must never read as a changed executable set (which would fire a spurious
|
||||||
|
* re-consent on every upgrade). Mirrors the instructionSurfaces exclusion precedent (ADR-2363
|
||||||
|
* D4) one field over.
|
||||||
|
*/
|
||||||
|
integrityStatus?: 'pinned' | 'commit-pinned' | 'unverified';
|
||||||
}
|
}
|
||||||
|
|
||||||
type StrictKnownRegistries = string[] | null | undefined;
|
type StrictKnownRegistries = string[] | null | undefined;
|
||||||
@@ -377,6 +388,14 @@ interface InstallTrustArgs {
|
|||||||
* constraint 2; see `ReviewerHostResolver`).
|
* constraint 2; see `ReviewerHostResolver`).
|
||||||
*/
|
*/
|
||||||
resolveHost?: ReviewerHostResolver;
|
resolveHost?: ReviewerHostResolver;
|
||||||
|
/**
|
||||||
|
* #3514 (epic #1900 F21c): what kind of pin the source content carries — 'sha512' (a supplied
|
||||||
|
* `--integrity` pin, verified by the resolver before the verdict runs), 'git-commit' (a git
|
||||||
|
* source pinned by `#sha:<40-hex-commit>`), or 'none'. Optional: absent ⇒ the disclosure
|
||||||
|
* carries no `integrityStatus` and the prompt renders no integrity line (legacy callers see
|
||||||
|
* byte-identical output).
|
||||||
|
*/
|
||||||
|
integrityPin?: 'sha512' | 'git-commit' | 'none';
|
||||||
}
|
}
|
||||||
|
|
||||||
interface InstallTrustVerdict {
|
interface InstallTrustVerdict {
|
||||||
@@ -1159,6 +1178,12 @@ function evaluateInstallTrust(args: InstallTrustArgs): InstallTrustVerdict {
|
|||||||
// openai-http reviewer lane to the human at install/upgrade time — it never affects the
|
// openai-http reviewer lane to the human at install/upgrade time — it never affects the
|
||||||
// consent-binding signature (disclosureSignature never reads resolvedHost; design constraint 2).
|
// consent-binding signature (disclosureSignature never reads resolvedHost; design constraint 2).
|
||||||
const disclosure = discloseExecutableSurfaces(manifest, stagedDir, resolveHost);
|
const disclosure = discloseExecutableSurfaces(manifest, stagedDir, resolveHost);
|
||||||
|
// #3514 (F21c): prompt-only integrity status. Set AFTER discloseExecutableSurfaces so the
|
||||||
|
// surface builder (and every signature computed from it) is untouched — see the field's
|
||||||
|
// disclosure-interface comment for why this must never reach disclosureSignature.
|
||||||
|
if (args.integrityPin === 'sha512') disclosure.integrityStatus = 'pinned';
|
||||||
|
else if (args.integrityPin === 'git-commit') disclosure.integrityStatus = 'commit-pinned';
|
||||||
|
else if (args.integrityPin === 'none') disclosure.integrityStatus = 'unverified';
|
||||||
|
|
||||||
// A manifest that declares a hook script or command module NOT present in the staged bundle
|
// A manifest that declares a hook script or command module NOT present in the staged bundle
|
||||||
// (missing, or escaping the bundle via an absolute/`..` path) is rejected: such an artifact
|
// (missing, or escaping the bundle via an absolute/`..` path) is rejected: such an artifact
|
||||||
@@ -1488,16 +1513,20 @@ function isRemoteMcpServer(s: McpServerSurface): boolean {
|
|||||||
function summarizeDisclosure(disclosure: Disclosure): string[] {
|
function summarizeDisclosure(disclosure: Disclosure): string[] {
|
||||||
const lines: string[] = [];
|
const lines: string[] = [];
|
||||||
const instructionLines = summarizeInstructionSurfaces(disclosure);
|
const instructionLines = summarizeInstructionSurfaces(disclosure);
|
||||||
|
// #3514 (F21c): computed once, appended before every return path so a future path cannot miss it.
|
||||||
|
const integrity = integrityStatusLine(disclosure);
|
||||||
if (!disclosure.hasExecutable) {
|
if (!disclosure.hasExecutable) {
|
||||||
// ADR-2363 D3: "declarative only" is true ONLY when there is no instruction surface either.
|
// ADR-2363 D3: "declarative only" is true ONLY when there is no instruction surface either.
|
||||||
// Claiming it unconditionally told a user their capability contributes nothing to weigh while
|
// Claiming it unconditionally told a user their capability contributes nothing to weigh while
|
||||||
// it was contributing agent instructions — the exact category error ADR-2363 was written to end.
|
// it was contributing agent instructions — the exact category error ADR-2363 was written to end.
|
||||||
if (instructionLines.length === 0) {
|
if (instructionLines.length === 0) {
|
||||||
lines.push('This capability ships no executable surfaces (declarative only).');
|
lines.push('This capability ships no executable surfaces (declarative only).');
|
||||||
|
if (integrity) lines.push(integrity);
|
||||||
return lines;
|
return lines;
|
||||||
}
|
}
|
||||||
lines.push('This capability ships no executable surfaces, but contributes agent instructions:');
|
lines.push('This capability ships no executable surfaces, but contributes agent instructions:');
|
||||||
for (const line of instructionLines) lines.push(line);
|
for (const line of instructionLines) lines.push(line);
|
||||||
|
if (integrity) lines.push(integrity);
|
||||||
return lines;
|
return lines;
|
||||||
}
|
}
|
||||||
lines.push('This capability ships executable surfaces that will run in your agent runtime:');
|
lines.push('This capability ships executable surfaces that will run in your agent runtime:');
|
||||||
@@ -1661,9 +1690,30 @@ function summarizeDisclosure(disclosure: Disclosure): string[] {
|
|||||||
lines.push(` - ${renderValueForPrompt(a)}`);
|
lines.push(` - ${renderValueForPrompt(a)}`);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
if (integrity) lines.push(integrity);
|
||||||
return lines;
|
return lines;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* #3514 (F21c): the one-line integrity status for the consent prompt, or null when the caller
|
||||||
|
* supplied no `integrityPin` (legacy — no line, byte-identical output). GSD-authored literals,
|
||||||
|
* never manifest data — no escaping needed. A consent-prompt claim must be EXACT: a git
|
||||||
|
* commit-pinned source renders its own line, never "sha512 pin" — no sha512 was supplied
|
||||||
|
* (isolated review finding).
|
||||||
|
*/
|
||||||
|
function integrityStatusLine(disclosure: Disclosure): string | null {
|
||||||
|
if (disclosure.integrityStatus === 'pinned') {
|
||||||
|
return ' content: sha512 pin supplied and verified before staging';
|
||||||
|
}
|
||||||
|
if (disclosure.integrityStatus === 'commit-pinned') {
|
||||||
|
return ' content: pinned to a git commit, checked out before staging';
|
||||||
|
}
|
||||||
|
if (disclosure.integrityStatus === 'unverified') {
|
||||||
|
return ' content: NO PINNED HASH — staged unverified (a computed sha512 is recorded in the ledger at install)';
|
||||||
|
}
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
|
||||||
// ---------------------------------------------------------------------------
|
// ---------------------------------------------------------------------------
|
||||||
// Exports
|
// Exports
|
||||||
// ---------------------------------------------------------------------------
|
// ---------------------------------------------------------------------------
|
||||||
|
|||||||
@@ -378,11 +378,6 @@ function isInsideRoot(candidatePath: string, rootDir: string): boolean {
|
|||||||
return target === root || target.startsWith(`${root}${path.sep}`);
|
return target === root || target.startsWith(`${root}${path.sep}`);
|
||||||
}
|
}
|
||||||
|
|
||||||
// #3484: restored from the retired SDK lineage (dropped as a bare 256 * 1024 literal
|
|
||||||
// in the ADR-0174 collapse). Counts String#length (UTF-16 code units), not bytes —
|
|
||||||
// semantics preserved from the deleted source.
|
|
||||||
const MAX_MODIFIED_FILE_BYTES = 256 * 1024;
|
|
||||||
|
|
||||||
function readModifiedFilesContent(projectDir: string, summaries: string[]): string {
|
function readModifiedFilesContent(projectDir: string, summaries: string[]): string {
|
||||||
const out: string[] = [];
|
const out: string[] = [];
|
||||||
let total = 0;
|
let total = 0;
|
||||||
@@ -395,7 +390,7 @@ function readModifiedFilesContent(projectDir: string, summaries: string[]): stri
|
|||||||
if (total >= 50) break;
|
if (total >= 50) break;
|
||||||
if (!file || !isInsideRoot(file, projectDir)) continue;
|
if (!file || !isInsideRoot(file, projectDir)) continue;
|
||||||
const raw = readIfExists(resolvePath(file, projectDir));
|
const raw = readIfExists(resolvePath(file, projectDir));
|
||||||
out.push(raw.length > MAX_MODIFIED_FILE_BYTES ? raw.slice(0, MAX_MODIFIED_FILE_BYTES) : raw);
|
out.push(raw.length > 256 * 1024 ? raw.slice(0, 256 * 1024) : raw);
|
||||||
total++;
|
total++;
|
||||||
}
|
}
|
||||||
if (total >= 50) break;
|
if (total >= 50) break;
|
||||||
|
|||||||
@@ -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 } });
|
const [rec] = lifecycle.outdatedCapabilities({ runtimeDir: dir, execOverrides: { npm: fakeNpm } });
|
||||||
assert.strictEqual(rec.status, 'pinned');
|
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');
|
||||||
|
});
|
||||||
|
|||||||
@@ -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');
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|||||||
@@ -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
|
// #3515 (epic #1900 F20) — MCP server config is INTENTIONALLY unconfined
|
||||||
// (unlike hooks); the consent disclosure must say so. Adopted decision: the
|
// (unlike hooks); the consent disclosure must say so. Adopted decision: the
|
||||||
|
|||||||
@@ -29,7 +29,7 @@ const fs = require('fs');
|
|||||||
const path = require('path');
|
const path = require('path');
|
||||||
|
|
||||||
const { parseDecisions, extractDecisions } = require('../gsd-core/bin/lib/decisions.cjs');
|
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 ───────────────────────────────
|
// ─── 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)}`);
|
`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)}`);
|
|
||||||
});
|
|
||||||
});
|
|
||||||
|
|||||||
Reference in New Issue
Block a user