From 0f13ff8f18897191a4008619f740011bf7e72c3f Mon Sep 17 00:00:00 2001 From: Ethan Dutton <46871249+ejdutton@users.noreply.github.com> Date: Tue, 16 Jun 2026 13:53:19 -0400 Subject: [PATCH 1/2] feat(resources): linkAuth validator wiring and per-host outcome codes (#113 slice 2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wires the slice-1 pure engine into ExternalLinkValidator and adds the five LINK_AUTH_* outcome codes. End-to-end: when an adopter sets resources.linkAuth in vibe-agent-toolkit.config.yaml, vat resources validate bypasses markdown-link-check for any URL whose host is claimed by a provider, issues an authenticated fetch against the rewritten URL, and classifies the response per design §7 into one of: LINK_AUTH_DEAD (404/410 from honest-404 hosts) LINK_AUTH_DEAD_OR_UNAUTHORIZED (404 from ambiguous hosts like GitHub) LINK_AUTH_FORBIDDEN (403) LINK_AUTH_UNAUTHORIZED (401) LINK_AUTH_UNVERIFIED (no token resolved) New surface: fetchAuthenticated() with cross-origin Authorization stripping (§8, sticky across chains) and 429/Retry-After honoring (§5.2, 60s DoS cap + 250ms good-neighbor floor); pure classifier per §7; project-config → engine bridge with post-expansion validation against InlineProviderSchema (catches typo'd macro overrides); auth cache keyed by rewritten URL and scoped to a per-OS-user subdirectory (§6.3) with an explicit version field forward-compat for slice 3. 196 new tests across 6 files. Doc-anchor coverage iterator (packages/agent-schema/test/docs/validation-codes.test.ts) iterates CODE_REGISTRY and asserts each code has a matching docs section. Adopter-visible breaking changes: the LinkAuthConfig type exported from @vibe-agent-toolkit/resources renames to LinkAuthProjectConfig (resolves auto-import ambiguity with the engine type of the same name); the external-link cache layout adds an auth-${osUser}/ subdirectory and a version field that invalidates pre-existing entries on first read. Fixed: ExternalLinkValidator.clearCache()/getCacheStats() now operate on both anonymous and authenticated caches (previously the auth cache survived a manual clear, surfacing stale 401/403 entries after token rotation). Deferred to later slices: content-fetch primitive and content cache (slice 3); cross-platform .cmd-shim system test, VAT_LINKAUTH_ALLOW_COMMAND=0 opt-out, and contributor docs (slice 4). See CHANGELOG.md [Unreleased] for full details. Co-Authored-By: Claude Opus 4.7 (1M context) --- CHANGELOG.md | 4 + docs/validation-codes.md | 39 ++ .../schemas/validation-config.json | 14 +- packages/agent-schema/src/validation-codes.ts | 30 ++ .../test/docs/validation-codes.test.ts | 25 ++ .../test/validation-codes.test.ts | 29 ++ packages/resources/src/external-link-cache.ts | 24 ++ .../resources/src/external-link-validator.ts | 267 +++++++++++- packages/resources/src/link-auth-classify.ts | 61 +++ .../resources/src/link-auth-config-build.ts | 108 +++++ packages/resources/src/link-auth-fetch.ts | 177 ++++++++ packages/resources/src/resource-registry.ts | 19 +- packages/resources/src/schemas/link-auth.ts | 13 +- packages/resources/test/auth-fetch-mocks.ts | 89 ++++ .../test/external-link-cache.test.ts | 19 + .../test/external-link-validator-auth.test.ts | 390 ++++++++++++++++++ .../resources/test/link-auth-classify.test.ts | 123 ++++++ .../test/link-auth-config-build.test.ts | 132 ++++++ .../resources/test/link-auth-fetch.test.ts | 254 ++++++++++++ packages/utils/src/link-auth/resolve-token.ts | 2 +- packages/utils/src/link-auth/resolve.ts | 15 +- 21 files changed, 1819 insertions(+), 15 deletions(-) create mode 100644 packages/agent-schema/test/docs/validation-codes.test.ts create mode 100644 packages/resources/src/link-auth-classify.ts create mode 100644 packages/resources/src/link-auth-config-build.ts create mode 100644 packages/resources/src/link-auth-fetch.ts create mode 100644 packages/resources/test/auth-fetch-mocks.ts create mode 100644 packages/resources/test/external-link-validator-auth.test.ts create mode 100644 packages/resources/test/link-auth-classify.test.ts create mode 100644 packages/resources/test/link-auth-config-build.test.ts create mode 100644 packages/resources/test/link-auth-fetch.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index fbd55bab..70834b4a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Internal +- **linkAuth validator wiring + per-host outcome codes (issue #113, slice 2).** The slice-1 pure engine is now end-to-end: when an adopter sets `resources.linkAuth` in `vibe-agent-toolkit.config.yaml`, `vat resources validate` bypasses the anonymous `markdown-link-check` path for any URL whose host is claimed by a provider, issues an authenticated `fetch()` against the rewritten URL with the configured token, and classifies the response per design §7 into one of five new `CODE_REGISTRY` entries: `LINK_AUTH_DEAD` (404/410 from an honest-404 host — `error`-severity, the only such code in the slice; design §7 establishes that an authenticated 404 against e.g. SharePoint is high-confidence link rot, satisfying the rule-design corpus-evidence bar), `LINK_AUTH_DEAD_OR_UNAUTHORIZED` (404 from an ambiguous host like GitHub that masks `403`s — warn), `LINK_AUTH_FORBIDDEN` (`403` — warn), `LINK_AUTH_UNAUTHORIZED` (`401` — warn, promote to `error` on strict CI lanes), and `LINK_AUTH_UNVERIFIED` (no token resolved — warn, never cached per §6.3). New files: `packages/resources/src/link-auth-fetch.ts` (`fetchAuthenticated()` — bounded redirect loop with **cross-origin `Authorization` stripping (§8)** that is sticky across the rest of the chain to defeat token-laundering, **429/`Retry-After` honoring (§5.2)** parsing both delta-seconds and HTTP-date forms with a 60s DoS cap *and* a 250ms good-neighbor floor, all dependency-injectable via `fetchImpl`/`sleep`/`signal`); `packages/resources/src/link-auth-classify.ts` (pure `(status, providerCheck) → outcome+code` per §7's table); `packages/resources/src/link-auth-config-build.ts` (bridge from adopter config to engine — runs `expandMacro` on `{ use: , ...overrides }` entries and **post-expansion validates against `InlineProviderSchema`** so a typo'd override field that passthroughs through the macro schema surfaces as a config error instead of silent runtime drift, with a compile-time `_KeysAgree` assertion locking the schema's top-level field set to the engine's `Provider` interface). New cache architecture: a second `ExternalLinkCache` instance for auth-branch results, **keyed by the rewritten URL** (the original `blob/` URL 404s — caching it would poison results) and scoped to a per-OS-user subdirectory `cacheDir/auth-${sanitizedOsUser}/` so two users on a shared CI host never read each other's authenticated results (§6.3); the OS user resolves through `os.userInfo()` → `USER`/`USERNAME` env → `'default'` with a one-shot `console.warn` on the last fallback so the cross-user-leak risk is observable. Cache entries gain an explicit `version: 1` field; reads of any other version produce a miss, so slice 3's content-cache evolution can change the entry shape without misparsing pre-existing files. Doc-anchor coverage iterator test (`packages/agent-schema/test/docs/validation-codes.test.ts`) iterates `CODE_REGISTRY` and asserts each code has a matching `### \`CODE\`` heading in `docs/validation-codes.md` plus a convention-matching `entry.reference` — 126 assertions covering all 63 codes, future-proofs the per-code docs requirement. Five new doc sections under "Authenticated External Link Codes" in `docs/validation-codes.md`. Engine surface gains: `Provider.check` now flows through on the verified `ResolveOutcome` so the classifier can route per-provider `notFoundMeaning`; `ExternalLinkValidatorOptions` gains `linkAuthConfig`, `fetchImpl`, `linkAuthDeps`, `sleep`, and `osUser` (the first two are adopter-usable for corporate-proxy/custom-TLS injection, the rest test-only); `LinkValidationResult` gains a `code?: IssueCode` field that the `resource-registry.ts` consumer prefers over the existing status-code-to-`EXTERNAL_URL_*` mapping. **196 new tests across `link-auth-classify` (13), `link-auth-fetch` (19), `external-link-validator-auth` (25), `link-auth-config-build` (10), `validation-codes` (+126 doc-anchor iterator, +2 LINK_AUTH_* registry), and `external-link-cache` (+1 version-gate).** Security-load-bearing tests pin the cross-origin Authorization strip (case-insensitive, sticky across chains, with userinfo/relative-Location edge cases), the path-traversal sanitizer (table-driven over 9 pathological `osUser` inputs), the post-expansion validation rejecting typo'd macro overrides, the unverified-no-cache invariant, the cache-hit re-classification (cache hits re-run the classifier against the current provider so a `notFoundMeaning` flip between runs surfaces the new code, not the old one), and an Object.hasOwn-based prototype-pollution defense on the `{ use }` discriminator in `buildLinkAuthEngineConfig`. Slice 4 (cross-platform `.cmd`-shim system test, `VAT_LINKAUTH_ALLOW_COMMAND=0` opt-out, contributor docs) and slice 3 (content-fetch primitive + content cache) are downstream. - **linkAuth pure engine foundation (issue #113, slice 1).** Adds a config-driven engine for authenticated external URL resolution, scoped to the pure-logic layer with no consumer wiring yet (the `ExternalLinkValidator` integration and the `LINK_AUTH_*` `CODE_REGISTRY` entries are slice 2; the content-fetch primitive is slice 3). New `link-auth/` module under `@vibe-agent-toolkit/utils` with eight files: `transforms.ts` (closed allowlist — `base64url`, `urlencode`, `lower` — with `Object.hasOwn`-based prototype-chain defense), `template.ts` (tiny `${name}` / `${transform(name)}` renderer, deliberately separate from the Handlebars renderer in `utils/template.ts`), `rewrite.ts` (ordered `when → vars → to` pipeline with fragment/query stripping per design §5.2), `build-headers.ts` (header rendering plus structural `Authorization` redaction), `select-provider.ts` (host-glob matching via picomatch with `excludeHost`), `expand-macro.ts` (YAML loader + deep-merge expander), `resolve-token.ts` (ordered env / `safeExecResult`-backed argv-command sources, first-non-empty wins, no shell), and `resolve.ts` (the public `resolveAuthenticatedUrl(url, config)` entry returning one of `{fetchUrl, headers}` / `{outcome: 'unsupported'}` / `{outcome: 'unverified', reason}`). Ships the `github` and `sharepoint` macros as a YAML data asset (`src/link-auth/macros.yaml`), with a new cross-platform `packages/dev-tools/src/copy-yaml-assets.ts` post-build step bundling `.yaml` into `dist/` — first YAML-asset shipping pattern in the utils package. Adds `yaml` as a utils dependency. Companion Zod schema in `@vibe-agent-toolkit/resources` (`src/schemas/link-auth.ts`) validates the `resources.linkAuth` config block (strict; accepts either `{ use: , ...overrides }` or full inline providers), wired as an optional field on `ResourcesConfigSchema`. 140 unit tests in utils + 29 schema tests in resources, all pure-logic with no network or filesystem dependencies; security-load-bearing tests pin the closed-allowlist guarantee, the `${__proto__}` bypass defense, the token-never-leaks-into-Authorization invariant, and `shell: false` literal-argv handling. - **Corpus seed expanded from 9 → 237 entries via a new committed importer at `packages/dev-tools/src/import-marketplace.ts` (`bun run import-marketplace [--allow-shrink]`).** The script fetches `.claude-plugin/marketplace.json` from `anthropics/claude-plugins-official` (205 of 209 raw entries kept) and `anthropics/knowledge-work-plugins` (30 of 60 — the knowledge-work catalog turns out to be ≈50% mirror entries of the official catalog) via `gh api`, maps each upstream entry to a `PluginEntry`, deduplicates by `source` URL (preserved VAT-owned entries always win; otherwise alphabetical-first-name wins within each duplicate cluster), and rewrites `corpus/seed.yaml`. Mapping rules: `bucket: official` uniformly (both catalogs are anthropics-curated marketplaces — `bucket` is the *reporting posture* per slice 1a, not code provenance); `confidence: first-party` for catalog-internal string sources and `github.com/anthropics/...` object sources, else `curated`; the `./partner-built/` knowledge-work convention overrides to `curated`; `maturity: production` for all entries. URL composition handles all five upstream source shapes (string, `git-subdir` ± `ref`, `url` ± `path`, `github`), throwing on unknown discriminators. The seven sample entries from slice 1a are regenerated from upstream manifests on every re-import. Re-import safety: the importer refuses to overwrite `corpus/seed.yaml` if either upstream catalog returned 0 plugins or the new entry count would drop more than 20% vs. the existing seed; `--allow-shrink` bypasses both gates for the rare case where shrinkage is real. The generated `seed.yaml` header dropped its earlier per-entry `validation:` claim (the importer throws on validation blocks today) and now states explicitly that entry `source` URLs pin a fragment ref (typically the default branch), not a per-entry commit SHA — the catalog SHAs in the header are this run's audit provenance. Issue #99 slice 1b — follows the schema change from PR #111 (slice 1a). - **Empirical compatibility harness (`packages/dev-tools/src/compat-empirical/`).** Per-#100 research scaffold for measuring skill compatibility across `claude-code`, `claude-cowork`, and `claude-chat`: a CLI (`predict`/`run`/`judge`/`report`/`all`) that joins VAT's static predictions with deterministic runtime observations and an LLM-judge semantic read into a reality-vs-prediction matrix — an evidence artifact for proposing detector improvements that each cite specific (skill, runtime) cells. Probe coverage: multi-prompt + repeat-N with adaptive N=3→N=5 extension, mandatory positive+negative prompt pairing per corpus entry, and negative-prompt agreement inversion so false-positive triggers surface as `vat-optimistic`. Evidence quality: the deterministic class is widened from 6 to 9 values (splitting `error` into `install-failed`/`runtime-error`, `not-invoked` into `not-invoked-engaged`/`not-invoked-empty`, adding `refused`), with a v2 judge prompt that adds a `refused` verdict. Report fidelity: coverage stats, per-bucket headline (own/official/community × ran/agree/optimistic/pessimistic/gray-zone), gray-zone (mixed-signal) and high-variance subsections, and per-attempt variance rendered inline (`runtime-error (2/3) / failed (3/3)`). Judge replay persists `judge-calls/---.json` artifacts that a new `re-judge` subcommand re-executes against an optionally different model or freshly-edited system prompt — without re-spending operator hours on the runtime side. Also landed: `git fetch --tags --force` before named-ref fetch (annotated tag refresh) and `setup()` teardown-first idempotency for the manual driver. No detector code or `RUNTIME_PROFILES` changes; lives entirely in the private `@vibe-agent-toolkit/dev-tools` package with no adopter-facing surface. Design: [the v2 harness design](./docs/research/2026-05-23-compat-empirical-harness-v2-design.md). Corpus authoring, the first real run, and the docs deliverable are the downstream work. @@ -21,10 +22,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **`validation.severity` / `validation.allow` keys are validated against real codes.** A mistyped code key (e.g. `LNIK_OUTSIDE_PROJECT`) is now a config-load error instead of a silent no-op. - **Corpus seed entries now require `bucket`, `confidence`, and `maturity` metadata fields.** `PluginEntrySchema` in `vat corpus scan`'s seed loader gains three required enum fields: `bucket: 'official' | 'community'`, `confidence: 'first-party' | 'curated' | 'listed'`, and `maturity: 'production' | 'experimental' | 'example'`. The bundled `corpus/seed.yaml` is updated; downstream callers running custom seeds must add the fields to every entry. `bucket` is the load-bearing discriminator (`official` entries report named findings; `community` entries are aggregate-only in follow-up work). The other two are descriptive metadata used by triage tooling. - **`vat claude marketplace publish` no longer reports the project root `package.json` version in the CLI banner, commit message, status YAML, or CHANGELOG section lookup.** The label is now derived from the staged `marketplace.json`. Single-plugin marketplaces use the plugin's version — banner reads `Publishing marketplace "X" v0.0.4`, commit subject reads `publish v0.0.4`. Multi-plugin marketplaces drop the `v` entirely — banner reads `Publishing marketplace "X"`, commit subject reads `publish X` — since the per-plugin `version` fields in the published `marketplace.json` are the source of truth for which plugin moved to which version. Two visible side-effects follow: (1) the status YAML's `published[*].version` field is now absent for multi-plugin marketplaces (previously it carried the misleading project version) — automation should read per-plugin versions from the published `marketplace.json` instead; (2) the stamped `## [X.Y.Z]` CHANGELOG lookup now uses the plugin's version rather than the project's, so a previously-ignored matching section will now be picked up as the commit body for single-plugin marketplaces. The `marketplace.json` schema's optional top-level `version` field is not yet consumed — that is a separate follow-up. +- **Adopter-facing `LinkAuthConfig` type renamed to `LinkAuthProjectConfig` (issue #113).** Both `@vibe-agent-toolkit/utils` (engine) and `@vibe-agent-toolkit/resources` (Zod-inferred adopter shape) previously exported a type named `LinkAuthConfig`, causing IDE auto-import ambiguity in any code that touched both. The adopter type — accessible as `import type { LinkAuthProjectConfig } from '@vibe-agent-toolkit/resources/schemas/link-auth'` — is the one renamed; the engine's `LinkAuthConfig` is unchanged (more API surface depends on it). Migration: rename the import. The Zod schema's name (`LinkAuthConfigSchema`) is unchanged. +- **External-link cache directory layout adds an `auth-${osUser}/` subdirectory and an entry `version: 1` field (issue #113 §6.3).** When `vat resources validate` runs with `resources.linkAuth` configured, authenticated-fetch results land under `/auth-${sanitizedOsUser}/external-links.json` rather than the shared `external-links.json` used by the anonymous `markdown-link-check` path — two users on the same host (e.g. shared CI runners) cannot read each other's authenticated cache entries. All cache entries now carry an explicit `version: 1` field; entries written under a different (or missing) version are treated as a cache miss, so any pre-existing `external-links.json` triggers a one-time re-fetch on first run after upgrade. The `version` gate is forward-compat for slice 3's content-cache shape evolution. - **`vat claude marketplace publish` no longer pushes per-plugin `-v` source-repo tags.** The post-publish tagging step (introduced alongside multi-plugin versioning) is removed entirely — no tags are created or pushed, and the misleading `Repository not found` / "tag already exists at a different commit" warnings it emitted on every cross-repo publish are gone ([#121](https://github.com/jdutton/vibe-agent-toolkit/issues/121)). The tags were pushed to the marketplace remote rather than a source remote, never landed anywhere useful, and there was no opt-in demand. Which plugin moved to which version is now determined solely by the per-plugin `version` fields in the published `marketplace.json`. No config key or flag is involved; if you relied on these tags, create them in your own release workflow. ### Fixed +- **`ExternalLinkValidator.clearCache()` and `getCacheStats()` now operate on both caches (issue #113).** Slice 2 introduced a second cache instance for authenticated-link results (per-OS-user scoping); the existing `clearCache()` / `getCacheStats()` methods continued to touch only the anonymous cache, so an adopter rotating a token would see stale `401`/`403` entries until the auth cache TTL expired. Both methods now clear/sum across both caches. - **Transformers.js integration tests now skip on Windows CI instead of flaking.** `transformers-embedding-provider.integration.test.ts` and the Transformers.js block of `comparison.integration.test.ts` skip on Windows (in addition to skipping when the optional `@xenova/transformers` dependency is absent), matching the existing `onnx-embedding-provider` test. These tests download a model over the network and load the `onnxruntime-node` native backend — both flaky in Windows CI. Such a failure was previously mislabeled `@xenova/transformers is not installed` by an over-broad `catch` in the provider's `loadPipeline` (the package was installed; the model download/inference is what failed), which is also why an availability-only guard did not prevent it. - **Config-first skill discovery now honors `..` in `skills.include` patterns.** `vat build`, `vat verify`, and `vat skills validate` all funnel through `discoverSkillsFromConfig`, which previously passed every include pattern to a single downward-only crawl rooted at `projectRoot` — so an include like `"../../docs/skills/*/SKILL.md"` (common in monorepos where SKILL.md sources live alongside, not inside, the package) silently matched zero skills. `vat audit` accepted the same config only because it has a separate filesystem-first walker. Each include pattern is now split into a literal base + glob remainder via `picomatch.scan`, patterns are grouped by their resolved absolute base, and the crawler runs once per base — making config-first discovery agree with audit. User-supplied excludes stay anchored to `projectRoot` so patterns like `docs/private/**` keep their original meaning, and a pattern resolving to a nonexistent base now silently produces zero matches. diff --git a/docs/validation-codes.md b/docs/validation-codes.md index 6b264ab7..90d58a7c 100644 --- a/docs/validation-codes.md +++ b/docs/validation-codes.md @@ -202,6 +202,45 @@ Codes that fire when `vat resources validate` checks external `http(s)://` links - **Why it matters:** A network-level failure often reflects a transient DNS or connectivity problem on the validating machine rather than a genuinely broken link, so it is surfaced as a warning rather than blocking the build. - **Fix:** Check the host (DNS, reachability, certificate); or set `severity.EXTERNAL_URL_ERROR` to `ignore` if the host is intentionally unreachable from the validation environment. +## Authenticated External Link Codes + +Codes that fire when `vat resources validate` checks external links whose host is claimed by a provider in `resources.linkAuth` (e.g. private GitHub repos, SharePoint files). The validator issues an authenticated request via the configured token source and classifies the response; outcome-to-code mapping is per-provider because hosts disagree about what a 404 means (GitHub masks `403 access-denied` as `404`; Microsoft Graph distinguishes cleanly). All five codes are configurable like any other code (`validation.severity` / `validation.allow`); the provider's `check` block routes an outcome to a *code*, never to a *severity*. + +### `LINK_AUTH_DEAD` + +- **Default:** `error` +- **What:** An authenticated external link returned `404` or `410` from a host whose provider declares `notFoundMeaning: dead` (i.e. honest-404 hosts like SharePoint). The request was authenticated, so `404` here is not a permissions problem — the resource is missing. +- **Why it matters:** Unlike the anonymous external-URL check, an authenticated 404 against an honest-404 host is high-confidence evidence of link rot, which is why this is the only `LINK_AUTH_*` code that defaults to `error`. The provider declares this property; hosts that mask access-denied as 404 instead use [`LINK_AUTH_DEAD_OR_UNAUTHORIZED`](#link_auth_dead_or_unauthorized). +- **Fix:** Fix or remove the link; or set `severity.LINK_AUTH_DEAD` to `ignore` if the path is expected to be transient. + +### `LINK_AUTH_DEAD_OR_UNAUTHORIZED` + +- **Default:** `warning` +- **What:** An authenticated external link returned `404` from a host whose provider declares `notFoundMeaning: ambiguous` (i.e. hosts that mask `403 access-denied` as `404`, like GitHub). The link is either rotted *or* inaccessible to the current identity — the response alone cannot distinguish. +- **Why it matters:** GitHub deliberately does not leak existence of private resources via `403`; both "the file doesn't exist" and "you don't have permission to see this file" return `404`. The checker cannot prove rot, so this is a warning rather than an error. Contributors on a repo where some links point at resources they lack access to will see warnings, not red builds. +- **Fix:** Verify the URL by hand (or with a more-privileged token) to disambiguate; fix or remove if rotted; or set `severity.LINK_AUTH_DEAD_OR_UNAUTHORIZED` to `ignore` if cross-identity ambiguity is expected. + +### `LINK_AUTH_FORBIDDEN` + +- **Default:** `warning` +- **What:** An authenticated external link returned `403`: the configured identity is authenticated, but the resource refuses access. (Only fires for hosts that emit honest `403`s — see `LINK_AUTH_DEAD_OR_UNAUTHORIZED` for hosts that mask access-denied as `404`.) +- **Why it matters:** A `403` is a permissions signal, not link rot. Treating it as an error would block builds whenever a contributor lacks access to some linked resource — common on cross-team docs. The remedy is granting access or switching identity, not editing the link. +- **Fix:** Grant the identity access to the resource; switch to an identity that has access; or set `severity.LINK_AUTH_FORBIDDEN` to `ignore` if cross-identity inaccessibility is expected. + +### `LINK_AUTH_UNAUTHORIZED` + +- **Default:** `warning` +- **What:** An authenticated external link returned `401`: the token sent with the request was missing, expired, or invalid for the resource's auth scheme. +- **Why it matters:** Usually a configuration or session problem — the token source resolved a value, but the host rejected it. The link itself may be perfectly fine. Strict CI lanes that should fail on stale credentials can promote this to `error`. +- **Fix:** Refresh the token (e.g. `gh auth login`, `az login`); check the `token` config in `resources.linkAuth`; or promote `severity.LINK_AUTH_UNAUTHORIZED` to `error` on strict CI lanes. + +### `LINK_AUTH_UNVERIFIED` + +- **Default:** `warning` +- **What:** A provider in `resources.linkAuth` claims this host, but no token source resolved — none of the configured env vars or argv commands produced a non-empty value, so no authenticated request was attempted. +- **Why it matters:** External-URL checking is opt-in, so silently skipping links the project asked to check would be misleading. But `unverified` is not link rot — the link may be fine; the validator just couldn't authenticate. The fix is configuration, not link editing. (`unverified` outcomes are never cached, because the result flips the moment a token appears.) +- **Fix:** Configure a `token` source (env var or argv command); log in to the underlying CLI (e.g. `gh auth login`, `az login`); or set `severity.LINK_AUTH_UNVERIFIED` to `ignore` if running without auth is intentional. + ## Packaging-Only Codes *Stance: see [Packaging](./skill-quality-and-compatibility.md#packaging).* diff --git a/packages/agent-schema/schemas/validation-config.json b/packages/agent-schema/schemas/validation-config.json index 640a24d7..8b36e2c2 100644 --- a/packages/agent-schema/schemas/validation-config.json +++ b/packages/agent-schema/schemas/validation-config.json @@ -74,7 +74,12 @@ "FRONTMATTER_UNKNOWN_LINK", "EXTERNAL_URL_DEAD", "EXTERNAL_URL_TIMEOUT", - "EXTERNAL_URL_ERROR" + "EXTERNAL_URL_ERROR", + "LINK_AUTH_DEAD", + "LINK_AUTH_DEAD_OR_UNAUTHORIZED", + "LINK_AUTH_FORBIDDEN", + "LINK_AUTH_UNAUTHORIZED", + "LINK_AUTH_UNVERIFIED" ] } }, @@ -169,7 +174,12 @@ "FRONTMATTER_UNKNOWN_LINK", "EXTERNAL_URL_DEAD", "EXTERNAL_URL_TIMEOUT", - "EXTERNAL_URL_ERROR" + "EXTERNAL_URL_ERROR", + "LINK_AUTH_DEAD", + "LINK_AUTH_DEAD_OR_UNAUTHORIZED", + "LINK_AUTH_FORBIDDEN", + "LINK_AUTH_UNAUTHORIZED", + "LINK_AUTH_UNVERIFIED" ] } } diff --git a/packages/agent-schema/src/validation-codes.ts b/packages/agent-schema/src/validation-codes.ts index 20acc6ff..f539c8a9 100644 --- a/packages/agent-schema/src/validation-codes.ts +++ b/packages/agent-schema/src/validation-codes.ts @@ -386,6 +386,36 @@ export const CODE_REGISTRY = { 'Check the host; or set severity: ignore.', 'external_url_error', ), + LINK_AUTH_DEAD: entry( + 'error', + 'Authenticated external link returned 404/410 from a host that gives honest 404s (notFoundMeaning: dead). Genuine link rot.', + 'Fix or remove the link; or set severity.LINK_AUTH_DEAD to ignore if the path is expected to be transient.', + 'link_auth_dead', + ), + LINK_AUTH_DEAD_OR_UNAUTHORIZED: entry( + 'warning', + 'Authenticated external link returned 404 from a host that masks access-denied as 404 (notFoundMeaning: ambiguous, e.g. GitHub). The link is either rotted or inaccessible to the current identity — cannot tell which.', + 'Verify the URL by hand (or with a more-privileged token) to disambiguate; fix or remove if rotted; or set severity.LINK_AUTH_DEAD_OR_UNAUTHORIZED to ignore if cross-identity ambiguity is expected.', + 'link_auth_dead_or_unauthorized', + ), + LINK_AUTH_FORBIDDEN: entry( + 'warning', + 'Authenticated external link returned 403: the configured identity is authenticated but lacks access to that resource. Not link rot.', + 'Grant the identity access to the resource; switch to an identity that has access; or set severity.LINK_AUTH_FORBIDDEN to ignore if cross-identity inaccessibility is expected.', + 'link_auth_forbidden', + ), + LINK_AUTH_UNAUTHORIZED: entry( + 'warning', + 'Authenticated external link returned 401: the configured token is missing, expired, or invalid.', + "Refresh the token (e.g. `gh auth login`, `az login`); check the `token` config in resources.linkAuth; or promote severity.LINK_AUTH_UNAUTHORIZED to error on strict CI lanes.", + 'link_auth_unauthorized', + ), + LINK_AUTH_UNVERIFIED: entry( + 'warning', + 'A provider in resources.linkAuth claims this host, but no token source resolved (none of the configured env/command sources produced a value).', + "Configure a `token` source (env var or argv command); log in to the underlying CLI (e.g. `gh auth login`, `az login`); or set severity.LINK_AUTH_UNVERIFIED to ignore if running without auth is intentional.", + 'link_auth_unverified', + ), } as const satisfies Record; export type IssueCode = keyof typeof CODE_REGISTRY; diff --git a/packages/agent-schema/test/docs/validation-codes.test.ts b/packages/agent-schema/test/docs/validation-codes.test.ts new file mode 100644 index 00000000..0677cc81 --- /dev/null +++ b/packages/agent-schema/test/docs/validation-codes.test.ts @@ -0,0 +1,25 @@ +import { readFileSync } from 'node:fs'; +import { fileURLToPath } from 'node:url'; + +import { describe, expect, it } from 'vitest'; + +import { CODE_REGISTRY } from '../../src/validation-codes.js'; + +const docsPath = fileURLToPath(new URL('../../../../docs/validation-codes.md', import.meta.url)); +// Path is derived from `import.meta.url`, not user input — points at the +// repo's docs/validation-codes.md, the source of truth for code reference anchors. +// eslint-disable-next-line security/detect-non-literal-fs-filename +const docs = readFileSync(docsPath, 'utf8'); + +describe('CODE_REGISTRY ↔ docs/validation-codes.md coverage', () => { + for (const code of Object.keys(CODE_REGISTRY)) { + it(`${code}: docs/validation-codes.md has the matching ### heading`, () => { + expect(docs).toContain(`### \`${code}\``); + }); + + it(`${code}: entry.reference is the lowercased code anchor`, () => { + const entry = CODE_REGISTRY[code as keyof typeof CODE_REGISTRY]; + expect(entry.reference).toBe(`#${code.toLowerCase()}`); + }); + } +}); diff --git a/packages/agent-schema/test/validation-codes.test.ts b/packages/agent-schema/test/validation-codes.test.ts index 606bd6ff..650d9c6c 100644 --- a/packages/agent-schema/test/validation-codes.test.ts +++ b/packages/agent-schema/test/validation-codes.test.ts @@ -85,6 +85,35 @@ describe('CODE_REGISTRY — capability and compat codes', () => { }); }); +describe('CODE_REGISTRY — link-auth codes (#113 slice 2)', () => { + it('registers all five LINK_AUTH_* codes', () => { + const codes: IssueCode[] = [ + 'LINK_AUTH_DEAD', + 'LINK_AUTH_DEAD_OR_UNAUTHORIZED', + 'LINK_AUTH_FORBIDDEN', + 'LINK_AUTH_UNAUTHORIZED', + 'LINK_AUTH_UNVERIFIED', + ]; + for (const code of codes) { + expect(CODE_REGISTRY[code], `registry missing ${code}`).toBeDefined(); + expect(CODE_REGISTRY[code].description.length).toBeGreaterThan(10); + expect(CODE_REGISTRY[code].fix.length).toBeGreaterThan(10); + expect(CODE_REGISTRY[code].reference).toMatch(/^#link_auth_/); + } + }); + + it('enforces expected default severities per #113 §7', () => { + // Only LINK_AUTH_DEAD is an error (honest-404 host like SharePoint — + // 404 with valid auth is link rot). The four others default to warning; + // strict CI lanes can promote LINK_AUTH_UNAUTHORIZED to error. + expect(CODE_REGISTRY.LINK_AUTH_DEAD.defaultSeverity).toBe('error'); + expect(CODE_REGISTRY.LINK_AUTH_DEAD_OR_UNAUTHORIZED.defaultSeverity).toBe('warning'); + expect(CODE_REGISTRY.LINK_AUTH_FORBIDDEN.defaultSeverity).toBe('warning'); + expect(CODE_REGISTRY.LINK_AUTH_UNAUTHORIZED.defaultSeverity).toBe('warning'); + expect(CODE_REGISTRY.LINK_AUTH_UNVERIFIED.defaultSeverity).toBe('warning'); + }); +}); + describe('IssueCodeSchema', () => { it('IssueCodeSchema enumerates exactly the registry keys', () => { expect(new Set(IssueCodeSchema.options)).toEqual(new Set(Object.keys(CODE_REGISTRY))); diff --git a/packages/resources/src/external-link-cache.ts b/packages/resources/src/external-link-cache.ts index a4ef5e70..e22918a6 100644 --- a/packages/resources/src/external-link-cache.ts +++ b/packages/resources/src/external-link-cache.ts @@ -3,6 +3,14 @@ import { promises as fs } from 'node:fs'; import { safePath } from '@vibe-agent-toolkit/utils'; +/** + * Cache schema version. Incremented when CacheEntry shape changes; entries + * written by an older code path that read as a different version are treated + * as cache misses (rather than misparsed) — forward-compat for slice 3's + * content-cache additions and any future entry-shape evolution. + */ +const CACHE_VERSION = 1; + /** * Cache entry for external link validation results */ @@ -10,6 +18,12 @@ interface CacheEntry { statusCode: number; statusMessage: string; timestamp: number; + /** + * Schema version. Optional for backwards-compat: legacy entries without + * `version` are treated as a cache miss, forcing a refetch. Reading on + * version mismatch produces a miss rather than a parse error. + */ + version?: number; } /** @@ -156,6 +170,15 @@ export class ExternalLinkCache { return null; } + // Forward-compat: an entry written under a different (or missing) + // schema version is treated as a miss. Forces a refetch rather than + // silently misparsing a slice-3+ shape with slice-2 reader code. + if (entry.version !== CACHE_VERSION) { + delete cache[key]; + await this.saveCache(); + return null; + } + if (this.isExpired(entry)) { delete cache[key]; await this.saveCache(); @@ -180,6 +203,7 @@ export class ExternalLinkCache { statusCode, statusMessage, timestamp: Date.now(), + version: CACHE_VERSION, }; await this.saveCache(); diff --git a/packages/resources/src/external-link-validator.ts b/packages/resources/src/external-link-validator.ts index b432172d..b5ea4cde 100644 --- a/packages/resources/src/external-link-validator.ts +++ b/packages/resources/src/external-link-validator.ts @@ -1,6 +1,99 @@ +import { userInfo } from 'node:os'; + +import type { IssueCode } from '@vibe-agent-toolkit/agent-schema'; +import { + resolveAuthenticatedUrl, + safePath, + type LinkAuthConfig, + type ResolveOutcome, +} from '@vibe-agent-toolkit/utils'; import markdownLinkCheck from 'markdown-link-check'; import { ExternalLinkCache } from './external-link-cache.js'; +import { classifyAuthenticatedResponse } from './link-auth-classify.js'; +import { fetchAuthenticated } from './link-auth-fetch.js'; + +/** + * Resolve the OS user for cache scoping. Falls back through several layers + * because `os.userInfo()` throws when the running user has no /etc/passwd + * entry (common in container environments) — that's recoverable, not fatal. + */ +function resolveOsUser(): string { + try { + const u = userInfo().username; + if (u !== '') return u; + } catch { + // userInfo() throws when the running user has no /etc/passwd entry — + // recover via env vars below. + } + // Use `||` (not `??`) so an empty-string USER/USERNAME also falls through. + // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing + const fromEnv = process.env['USER'] || process.env['USERNAME']; + if (fromEnv) return fromEnv; + // Last resort: two distinct OS users on the same host both end up here + // would share the auth cache (cross-user leak). Warn once so the + // collision is at least observable. Adopters whose container clears + // these env vars deliberately can set `USER=container-name` to scope. + warnDefaultUserFallbackOnce(); + return 'default'; +} + +let defaultUserWarned = false; +function warnDefaultUserFallbackOnce(): void { + if (defaultUserWarned) return; + defaultUserWarned = true; + console.warn( + "[vat] linkAuth: could not determine an OS user via os.userInfo() or USER/USERNAME env; " + + "scoping auth cache to 'default'. Two users on this host would share auth-cached results — " + + 'set the USER env var explicitly to scope per-user.', + ); +} + +/** + * Make an OS username safe for use as a directory component. Replaces path + * separators (`/`, `\`), the parent-directory shorthand (`..`), and other + * characters that could escape the cacheDir. Windows can produce + * `DOMAIN\user` forms; tests inject pathological values like `../escaped`. + */ +function sanitizeOsUser(user: string): string { + // Replace any character outside `[A-Za-z0-9._-]` with `_`. Then collapse any + // remaining `..` so a name like `..foo` cannot recombine into a traversal. + const replaced = user.replaceAll(/[^A-Za-z0-9._-]/g, '_').replaceAll('..', '__'); + return replaced.length > 0 ? replaced : 'default'; +} + +type LinkAuthDeps = Parameters[2]; +type VerifiedPlan = Extract; + +/** + * Build a LinkValidationResult from a status code by re-classifying it under + * the current run's provider `check` block. Used by both the cache-hit and + * cache-miss paths so the two produce identical results for the same + * `(url, provider)` pair — the cache only persists `statusCode`, not the + * derived `code`, so the code must be re-derived on every read. + */ +function buildAuthResult( + originalUrl: string, + statusCode: number, + plan: VerifiedPlan, + cached: boolean, + cachedStatusMessage?: string, +): LinkValidationResult { + const classified = classifyAuthenticatedResponse(statusCode, plan.check); + const result: LinkValidationResult = { + url: originalUrl, + status: classified?.outcome === 'alive' ? 'ok' : 'error', + statusCode, + cached, + }; + if (classified?.code != null) { + result.code = classified.code; + } + if (result.status === 'error') { + result.error = cachedStatusMessage ?? `HTTP ${statusCode}`; + } + return result; +} /** * Safely serialize an error to a string, preventing [object Object] issues. @@ -51,6 +144,38 @@ export interface ExternalLinkValidatorOptions { retries?: number; /** User agent string for requests (default: generic) */ userAgent?: string; + /** + * Optional linkAuth config (per issue #113). When set, URLs whose host is + * claimed by a provider in this config bypass the anonymous markdown-link-check + * path and use an authenticated direct fetch instead. URLs no provider claims + * continue to use markdown-link-check. + */ + linkAuthConfig?: LinkAuthConfig; + /** + * Override the `fetch` implementation used by the authenticated branch. + * Defaults to `globalThis.fetch`. Tests inject a stub; advanced adopters + * may inject a wrapper for corporate proxies, custom TLS, or telemetry. + */ + fetchImpl?: typeof fetch; + /** + * Override the linkAuth engine's token-resolution dependencies (env map + + * runCommand). Defaults to reading `process.env` and running real commands + * via `safeExecSync`. Tests inject a deterministic env; advanced adopters + * usually leave unset. + */ + linkAuthDeps?: LinkAuthDeps; + /** + * Override the sleep used between 429 retries. Defaults to `setTimeout`. + * Test-only — production benefits from real wall-clock delay so Retry-After + * hints are honored. + */ + sleep?: (ms: number) => Promise; + /** + * OS user to scope the authenticated cache by (#113 §6.3). Defaults to + * `os.userInfo().username` with env-var fallbacks. Tests inject a fixed + * value; production callers omit. + */ + osUser?: string; } /** @@ -67,6 +192,21 @@ export interface LinkValidationResult { error?: string; /** Whether result came from cache */ cached: boolean; + /** + * Set when the authenticated branch classified the response (issue #113 §7). + * Consumers use this directly instead of mapping `statusCode` to a code, so + * `notFoundMeaning`-dependent routing (404 → `LINK_AUTH_DEAD` vs + * `LINK_AUTH_DEAD_OR_UNAUTHORIZED`) reflects the matched provider's config. + * + * Invariant: `code` is set iff the URL hit the authenticated branch AND + * the outcome maps to a `LINK_AUTH_*` code per §7 (`unverified`, or any + * classified status: `unauthorized`, `forbidden`, `dead`, + * `dead_or_unauthorized`). Unset on the anonymous markdown-link-check path, + * on `unsupported` (no provider claimed the host), and on classifier-`null` + * statuses (5xx, unclassified) where the consumer's status-code mapping + * (`EXTERNAL_URL_*`) is the correct fallback. + */ + code?: IssueCode; } /** @@ -90,7 +230,23 @@ export interface LinkValidationResult { */ export class ExternalLinkValidator { private readonly cache: ExternalLinkCache; - private readonly options: Required; + /** + * Auth-branch cache — scoped to a per-OS-user subdirectory of `cacheDir` + * (#113 §6.3) so two users on a shared machine never read each other's + * authenticated results. Distinct from `cache` (the anonymous cache, which + * stays shared across users for the markdown-link-check path). + */ + private readonly authCache: ExternalLinkCache; + private readonly options: { + cacheTtlHours: number; + timeout: number; + retries: number; + userAgent: string; + }; + private readonly linkAuthConfig: LinkAuthConfig | undefined; + private readonly fetchImpl: typeof fetch; + private readonly linkAuthDeps: LinkAuthDeps; + private readonly sleep: ((ms: number) => Promise) | undefined; /** * Create a new external link validator @@ -108,7 +264,16 @@ export class ExternalLinkValidator { 'Mozilla/5.0 (compatible; VAT-LinkChecker/1.0; +https://github.com/jdutton/vibe-agent-toolkit)', }; + this.linkAuthConfig = options.linkAuthConfig; + this.fetchImpl = options.fetchImpl ?? globalThis.fetch; + this.linkAuthDeps = options.linkAuthDeps; + this.sleep = options.sleep; + this.cache = new ExternalLinkCache(cacheDir, this.options.cacheTtlHours); + + const osUser = sanitizeOsUser(options.osUser ?? resolveOsUser()); + const authCacheDir = safePath.join(cacheDir, `auth-${osUser}`); + this.authCache = new ExternalLinkCache(authCacheDir, this.options.cacheTtlHours); } /** @@ -118,6 +283,29 @@ export class ExternalLinkValidator { * @returns Validation result */ async validateLink(url: string): Promise { + // Authenticated branch (issue #113): if a provider in linkAuthConfig + // claims this URL's host, bypass markdown-link-check and do a direct + // authenticated fetch + per-§7 classify. + if (this.linkAuthConfig) { + const plan = resolveAuthenticatedUrl(url, this.linkAuthConfig, this.linkAuthDeps); + if ('fetchUrl' in plan) { + return this.validateAuthenticatedLink(url, plan); + } + if (plan.outcome === 'unverified') { + // Per §6.3: never cache unverified outcomes — the result flips the + // moment a token appears, so caching a "no token" result is wrong. + return { + url, + status: 'error', + statusCode: 0, + error: plan.reason, + cached: false, + code: 'LINK_AUTH_UNVERIFIED', + }; + } + // 'unsupported' → fall through to anonymous markdown-link-check path + } + // Check cache first const cached = await this.cache.get(url); if (cached) { @@ -165,6 +353,70 @@ export class ExternalLinkValidator { return Promise.all(urls.map((url) => this.validateLink(url))); } + /** + * Issue an authenticated fetch for `originalUrl` using the engine's plan + * (rewritten URL + auth headers + provider's check config), classify the + * response per #113 §7, and write a status-cache entry. Cache is keyed by + * the *rewritten* URL (§6.3) — the original `blob/` URL 404s, so caching + * by original would poison results. + */ + private async validateAuthenticatedLink( + originalUrl: string, + plan: VerifiedPlan, + ): Promise { + // Cache check, keyed by rewritten URL. IMPORTANT: re-classify the cached + // statusCode under the *current* run's provider `check` block, rather + // than treating cache-hit as a status-only short-circuit. The cache + // only persists `statusCode` — without re-classifying we'd drop the + // LINK_AUTH_* `code`, and the consumer would fall back to + // `EXTERNAL_URL_DEAD` (error) instead of `LINK_AUTH_DEAD_OR_UNAUTHORIZED` + // (warning). Cache-hit semantics must match cache-miss semantics for + // the same (url, provider) pair. + const cached = await this.authCache.get(plan.fetchUrl); + if (cached) { + return buildAuthResult(originalUrl, cached.statusCode, plan, true, cached.statusMessage); + } + + // Fresh fetch via the auth-aware helper (handles cross-origin redirect + // Authorization stripping + 429/Retry-After per §5.2 §8). The conditional + // spread builds the object in one pass — AuthFetchOptions.sleep is + // readonly so post-construction assignment would be a TS error. + const fetchOptions: Parameters[3] = { + signal: AbortSignal.timeout(this.options.timeout), + ...(this.sleep === undefined ? {} : { sleep: this.sleep }), + }; + + let response: Response; + try { + response = await fetchAuthenticated( + plan.fetchUrl, + plan.headers, + this.fetchImpl, + fetchOptions, + ); + } catch (err) { + // Network-level failure (DNS/connect/TLS/timeout) — return error with + // no code so the consumer's existing statusCode→IssueCode mapping + // (EXTERNAL_URL_ERROR / EXTERNAL_URL_TIMEOUT) applies. + const message = safeSerializeError(err) ?? 'Authenticated fetch failed'; + return { + url: originalUrl, + status: 'error', + statusCode: 0, + error: message, + cached: false, + }; + } + + const result = buildAuthResult(originalUrl, response.status, plan, false); + // Persist by rewritten URL so subsequent runs hit the cache. The cache + // only stores statusCode + statusMessage; the `code` is re-derived on + // read by re-running classifyAuthenticatedResponse against the current + // provider (see the authCache.get branch above). + await this.authCache.set(plan.fetchUrl, result.statusCode, result.error ?? 'OK'); + return result; + } + /** * Check a link using markdown-link-check */ @@ -235,16 +487,21 @@ export class ExternalLinkValidator { } /** - * Clear the validation cache + * Clear both validation caches (anonymous + authenticated). + * + * Adopters call this after rotating a token or invalidating link data — + * the operation must wipe both surfaces or stale auth results survive + * (a real bug: a 401 cached under the old token would haunt the new one). */ async clearCache(): Promise { - await this.cache.clear(); + await Promise.all([this.cache.clear(), this.authCache.clear()]); } /** - * Get cache statistics + * Get combined cache statistics across both caches. */ async getCacheStats(): Promise<{ total: number; expired: number }> { - return this.cache.getStats(); + const [anon, auth] = await Promise.all([this.cache.getStats(), this.authCache.getStats()]); + return { total: anon.total + auth.total, expired: anon.expired + auth.expired }; } } diff --git a/packages/resources/src/link-auth-classify.ts b/packages/resources/src/link-auth-classify.ts new file mode 100644 index 00000000..be6ca4b3 --- /dev/null +++ b/packages/resources/src/link-auth-classify.ts @@ -0,0 +1,61 @@ +/** + * Classify a response from an authenticated external-link fetch into one of + * the outcomes defined in design issue #113 §7. + * + * Pure function: takes the HTTP status from a completed fetch plus the + * matched provider's `check` block, returns the outcome name and the matching + * `LINK_AUTH_*` code from `CODE_REGISTRY`. Pre-fetch outcomes (`unsupported`, + * `unverified`) are produced upstream by `resolveAuthenticatedUrl` and never + * reach this classifier. + * + * Status codes that don't match any classified outcome return `null` — the + * caller falls through to the existing anonymous `EXTERNAL_URL_DEAD` path + * (per §7 "unsupported"), since the authenticated fetch produced no signal + * the per-provider table can speak to. + */ + +import type { ProviderCheck } from '@vibe-agent-toolkit/utils'; + +export type LinkAuthOutcome = + | { readonly outcome: 'alive'; readonly code: null } + | { readonly outcome: 'dead'; readonly code: 'LINK_AUTH_DEAD' } + | { + readonly outcome: 'dead_or_unauthorized'; + readonly code: 'LINK_AUTH_DEAD_OR_UNAUTHORIZED'; + } + | { readonly outcome: 'forbidden'; readonly code: 'LINK_AUTH_FORBIDDEN' } + | { readonly outcome: 'unauthorized'; readonly code: 'LINK_AUTH_UNAUTHORIZED' }; + +export function classifyAuthenticatedResponse( + status: number, + check: ProviderCheck, +): LinkAuthOutcome | null { + // aliveStatus wins first: a provider can in principle declare any code as + // alive (e.g. a future host where 204 means alive), so the membership test + // runs before the well-known-status branches. + if (check.aliveStatus.includes(status)) { + return { outcome: 'alive', code: null }; + } + + if (status === 401) { + return { outcome: 'unauthorized', code: 'LINK_AUTH_UNAUTHORIZED' }; + } + + if (status === 403) { + return { outcome: 'forbidden', code: 'LINK_AUTH_FORBIDDEN' }; + } + + if (status === 404 || status === 410) { + if (check.notFoundMeaning === 'dead') { + return { outcome: 'dead', code: 'LINK_AUTH_DEAD' }; + } + return { + outcome: 'dead_or_unauthorized', + code: 'LINK_AUTH_DEAD_OR_UNAUTHORIZED', + }; + } + + // 2xx not in aliveStatus, 3xx redirects, 429 rate-limit, 5xx server errors + // — none of these are classified per §7. Caller decides what to do. + return null; +} diff --git a/packages/resources/src/link-auth-config-build.ts b/packages/resources/src/link-auth-config-build.ts new file mode 100644 index 00000000..a9444d68 --- /dev/null +++ b/packages/resources/src/link-auth-config-build.ts @@ -0,0 +1,108 @@ +/** + * Bridge between the adopter-facing linkAuth config (Zod-validated, with + * macro references) and the engine-facing config (fully-expanded providers). + * + * Adopter config providers can be either: + * - `{ use: , ...overrides }` — reference a shipped macro; deep-merge + * `overrides` on top of the macro's defaults + * - A full inline `{ match, rewrite, auth, token, check }` — used as-is + * + * This function walks each provider entry, runs macro expansion when needed, + * and re-validates each fully-expanded provider against the inline schema + * (the macro schema's `.passthrough()` accepts unknown override keys, which + * silently no-op at expansion time — post-expansion validation is where + * typo'd override fields surface as errors, per #113 §5). + * + * Per design issue #113 §5 (macros are config, not a privileged code path) + * and §4 (engine vocabulary). + */ + +import { + expandMacro, + type LinkAuthConfig, + type Provider, +} from '@vibe-agent-toolkit/utils'; +import type { z } from 'zod'; + +import { InlineProviderSchema, type LinkAuthProjectConfig } from './schemas/link-auth.js'; + +/** + * Compile-time drift defense: top-level field sets must match between the + * Zod schema and the engine's `Provider` interface. A strict structural + * check would also catch sub-type drift, but TS variance (readonly arrays in + * engine vs mutable in Zod inference) makes that noisy — the realistic + * drift mode is adding/renaming a top-level field on one side and forgetting + * the other, which this top-level key comparison catches at `tsc` time. + * + * The schema and engine type are kept in sync by review (per slice 1 design + * decision); this guard makes that review easier. + */ +type _SchemaKeys = keyof z.infer; +type _EngineKeys = keyof Provider; +type _KeysAgree = [_SchemaKeys] extends [_EngineKeys] + ? [_EngineKeys] extends [_SchemaKeys] + ? true + : { error: 'engine Provider has a field that InlineProviderSchema lacks'; missing: Exclude<_EngineKeys, _SchemaKeys> } + : { error: 'InlineProviderSchema has a field that engine Provider lacks'; missing: Exclude<_SchemaKeys, _EngineKeys> }; +// Type-level assert: the declaration must compile to `true` (i.e. both +// directions of the key-set check pass). The exported function holds a +// reference to the type so noUnusedLocals doesn't fire. +export const _assertSchemaKeysAgreeWithEngine: _KeysAgree = true; + +export function buildLinkAuthEngineConfig(adopter: LinkAuthProjectConfig): LinkAuthConfig { + const providers: Provider[] = adopter.providers.map((entry, index) => + expandProviderEntry(entry, index), + ); + return { providers }; +} + +function expandProviderEntry(entry: unknown, index: number): Provider { + // Discriminate via Object.hasOwn (not `'use' in entry`) so a prototype- + // injected `use` cannot reroute an inline entry into macro expansion. The + // Zod parser already produces plain JSON-shaped objects, but defending in + // depth costs nothing. + const isMacroRef = + typeof entry === 'object' && + entry !== null && + Object.hasOwn(entry, 'use'); + + if (!isMacroRef) { + return entry as Provider; + } + + const { use, overrides } = splitMacroEntry(entry as Record); + const expanded = expandMacro(use, overrides); + + // Re-validate the fully-expanded shape. A macro override that uses a typo'd + // field name (e.g. `notFoundMeaningg`) is accepted by MacroProviderSchema's + // .passthrough(), survives expansion as a stray field on the merged object, + // and is caught here by InlineProviderSchema's .strict() — surfaced as a + // config error rather than as silently-wrong runtime behavior. + const parsed = InlineProviderSchema.safeParse(expanded); + if (!parsed.success) { + throw new Error( + `linkAuth providers[${index}] (use: ${JSON.stringify(use)}) ` + + `produced an invalid provider after macro expansion: ${parsed.error.message}`, + ); + } + return parsed.data as Provider; +} + +function splitMacroEntry(entry: Record): { + use: string; + overrides: Record; +} { + // `use` value comes from a Zod-validated source so the type is string at + // this point — but defensively-check it anyway: `Object.hasOwn` upstream + // tells us the key exists, not what its value's type is. + const useValue = entry['use']; + if (typeof useValue !== 'string') { + throw new TypeError(`linkAuth provider \`use\` must be a string, got ${typeof useValue}`); + } + const overrides: Record = {}; + for (const [key, value] of Object.entries(entry)) { + if (key === 'use') continue; + overrides[key] = value; + } + return { use: useValue, overrides }; +} diff --git a/packages/resources/src/link-auth-fetch.ts b/packages/resources/src/link-auth-fetch.ts new file mode 100644 index 00000000..efceb98b --- /dev/null +++ b/packages/resources/src/link-auth-fetch.ts @@ -0,0 +1,177 @@ +/** + * Authenticated fetch primitive for link checking (design issue #113 §5.2, §8). + * + * Wraps a caller-supplied `fetchImpl` with two behaviors required by the design: + * + * §8 (cross-origin token leak defense) — `redirect: 'manual'`, then a + * bounded loop that follows Location headers. On any redirect to a + * different origin the `Authorization` header is stripped and stays + * stripped for subsequent hops (defeats token-laundering via a cross-origin + * bounce back to the original host). + * + * §5.2 (rate-limit handling) — on HTTP 429, parse `Retry-After` (seconds or + * HTTP-date), wait, retry. Bounded by `maxRetries` so a stuck host cannot + * hang validation; bounded by `maxRetryAfterMs` so a hostile or buggy host + * cannot pin a validation run for an hour with `Retry-After: 86400`. + * + * Pure-ish: `fetchImpl` and `sleep` are dependency-injected so tests do not + * touch the network or wall-clock. Production callers pass `globalThis.fetch`. + */ + +export interface AuthFetchOptions { + /** Maximum redirect hops to follow before returning the last 3xx response (default: 5). */ + readonly maxRedirects?: number; + /** Maximum 429 retries before returning the final 429 response (default: 2). */ + readonly maxRetries?: number; + /** Cap on Retry-After in ms; defends against hostile/buggy long values (default: 60_000). */ + readonly maxRetryAfterMs?: number; + /** Abort signal propagated to every fetchImpl call (validator's per-request timeout budget). */ + readonly signal?: AbortSignal; + /** Test-only sleep injection. Production callers omit. */ + readonly sleep?: (ms: number) => Promise; +} + +const DEFAULT_MAX_REDIRECTS = 5; +const DEFAULT_MAX_RETRIES = 2; +const DEFAULT_MAX_RETRY_AFTER_MS = 60_000; +/** + * Minimum delay between 429 retries. A hostile host can send `Retry-After: 0` + * (or an HTTP-date in the past, parsing to 0); without a floor, we'd retry + * immediately, which is worse-than-useless for the host's rate-limiting and + * makes us a poor neighbor. Bounded by `maxRetries` regardless, but the floor + * gives the host's window a chance to slide. + */ +const MIN_RETRY_AFTER_MS = 250; + +const defaultSleep = (ms: number): Promise => + new Promise((resolve) => { + setTimeout(resolve, ms); + }); + +/** + * Parse a `Retry-After` header value into milliseconds-to-wait. + * + * Returns `null` when the value is missing, empty, negative, or unparseable — + * callers treat `null` as "don't retry" (we can't pick a delay without a hint). + * HTTP-date values in the past return `0` (the server's hint is "you can retry + * now"). RFC 7231 §7.1.3. + */ +export function parseRetryAfter(value: string | null): number | null { + if (value === null) return null; + const trimmed = value.trim(); + if (trimmed === '') return null; + + // delta-seconds form: positive integer count of seconds. + if (/^\d+$/.test(trimmed)) { + return Number(trimmed) * 1000; + } + + // HTTP-date form per RFC 7231 §7.1.1.1 — always contains weekday and month + // names. Guard with /[A-Za-z]/ because Date.parse() is permissive: '-5' + // parses as year-5 BC, '0' as 1 BC, etc. Requiring at least one letter + // restricts us to real HTTP-date strings. + if (!/[A-Za-z]/.test(trimmed)) return null; + const parsedMs = Date.parse(trimmed); + if (!Number.isFinite(parsedMs)) return null; + return Math.max(0, parsedMs - Date.now()); +} + +export async function fetchAuthenticated( + url: string, + headers: Record, + fetchImpl: typeof fetch, + options: AuthFetchOptions = {}, +): Promise { + const maxRedirects = options.maxRedirects ?? DEFAULT_MAX_REDIRECTS; + const maxRetries = options.maxRetries ?? DEFAULT_MAX_RETRIES; + const maxRetryAfterMs = options.maxRetryAfterMs ?? DEFAULT_MAX_RETRY_AFTER_MS; + const sleep = options.sleep ?? defaultSleep; + + let currentUrl = url; + let currentHeaders: Record = { ...headers }; + let redirects = 0; + let retries = 0; + + // Bounded loop: every iteration either returns or strictly advances one of + // (retries, redirects), both capped, so termination is guaranteed within + // `maxRetries + maxRedirects + 1` fetchImpl calls. + while (redirects + retries < maxRedirects + maxRetries + 1) { + const init: RequestInit = { + headers: currentHeaders, + redirect: 'manual', + }; + if (options.signal !== undefined) init.signal = options.signal; + + const response = await fetchImpl(currentUrl, init); + + const retryDelay = + retries < maxRetries ? computeRetryDelay(response, maxRetryAfterMs) : null; + if (retryDelay !== null) { + await sleep(retryDelay); + retries++; + continue; + } + + if (redirects < maxRedirects) { + const next = computeRedirect(response, currentUrl, currentHeaders); + if (next !== null) { + currentUrl = next.url; + currentHeaders = next.headers; + redirects++; + continue; + } + } + + return response; + } + // Unreachable: the loop returns or continues on every iteration, and the + // loop condition strictly bounds total iterations. + throw new Error('fetchAuthenticated: unreachable iteration cap exceeded'); +} + +/** + * Decide the wait-time for a 429 response, or `null` if the response is not + * a 429 OR has no parseable Retry-After hint (no hint → caller returns the 429 + * rather than retrying blindly). + */ +function computeRetryDelay(response: Response, maxRetryAfterMs: number): number | null { + if (response.status !== 429) return null; + const retryAfter = parseRetryAfter(response.headers.get('retry-after')); + if (retryAfter === null) return null; + // Two-sided clamp: never longer than the DoS cap, never shorter than the + // good-neighbor floor (defends against `Retry-After: 0` busy-loops). + return Math.min(Math.max(retryAfter, MIN_RETRY_AFTER_MS), maxRetryAfterMs); +} + +/** + * Decide the next hop for a 3xx redirect, or `null` if the response is not a + * redirect / has no Location. Strips Authorization on cross-origin per §8. + */ +function computeRedirect( + response: Response, + currentUrl: string, + currentHeaders: Record, +): { url: string; headers: Record } | null { + if (response.status < 300 || response.status >= 400) return null; + const location = response.headers.get('location'); + if (location === null) return null; + const nextUrl = new URL(location, currentUrl).toString(); + const sameOrigin = new URL(nextUrl).origin === new URL(currentUrl).origin; + return { + url: nextUrl, + headers: sameOrigin ? currentHeaders : stripAuthorization(currentHeaders), + }; +} + +/** + * Remove any header whose name is `Authorization` (case-insensitive). Returns + * a new object — does not mutate the input. + */ +function stripAuthorization(headers: Record): Record { + const result: Record = {}; + for (const [name, value] of Object.entries(headers)) { + if (name.toLowerCase() === 'authorization') continue; + result[name] = value; + } + return result; +} diff --git a/packages/resources/src/resource-registry.ts b/packages/resources/src/resource-registry.ts index 6c844ac1..7aec8f6f 100644 --- a/packages/resources/src/resource-registry.ts +++ b/packages/resources/src/resource-registry.ts @@ -22,6 +22,7 @@ import { type FrontmatterExternalUrl, } from './frontmatter-link-validator.js'; import { validateFrontmatter } from './frontmatter-validator.js'; +import { buildLinkAuthEngineConfig } from './link-auth-config-build.js'; import { parseMarkdown } from './link-parser.js'; import { validateLink, type ValidateLinkOptions } from './link-validator.js'; import type { ResourceCollectionInterface } from './resource-collection-interface.js'; @@ -757,10 +758,20 @@ export class ResourceRegistry implements ResourceCollectionInterface { // Determine cache directory const cacheDir = this.getCacheDirectory(); + // Expand any `resources.linkAuth` providers from macro refs into the engine + // shape (#113 §5). When the adopter has no linkAuth config, the validator + // skips the authenticated branch and uses the existing markdown-link-check + // path for every URL — identical to pre-#113 behavior. + const adopterLinkAuth = this.config?.resources?.linkAuth; + const linkAuthConfig = adopterLinkAuth + ? buildLinkAuthEngineConfig(adopterLinkAuth) + : undefined; + // Create validator const validator = new ExternalLinkValidator(cacheDir, { timeout: 15000, cacheTtlHours: noCache ? 0 : 24, + ...(linkAuthConfig !== undefined && { linkAuthConfig }), }); // Collect all external URLs from all resources @@ -830,7 +841,7 @@ export class ResourceRegistry implements ResourceCollectionInterface { * @private */ private convertValidationResultsToIssues( - results: Array<{ url: string; status: 'ok' | 'error'; statusCode: number; error?: string }>, + results: Array<{ url: string; status: 'ok' | 'error'; statusCode: number; error?: string; code?: IssueCode }>, urlsToValidate: Map>, ): ValidationIssue[] { const issues: ValidationIssue[] = []; @@ -845,7 +856,11 @@ export class ResourceRegistry implements ResourceCollectionInterface { continue; } - const issueCode = this.determineExternalUrlIssueCode(result.statusCode, result.error); + // Prefer a code surfaced by the validator's authenticated branch (#113 §7) — + // those LINK_AUTH_* codes encode per-provider notFoundMeaning routing that + // statusCode alone cannot express. Fall back to the statusCode mapping for + // the anonymous markdown-link-check path. + const issueCode = result.code ?? this.determineExternalUrlIssueCode(result.statusCode, result.error); const errorMessage = result.error ?? `HTTP ${result.statusCode}`; for (const location of locations) { diff --git a/packages/resources/src/schemas/link-auth.ts b/packages/resources/src/schemas/link-auth.ts index fb20db02..c2a757b1 100644 --- a/packages/resources/src/schemas/link-auth.ts +++ b/packages/resources/src/schemas/link-auth.ts @@ -12,7 +12,8 @@ * are NOT strictly typed at this layer because they must match the * macro's shape, which we cannot see until expansion. A typo in an * override silently no-ops; the full provider shape is validated - * after expansion (slice 2 wires that). + * after expansion (`buildLinkAuthEngineConfig` validates the + * fully-expanded provider against `InlineProviderSchema`). * - A full inline `{ match, rewrite, auth, token, check }` — every field * required, every nested object strict. * @@ -92,7 +93,7 @@ export const ProviderCheckSchema = z // Provider entry (inline OR macro reference) // --------------------------------------------------------------------------- -const InlineProviderSchema = z +export const InlineProviderSchema = z .object({ match: ProviderMatchSchema, rewrite: z.array(RewriteRuleSchema).min(1).describe('Ordered rewrite rules — first matching wins'), @@ -140,7 +141,13 @@ export const LinkAuthConfigSchema = z .strict() .describe('`resources.linkAuth` — authenticated external link resolution config'); -export type LinkAuthConfig = z.infer; +/** + * Adopter-facing config shape — what an `vibe-agent-toolkit.config.yaml` + * parses to. Distinct from `@vibe-agent-toolkit/utils`'s `LinkAuthConfig` + * (the engine shape, with fully-expanded providers only). The bridge + * function `buildLinkAuthEngineConfig` converts between the two. + */ +export type LinkAuthProjectConfig = z.infer; export type ProviderEntry = z.infer; export type RewriteRule = z.infer; export type TokenSource = z.infer; diff --git a/packages/resources/test/auth-fetch-mocks.ts b/packages/resources/test/auth-fetch-mocks.ts new file mode 100644 index 00000000..f9858659 --- /dev/null +++ b/packages/resources/test/auth-fetch-mocks.ts @@ -0,0 +1,89 @@ +/** + * Test-only mocks for `fetch`-shaped functions, used by the linkAuth tests + * in this package and intended for reuse by slice 3+ content-fetch tests. + * + * Three patterns: + * - `sequenceFetch(responses)` — script an ordered sequence of responses + * with per-call URL/header assertions. Used by `link-auth-fetch.test.ts` + * to test redirect + 429 chains. + * - `countingFetch()` — observe whether `fetch` was called and how many + * times. Used to assert "unverified short-circuit doesn't call fetch." + * - `capturingFetch(extract)` — capture one piece of request data (URL or + * headers) for later assertion. Returns a 200 response so the caller + * can run an end-to-end flow. + * + * Not test files themselves (no `.test.ts` suffix); vitest's + * `test/...test.ts` include pattern skips this file. + */ + +export type ResponseSpec = { + readonly status: number; + readonly headers?: Record; + readonly assertUrl?: (url: string) => void; + readonly assertHeaders?: (headers: Record) => void; +}; + +/** + * Build a deterministic `fetchImpl` stub from an ordered list of responses. + * Each call consumes one response; calling beyond the list throws. + */ +export function sequenceFetch(responses: readonly ResponseSpec[]): typeof fetch { + let i = 0; + return (async (input: string | URL, init?: RequestInit): Promise => { + const spec = responses[i]; + if (spec === undefined) { + throw new Error(`sequenceFetch: ran out of responses after ${i} call(s)`); + } + i++; + const url = typeof input === 'string' ? input : input.toString(); + const headers = headersToObject(init?.headers); + spec.assertUrl?.(url); + spec.assertHeaders?.(headers); + return new Response(null, { + status: spec.status, + headers: spec.headers ?? {}, + }); + }) as typeof fetch; +} + +/** + * Build a `fetchImpl` that increments a counter on every call and returns an + * empty 200 response. Useful for "fetch must NOT be called" assertions. + */ +export function countingFetch(): { fetchImpl: typeof fetch; calls: () => number } { + let count = 0; + const fetchImpl = (async () => { + count++; + return new Response(); + }) as typeof fetch; + return { fetchImpl, calls: () => count }; +} + +/** + * Build a `fetchImpl` that captures one piece of request data (URL or headers) + * via the caller's extractor and always returns 200. Useful for end-to-end + * "what did the validator actually send?" assertions. + */ +export function capturingFetch( + extract: (url: string | URL, init?: RequestInit) => T, +): { fetchImpl: typeof fetch; getCaptured: () => T | undefined } { + let captured: T | undefined; + const fetchImpl = ((url: string | URL, init?: RequestInit) => { + captured = extract(url, init); + return Promise.resolve(new Response(null, { status: 200 })); + }) as typeof fetch; + return { fetchImpl, getCaptured: () => captured }; +} + +function headersToObject(headers: unknown): Record { + if (headers === undefined || headers === null) return {}; + if (headers instanceof Headers) { + const out: Record = {}; + for (const [k, v] of headers) { + out[k] = v; + } + return out; + } + if (Array.isArray(headers)) return Object.fromEntries(headers); + return { ...(headers as Record) }; +} diff --git a/packages/resources/test/external-link-cache.test.ts b/packages/resources/test/external-link-cache.test.ts index 67639fa1..d66f6629 100644 --- a/packages/resources/test/external-link-cache.test.ts +++ b/packages/resources/test/external-link-cache.test.ts @@ -30,6 +30,7 @@ describe('ExternalLinkCache', () => { statusCode: 200, statusMessage: 'OK', timestamp: expect.any(Number), + version: 1, }); }); @@ -70,6 +71,7 @@ describe('ExternalLinkCache', () => { statusCode: 404, statusMessage: 'Not Found', timestamp: expect.any(Number), + version: 1, }); }); @@ -82,6 +84,7 @@ describe('ExternalLinkCache', () => { statusCode: 404, statusMessage: 'Not Found', timestamp: expect.any(Number), + version: 1, }); }); @@ -110,6 +113,22 @@ describe('ExternalLinkCache', () => { expect(result?.statusCode).toBe(200); }); + it('treats entries with a missing version field as a cache miss (forward-compat)', async () => { + // Hand-craft a legacy cache file (pre-version) and confirm get() rejects it. + // Forward-compat per #113 — keeps slice 3+ free to evolve CacheEntry without + // risking misparse against pre-existing files. + const cacheFile = safePath.join(tempDir, 'external-links.json'); + const url = 'https://legacy.example.com'; + const crypto = await import('node:crypto'); + const key = crypto.createHash('sha256').update(url).digest('hex'); + const legacy = { [key]: { statusCode: 200, statusMessage: 'OK', timestamp: Date.now() } }; + // eslint-disable-next-line security/detect-non-literal-fs-filename -- tempDir from mkdtemp + await fs.writeFile(cacheFile, JSON.stringify(legacy)); + + const result = await cache.get(url); + expect(result).toBeNull(); + }); + it('should handle corrupted cache file gracefully', async () => { // Write invalid JSON to cache file const cacheFile = safePath.join(tempDir, 'external-links.json'); diff --git a/packages/resources/test/external-link-validator-auth.test.ts b/packages/resources/test/external-link-validator-auth.test.ts new file mode 100644 index 00000000..72e2f7c7 --- /dev/null +++ b/packages/resources/test/external-link-validator-auth.test.ts @@ -0,0 +1,390 @@ +import { createHash } from 'node:crypto'; +import { existsSync, readFileSync } from 'node:fs'; +import { mkdtemp, rm } from 'node:fs/promises'; + +import { normalizedTmpdir, safePath, type LinkAuthConfig, type Provider } from '@vibe-agent-toolkit/utils'; +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; + +import { ExternalLinkValidator } from '../src/external-link-validator.js'; + +import { capturingFetch, countingFetch } from './auth-fetch-mocks.js'; + +const TEST_TOKEN = 'gh_test_token_abc'; +const HOST = 'https://github.com/owner/repo/blob/main/file.md'; +const REWRITTEN = 'https://api.github.com/repos/owner/repo/contents/file.md?ref=main'; +const CACHE_FILE = 'external-links.json'; + +/** + * Path-derived `existsSync` check — wraps the lint disable in one place. The + * paths are tempDir-rooted, controlled by the test, not user input. + */ +function fsExists(p: string): boolean { + // eslint-disable-next-line security/detect-non-literal-fs-filename + return existsSync(p); +} + +function buildProvider(notFoundMeaning: 'ambiguous' | 'dead' = 'ambiguous'): Provider { + return { + match: { host: 'github.com' }, + rewrite: [ + { + when: String.raw`^https://github\.com/(?[^/]+)/(?[^/]+)/blob/(?[^/]+)/(?.+)$`, + to: 'https://api.github.com/repos/${owner}/${repo}/contents/${path}?ref=${ref}', + }, + ], + auth: { headers: { Authorization: 'Bearer ${token}', Accept: 'application/vnd.github+json' } }, + token: [{ env: 'TEST_GH_TOKEN' }], + check: { method: 'GET', aliveStatus: [200], notFoundMeaning }, + }; +} + +function configWithProvider(notFoundMeaning: 'ambiguous' | 'dead' = 'ambiguous'): LinkAuthConfig { + return { providers: [buildProvider(notFoundMeaning)] }; +} + +function stubFetch(status: number, extraHeaders: Record = {}): typeof fetch { + return (async () => new Response(null, { status, headers: extraHeaders })) as typeof fetch; +} + +const ENV_WITH_TOKEN = { TEST_GH_TOKEN: TEST_TOKEN }; +const ENV_EMPTY = {}; + +let tempDir: string; +beforeEach(async () => { + tempDir = await mkdtemp(safePath.join(normalizedTmpdir(), 'link-auth-validator-')); +}); +afterEach(async () => { + await rm(tempDir, { recursive: true, force: true }); +}); + +describe('ExternalLinkValidator — authenticated branch (verified)', () => { + it('200 → status=ok, no code, statusCode=200', async () => { + const validator = new ExternalLinkValidator(tempDir, { + linkAuthConfig: configWithProvider(), + linkAuthDeps: { env: ENV_WITH_TOKEN }, + fetchImpl: stubFetch(200), + }); + const result = await validator.validateLink(HOST); + expect(result.status).toBe('ok'); + expect(result.statusCode).toBe(200); + expect(result.code).toBeUndefined(); + }); + + it('401 → status=error, code=LINK_AUTH_UNAUTHORIZED', async () => { + const validator = new ExternalLinkValidator(tempDir, { + linkAuthConfig: configWithProvider(), + linkAuthDeps: { env: ENV_WITH_TOKEN }, + fetchImpl: stubFetch(401), + }); + const result = await validator.validateLink(HOST); + expect(result.status).toBe('error'); + expect(result.statusCode).toBe(401); + expect(result.code).toBe('LINK_AUTH_UNAUTHORIZED'); + }); + + it('403 → code=LINK_AUTH_FORBIDDEN', async () => { + const validator = new ExternalLinkValidator(tempDir, { + linkAuthConfig: configWithProvider(), + linkAuthDeps: { env: ENV_WITH_TOKEN }, + fetchImpl: stubFetch(403), + }); + const result = await validator.validateLink(HOST); + expect(result.code).toBe('LINK_AUTH_FORBIDDEN'); + }); + + it('404 with notFoundMeaning=ambiguous → code=LINK_AUTH_DEAD_OR_UNAUTHORIZED', async () => { + const validator = new ExternalLinkValidator(tempDir, { + linkAuthConfig: configWithProvider('ambiguous'), + linkAuthDeps: { env: ENV_WITH_TOKEN }, + fetchImpl: stubFetch(404), + }); + const result = await validator.validateLink(HOST); + expect(result.code).toBe('LINK_AUTH_DEAD_OR_UNAUTHORIZED'); + }); + + it('404 with notFoundMeaning=dead → code=LINK_AUTH_DEAD', async () => { + const validator = new ExternalLinkValidator(tempDir, { + linkAuthConfig: configWithProvider('dead'), + linkAuthDeps: { env: ENV_WITH_TOKEN }, + fetchImpl: stubFetch(404), + }); + const result = await validator.validateLink(HOST); + expect(result.code).toBe('LINK_AUTH_DEAD'); + }); + + it('500 (unclassified) → status=error, no code (consumer falls through to statusCode-mapping)', async () => { + const validator = new ExternalLinkValidator(tempDir, { + linkAuthConfig: configWithProvider(), + linkAuthDeps: { env: ENV_WITH_TOKEN }, + fetchImpl: stubFetch(500), + }); + const result = await validator.validateLink(HOST); + expect(result.status).toBe('error'); + expect(result.statusCode).toBe(500); + expect(result.code).toBeUndefined(); + }); +}); + +describe('ExternalLinkValidator — authenticated branch (unverified, no token)', () => { + it('returns LINK_AUTH_UNVERIFIED without calling fetch', async () => { + const { fetchImpl, calls } = countingFetch(); + const validator = new ExternalLinkValidator(tempDir, { + linkAuthConfig: configWithProvider(), + linkAuthDeps: { env: ENV_EMPTY }, + fetchImpl, + }); + const result = await validator.validateLink(HOST); + expect(result.code).toBe('LINK_AUTH_UNVERIFIED'); + expect(result.status).toBe('error'); + expect(calls()).toBe(0); + }); +}); + +describe('ExternalLinkValidator — engine sends rewritten URL + auth headers', () => { + it('passes the rewritten URL (not the original) to fetchImpl', async () => { + const { fetchImpl, getCaptured } = capturingFetch((url) => + typeof url === 'string' ? url : url.toString(), + ); + const validator = new ExternalLinkValidator(tempDir, { + linkAuthConfig: configWithProvider(), + linkAuthDeps: { env: ENV_WITH_TOKEN }, + fetchImpl, + }); + await validator.validateLink(HOST); + expect(getCaptured()).toBe(REWRITTEN); + }); + + it('passes the auth headers (with resolved token) to fetchImpl', async () => { + const { fetchImpl, getCaptured } = capturingFetch( + (_url, init) => (init?.headers ?? {}) as Record, + ); + const validator = new ExternalLinkValidator(tempDir, { + linkAuthConfig: configWithProvider(), + linkAuthDeps: { env: ENV_WITH_TOKEN }, + fetchImpl, + }); + await validator.validateLink(HOST); + const headers = getCaptured() ?? {}; + expect(headers['Authorization']).toBe(`Bearer ${TEST_TOKEN}`); + expect(headers['Accept']).toBe('application/vnd.github+json'); + }); +}); + +describe('ExternalLinkValidator — cache hit preserves LINK_AUTH_* code (regression: do not silently demote to EXTERNAL_URL_DEAD)', () => { + it('two consecutive 404 fetches yield the same code on cache hit as on miss', async () => { + let fetchCount = 0; + const fetchImpl = (async () => { + fetchCount++; + return new Response(null, { status: 404 }); + }) as typeof fetch; + + const validator = new ExternalLinkValidator(tempDir, { + linkAuthConfig: configWithProvider('ambiguous'), + linkAuthDeps: { env: ENV_WITH_TOKEN }, + fetchImpl, + }); + + const first = await validator.validateLink(HOST); + const second = await validator.validateLink(HOST); + + // Cache miss: classifier ran against the provider; code is set. + expect(first.cached).toBe(false); + expect(first.code).toBe('LINK_AUTH_DEAD_OR_UNAUTHORIZED'); + + // Cache hit: classifier re-ran with the same provider, code preserved. + expect(second.cached).toBe(true); + expect(second.code).toBe('LINK_AUTH_DEAD_OR_UNAUTHORIZED'); + expect(second.statusCode).toBe(404); + + // Only one network call — the second result really came from cache. + expect(fetchCount).toBe(1); + }); + + it('cache hit re-classifies under the current provider (notFoundMeaning change between runs)', async () => { + const fetchImpl = (async () => new Response(null, { status: 404 })) as typeof fetch; + + // First run: notFoundMeaning='ambiguous' → cache-write LINK_AUTH_DEAD_OR_UNAUTHORIZED. + const v1 = new ExternalLinkValidator(tempDir, { + linkAuthConfig: configWithProvider('ambiguous'), + linkAuthDeps: { env: ENV_WITH_TOKEN }, + fetchImpl, + }); + const ambiguous = await v1.validateLink(HOST); + expect(ambiguous.code).toBe('LINK_AUTH_DEAD_OR_UNAUTHORIZED'); + + // Second run: same cacheDir, but provider's notFoundMeaning flipped to 'dead'. + // The cache hit must re-classify under the NEW provider, not return the + // cached cassette's interpretation. + const v2 = new ExternalLinkValidator(tempDir, { + linkAuthConfig: configWithProvider('dead'), + linkAuthDeps: { env: ENV_WITH_TOKEN }, + fetchImpl, + }); + const dead = await v2.validateLink(HOST); + expect(dead.cached).toBe(true); + expect(dead.code).toBe('LINK_AUTH_DEAD'); + }); +}); + +describe('ExternalLinkValidator — auth cache scoping (#113 §6.3)', () => { + it('cache key is the REWRITTEN URL, not the original', async () => { + const validator = new ExternalLinkValidator(tempDir, { + linkAuthConfig: configWithProvider(), + linkAuthDeps: { env: ENV_WITH_TOKEN }, + fetchImpl: stubFetch(200), + osUser: 'testuser', + }); + await validator.validateLink(HOST); + + const cacheFile = safePath.join(tempDir, 'auth-testuser', CACHE_FILE); + expect(fsExists(cacheFile)).toBe(true); + + // eslint-disable-next-line security/detect-non-literal-fs-filename -- test reads its own write + const cacheData = JSON.parse(readFileSync(cacheFile, 'utf8')) as Record; + const expectedKey = createHash('sha256').update(REWRITTEN).digest('hex'); + const originalKey = createHash('sha256').update(HOST).digest('hex'); + expect(Object.hasOwn(cacheData, expectedKey)).toBe(true); + expect(Object.hasOwn(cacheData, originalKey)).toBe(false); + }); + + it('auth cache lives under cacheDir/auth-${osUser} (not the shared anonymous cache)', async () => { + const validator = new ExternalLinkValidator(tempDir, { + linkAuthConfig: configWithProvider(), + linkAuthDeps: { env: ENV_WITH_TOKEN }, + fetchImpl: stubFetch(200), + osUser: 'alice', + }); + await validator.validateLink(HOST); + + // Auth cache file written under the user-scoped sub-directory. + expect(fsExists(safePath.join(tempDir, 'auth-alice', CACHE_FILE))).toBe(true); + // Shared anonymous cache file NOT touched (auth result didn't leak into it). + expect(fsExists(safePath.join(tempDir, CACHE_FILE))).toBe(false); + }); + + it('two validators with different osUser do NOT share auth-cache state', async () => { + let fetchCount = 0; + const fetchImpl = (async () => { + fetchCount++; + return new Response(null, { status: 200 }); + }) as typeof fetch; + + const aliceValidator = new ExternalLinkValidator(tempDir, { + linkAuthConfig: configWithProvider(), + linkAuthDeps: { env: ENV_WITH_TOKEN }, + fetchImpl, + osUser: 'alice', + }); + const bobValidator = new ExternalLinkValidator(tempDir, { + linkAuthConfig: configWithProvider(), + linkAuthDeps: { env: ENV_WITH_TOKEN }, + fetchImpl, + osUser: 'bob', + }); + + const aliceFirst = await aliceValidator.validateLink(HOST); + const bobFirst = await bobValidator.validateLink(HOST); + expect(aliceFirst.cached).toBe(false); + expect(bobFirst.cached).toBe(false); // Bob does NOT see Alice's cache. + expect(fetchCount).toBe(2); // Both ran the network call. + + // But Alice's second call hits Alice's own cache. + const aliceSecond = await aliceValidator.validateLink(HOST); + expect(aliceSecond.cached).toBe(true); + expect(fetchCount).toBe(2); + }); + + // Table-driven sanitizer test: every pathological osUser must produce a + // cache directory that lives strictly inside `tempDir`. The exact sanitized + // form is an implementation detail; the security property is "no escape". + const SANITIZER_INPUTS = [ + '..', + '../escaped', + '.', + '/', + String.raw`\..\..`, + 'a/../b', + '', + '...', + '-..-', + ]; + for (const badUser of SANITIZER_INPUTS) { + it(`sanitizes path-traversal-shaped osUser ${JSON.stringify(badUser)} (stays inside cacheDir)`, async () => { + const validator = new ExternalLinkValidator(tempDir, { + linkAuthConfig: configWithProvider(), + linkAuthDeps: { env: ENV_WITH_TOKEN }, + fetchImpl: stubFetch(200), + osUser: badUser, + }); + await validator.validateLink(HOST); + // Whatever the sanitized name is, the resulting auth-* directory must + // be a direct child of tempDir (no `..` escape, no slash-injected + // grandchild path). + const { readdirSync, statSync } = await import('node:fs'); + const entries = readdirSync(tempDir); + const authDir = entries.find((n) => n.startsWith('auth-')); + expect(authDir, `no auth-* dir for osUser=${JSON.stringify(badUser)}`).toBeDefined(); + if (authDir === undefined) return; // unreachable; satisfies the type guard + // The auth dir name must not itself contain a path separator or `..` + // (defense-in-depth — even if safePath.join cleaned, the persisted + // directory name shouldn't carry traversal-shaped fragments). + expect(authDir.includes('/')).toBe(false); + expect(authDir.includes('\\')).toBe(false); + expect(authDir.includes('..')).toBe(false); + const full = safePath.join(tempDir, authDir); + expect(statSync(full).isDirectory()).toBe(true); + }); + } + + it('sanitizes path-traversal characters in osUser', async () => { + // OS usernames are normally ASCII, but Windows can produce DOMAIN\user + // forms. The cache directory derivation must not allow '..' or + // separators to escape the cacheDir. + const validator = new ExternalLinkValidator(tempDir, { + linkAuthConfig: configWithProvider(), + linkAuthDeps: { env: ENV_WITH_TOKEN }, + fetchImpl: stubFetch(200), + osUser: '../escaped', + }); + await validator.validateLink(HOST); + + // Security property: cache must NOT be written outside tempDir, even + // though osUser contains `..` and `/`. + const escapedFile = safePath.join(tempDir, '..', 'escaped', CACHE_FILE); + expect(fsExists(escapedFile)).toBe(false); + + // Some user-scoped subdirectory of tempDir exists — the exact sanitized + // form is an implementation detail (slashes → `_`, `..` → `__`), so just + // assert that the safe target landed inside tempDir. + const { readdirSync } = await import('node:fs'); + const entries = readdirSync(tempDir); + const authDir = entries.find((name) => name.startsWith('auth-')); + expect(authDir).toBeDefined(); + expect(fsExists(safePath.join(tempDir, authDir as string, CACHE_FILE))).toBe( + true, + ); + }); +}); + +describe('ExternalLinkValidator — unsupported host falls through', () => { + it('a URL no provider claims uses the anonymous markdown-link-check path (no code set)', async () => { + // Wire a config with a provider that only claims github.com; validate an unrelated host. + // The unsupported branch falls through to the existing markdown-link-check path, which + // we don't mock here — the test just confirms we DON'T crash and the auth fetchImpl + // is not invoked. + const { fetchImpl, calls } = countingFetch(); + const validator = new ExternalLinkValidator(tempDir, { + linkAuthConfig: configWithProvider(), + linkAuthDeps: { env: ENV_WITH_TOKEN }, + fetchImpl, + timeout: 100, // Fast bail when markdown-link-check tries to reach the URL. + retries: 0, + }); + // Use a host no provider claims; the real markdown-link-check will try to fetch and + // either error or return some status — either way, the auth fetchImpl must not fire. + await validator + .validateLink('https://this-host-not-claimed.example.invalid/') + .catch(() => undefined); + expect(calls()).toBe(0); + }); +}); diff --git a/packages/resources/test/link-auth-classify.test.ts b/packages/resources/test/link-auth-classify.test.ts new file mode 100644 index 00000000..d6046c21 --- /dev/null +++ b/packages/resources/test/link-auth-classify.test.ts @@ -0,0 +1,123 @@ +import type { ProviderCheck } from '@vibe-agent-toolkit/utils'; +import { describe, expect, it } from 'vitest'; + +import { classifyAuthenticatedResponse } from '../src/link-auth-classify.js'; + +const DEAD: ProviderCheck = { + method: 'GET', + aliveStatus: [200], + notFoundMeaning: 'dead', +}; + +const AMBIGUOUS: ProviderCheck = { + method: 'GET', + aliveStatus: [200], + notFoundMeaning: 'ambiguous', +}; + +describe('classifyAuthenticatedResponse — alive', () => { + it('classifies a status listed in aliveStatus as alive (no code)', () => { + expect(classifyAuthenticatedResponse(200, DEAD)).toEqual({ outcome: 'alive', code: null }); + expect(classifyAuthenticatedResponse(200, AMBIGUOUS)).toEqual({ outcome: 'alive', code: null }); + }); + + it('honors a multi-value aliveStatus', () => { + const check: ProviderCheck = { method: 'GET', aliveStatus: [200, 206], notFoundMeaning: 'dead' }; + expect(classifyAuthenticatedResponse(206, check)).toEqual({ outcome: 'alive', code: null }); + }); + + it('an aliveStatus entry wins even when it would otherwise be 404 → dead', () => { + // A provider could (in principle) declare 404 as alive — confirm the + // aliveStatus check fires first, so the routing is purely config-driven. + const check: ProviderCheck = { method: 'GET', aliveStatus: [404], notFoundMeaning: 'dead' }; + expect(classifyAuthenticatedResponse(404, check)).toEqual({ outcome: 'alive', code: null }); + }); +}); + +describe('classifyAuthenticatedResponse — 401 / 403 are status-only, ignore notFoundMeaning', () => { + it('401 → unauthorized + LINK_AUTH_UNAUTHORIZED', () => { + expect(classifyAuthenticatedResponse(401, DEAD)).toEqual({ + outcome: 'unauthorized', + code: 'LINK_AUTH_UNAUTHORIZED', + }); + expect(classifyAuthenticatedResponse(401, AMBIGUOUS)).toEqual({ + outcome: 'unauthorized', + code: 'LINK_AUTH_UNAUTHORIZED', + }); + }); + + it('403 → forbidden + LINK_AUTH_FORBIDDEN', () => { + expect(classifyAuthenticatedResponse(403, DEAD)).toEqual({ + outcome: 'forbidden', + code: 'LINK_AUTH_FORBIDDEN', + }); + expect(classifyAuthenticatedResponse(403, AMBIGUOUS)).toEqual({ + outcome: 'forbidden', + code: 'LINK_AUTH_FORBIDDEN', + }); + }); +}); + +describe('classifyAuthenticatedResponse — 404 / 410 split by notFoundMeaning', () => { + it('404 + notFoundMeaning=dead → dead + LINK_AUTH_DEAD', () => { + expect(classifyAuthenticatedResponse(404, DEAD)).toEqual({ + outcome: 'dead', + code: 'LINK_AUTH_DEAD', + }); + }); + + it('410 + notFoundMeaning=dead → dead + LINK_AUTH_DEAD', () => { + expect(classifyAuthenticatedResponse(410, DEAD)).toEqual({ + outcome: 'dead', + code: 'LINK_AUTH_DEAD', + }); + }); + + it('404 + notFoundMeaning=ambiguous → dead_or_unauthorized + LINK_AUTH_DEAD_OR_UNAUTHORIZED', () => { + expect(classifyAuthenticatedResponse(404, AMBIGUOUS)).toEqual({ + outcome: 'dead_or_unauthorized', + code: 'LINK_AUTH_DEAD_OR_UNAUTHORIZED', + }); + }); + + it('410 + notFoundMeaning=ambiguous → dead_or_unauthorized + LINK_AUTH_DEAD_OR_UNAUTHORIZED', () => { + expect(classifyAuthenticatedResponse(410, AMBIGUOUS)).toEqual({ + outcome: 'dead_or_unauthorized', + code: 'LINK_AUTH_DEAD_OR_UNAUTHORIZED', + }); + }); +}); + +describe('classifyAuthenticatedResponse — unclassified status falls through', () => { + it('500 → null (caller falls through to the anonymous EXTERNAL_URL_DEAD path)', () => { + expect(classifyAuthenticatedResponse(500, DEAD)).toBeNull(); + expect(classifyAuthenticatedResponse(500, AMBIGUOUS)).toBeNull(); + }); + + it('301 / 302 → null (redirects are caller concern, not a classified outcome)', () => { + expect(classifyAuthenticatedResponse(301, DEAD)).toBeNull(); + expect(classifyAuthenticatedResponse(302, DEAD)).toBeNull(); + }); + + it('429 → null (rate-limit handling is caller concern per §5.2 Retry-After)', () => { + expect(classifyAuthenticatedResponse(429, DEAD)).toBeNull(); + }); +}); + +describe('classifyAuthenticatedResponse — returned code is a real CODE_REGISTRY entry', () => { + it('every non-null returned code matches a literal LINK_AUTH_* registry key', async () => { + // External-constant cross-check: assert the classifier's outputs match the + // exact registry keys, not via the classifier itself. + const { CODE_REGISTRY } = await import('@vibe-agent-toolkit/agent-schema'); + const codes = [ + classifyAuthenticatedResponse(401, DEAD).code, + classifyAuthenticatedResponse(403, DEAD).code, + classifyAuthenticatedResponse(404, DEAD).code, + classifyAuthenticatedResponse(404, AMBIGUOUS).code, + ]; + for (const code of codes) { + expect(code).not.toBeNull(); + expect(Object.hasOwn(CODE_REGISTRY, code as string)).toBe(true); + } + }); +}); diff --git a/packages/resources/test/link-auth-config-build.test.ts b/packages/resources/test/link-auth-config-build.test.ts new file mode 100644 index 00000000..86bc6433 --- /dev/null +++ b/packages/resources/test/link-auth-config-build.test.ts @@ -0,0 +1,132 @@ +import { resolveAuthenticatedUrl } from '@vibe-agent-toolkit/utils'; +import { describe, expect, it } from 'vitest'; + +import { buildLinkAuthEngineConfig } from '../src/link-auth-config-build.js'; +import type { LinkAuthProjectConfig } from '../src/schemas/link-auth.js'; + +const INLINE_HOST = 'example.com'; +const INLINE_PROVIDER = { + match: { host: INLINE_HOST }, + rewrite: [{ when: '.*', to: 'https://api.example.com' }], + auth: { headers: { Authorization: 'Bearer ${token}' } }, + token: [{ env: 'TOK' }], + check: { method: 'GET' as const, aliveStatus: [200], notFoundMeaning: 'dead' as const }, +}; + +describe('buildLinkAuthEngineConfig — empty + inline pass-through', () => { + it('empty providers array yields an engine config with empty providers', () => { + const engine = buildLinkAuthEngineConfig({ providers: [] }); + expect(engine.providers).toEqual([]); + }); + + it('inline providers pass through unchanged (no macro expansion)', () => { + const engine = buildLinkAuthEngineConfig({ providers: [INLINE_PROVIDER] }); + expect(engine.providers).toHaveLength(1); + expect(engine.providers[0]).toMatchObject({ + match: { host: INLINE_HOST }, + check: { notFoundMeaning: 'dead' }, + }); + }); + + it('order of providers is preserved (first-claiming-wins is determined by position)', () => { + const a = { ...INLINE_PROVIDER, match: { host: 'a.com' } }; + const b = { ...INLINE_PROVIDER, match: { host: 'b.com' } }; + const engine = buildLinkAuthEngineConfig({ providers: [a, b] }); + expect(engine.providers[0]?.match.host).toBe('a.com'); + expect(engine.providers[1]?.match.host).toBe('b.com'); + }); +}); + +describe('buildLinkAuthEngineConfig — macro expansion', () => { + it('{ use: "github" } expands to the github macro shape', () => { + const engine = buildLinkAuthEngineConfig({ providers: [{ use: 'github' }] }); + expect(engine.providers).toHaveLength(1); + const [p] = engine.providers; + expect(p?.match.host).toBe('github.com'); + expect(p?.check.notFoundMeaning).toBe('ambiguous'); + expect(p?.auth.headers['Authorization']).toBe('Bearer ${token}'); + }); + + it('{ use: "sharepoint" } expands to the sharepoint macro shape', () => { + const engine = buildLinkAuthEngineConfig({ providers: [{ use: 'sharepoint' }] }); + expect(engine.providers[0]?.match.host).toBe('*.sharepoint.com'); + expect(engine.providers[0]?.check.notFoundMeaning).toBe('dead'); + }); + + it('overrides deep-merge on top of macro defaults', () => { + const engine = buildLinkAuthEngineConfig({ + providers: [ + { + use: 'github', + match: { host: 'github.example.internal' }, + token: [{ env: 'INTERNAL_GH_TOKEN' }], + }, + ], + }); + const [p] = engine.providers; + expect(p?.match.host).toBe('github.example.internal'); + // Token list replaced wholesale (arrays don't element-merge). + expect(p?.token).toEqual([{ env: 'INTERNAL_GH_TOKEN' }]); + // Other fields preserved from the macro. + expect(p?.check.notFoundMeaning).toBe('ambiguous'); + }); + + it('throws UnknownMacroError for an unknown macro name', () => { + expect(() => buildLinkAuthEngineConfig({ providers: [{ use: 'not-a-real-macro' }] })).toThrow( + /not-a-real-macro/, + ); + }); + + it('post-expansion validation: a macro override that produces an invalid provider throws', () => { + // Override `check.notFoundMeaning` to an invalid enum value. The macro + // schema layer can't catch this (overrides pass through unchecked), so + // post-expansion validation must. + expect(() => + buildLinkAuthEngineConfig({ + providers: [{ use: 'github', check: { notFoundMeaning: 'totally-invalid' } }] as LinkAuthProjectConfig['providers'], + }), + ).toThrow(/providers\[0\]/); + }); +}); + +describe('buildLinkAuthEngineConfig — interop with the engine', () => { + it('produced config can drive resolveAuthenticatedUrl end-to-end', () => { + const engine = buildLinkAuthEngineConfig({ providers: [{ use: 'github' }] }); + const outcome = resolveAuthenticatedUrl( + 'https://github.com/owner/repo/blob/main/file.md', + engine, + { + env: { GITHUB_TOKEN: 'gh_abc' }, + // Bypass the macro's `gh auth token` command source so the test does + // not pick up a real `gh` login from the dev machine — return an + // empty stdout so the engine falls through to the env source. + runCommand: () => ({ success: true, stdout: '' }), + }, + ); + expect('fetchUrl' in outcome).toBe(true); + if (!('fetchUrl' in outcome)) return; + expect(outcome.fetchUrl).toContain('api.github.com'); + expect(outcome.headers['Authorization']).toBe('Bearer gh_abc'); + expect(outcome.check.notFoundMeaning).toBe('ambiguous'); + }); +}); + +describe('buildLinkAuthEngineConfig — prototype-pollution defense in macro entries', () => { + it('uses Object.hasOwn to discriminate {use} so prototype-injected `use` cannot trigger macro expansion', () => { + // Build an object whose prototype carries `use: github` but whose own + // property set has only the inline provider shape. The function must + // discriminate via Object.hasOwn (not `'use' in entry`), so this object + // is treated as inline, not as a macro reference. + const pollutedProto = { use: 'github' }; + const inline = Object.create(pollutedProto) as Record; + Object.assign(inline, INLINE_PROVIDER); + + const engine = buildLinkAuthEngineConfig({ + providers: [inline] as LinkAuthProjectConfig['providers'], + }); + // If `'use' in inline` were used, it would have read 'github' from the + // prototype and expanded the github macro — match.host would be 'github.com'. + // With Object.hasOwn, the prototype is ignored and the inline values win. + expect(engine.providers[0]?.match.host).toBe(INLINE_HOST); + }); +}); diff --git a/packages/resources/test/link-auth-fetch.test.ts b/packages/resources/test/link-auth-fetch.test.ts new file mode 100644 index 00000000..755c34f8 --- /dev/null +++ b/packages/resources/test/link-auth-fetch.test.ts @@ -0,0 +1,254 @@ +import { describe, expect, it, vi } from 'vitest'; + +import { fetchAuthenticated, parseRetryAfter } from '../src/link-auth-fetch.js'; + +import { sequenceFetch } from './auth-fetch-mocks.js'; + +const TEST_TOKEN = 'Bearer test-token-12345'; +const ORIGIN_URL = 'https://api.github.com/x'; +const ATTACKER_URL = 'https://attacker.example.com/leak'; + +const AUTH_HEADERS = { Authorization: TEST_TOKEN, Accept: 'application/json' }; + +describe('parseRetryAfter', () => { + it('null/empty → null', () => { + expect(parseRetryAfter(null)).toBeNull(); + expect(parseRetryAfter('')).toBeNull(); + expect(parseRetryAfter(' ')).toBeNull(); + }); + + it('delta-seconds → milliseconds', () => { + expect(parseRetryAfter('5')).toBe(5000); + expect(parseRetryAfter('0')).toBe(0); + expect(parseRetryAfter(' 30 ')).toBe(30_000); + }); + + it('negative or non-numeric junk → null', () => { + expect(parseRetryAfter('-5')).toBeNull(); + expect(parseRetryAfter('abc')).toBeNull(); + expect(parseRetryAfter('5.5.5')).toBeNull(); + }); + + it('HTTP-date in the future → ms from now (approximate)', () => { + const future = new Date(Date.now() + 10_000).toUTCString(); + const result = parseRetryAfter(future); + expect(result).not.toBeNull(); + // Allow small slop for clock drift between Date.now() calls. + expect(result).toBeGreaterThan(8_000); + expect(result).toBeLessThan(12_000); + }); + + it('HTTP-date in the past → 0 (do not wait, but retry is still warranted)', () => { + const past = new Date(Date.now() - 60_000).toUTCString(); + expect(parseRetryAfter(past)).toBe(0); + }); +}); + +describe('fetchAuthenticated — happy path (no redirect, no retry)', () => { + it('passes URL and headers through to fetchImpl, returns response', async () => { + const impl = sequenceFetch([ + { + status: 200, + assertUrl: (url) => expect(url).toBe('https://api.github.com/repos/o/r/contents/f'), + assertHeaders: (h) => expect(h['Authorization']).toBe(TEST_TOKEN), + }, + ]); + const response = await fetchAuthenticated( + 'https://api.github.com/repos/o/r/contents/f', + AUTH_HEADERS, + impl, + ); + expect(response.status).toBe(200); + }); +}); + +describe('fetchAuthenticated — cross-origin Authorization stripping (§8)', () => { + it('same-origin redirect preserves Authorization header', async () => { + const impl = sequenceFetch([ + { status: 302, headers: { location: 'https://api.github.com/redirected' } }, + { + status: 200, + assertHeaders: (h) => expect(h['Authorization']).toBe(TEST_TOKEN), + }, + ]); + const response = await fetchAuthenticated( + ORIGIN_URL, + AUTH_HEADERS, + impl, + ); + expect(response.status).toBe(200); + }); + + it('cross-origin redirect strips Authorization header (different host)', async () => { + const impl = sequenceFetch([ + { status: 302, headers: { location: ATTACKER_URL } }, + { + status: 200, + assertHeaders: (h) => expect(h['Authorization']).toBeUndefined(), + }, + ]); + const response = await fetchAuthenticated( + ORIGIN_URL, + AUTH_HEADERS, + impl, + ); + expect(response.status).toBe(200); + }); + + it('cross-origin redirect strips Authorization case-insensitively', async () => { + // A buggy/exotic caller might pass header key as 'authorization' instead. + // The strip must match Headers semantics (case-insensitive). + const impl = sequenceFetch([ + { status: 302, headers: { location: ATTACKER_URL } }, + { + status: 200, + assertHeaders: (h) => { + for (const key of Object.keys(h)) { + expect(key.toLowerCase()).not.toBe('authorization'); + } + }, + }, + ]); + await fetchAuthenticated( + 'https://api.github.com/o', + { authorization: 'Bearer t', Accept: 'application/json' }, + impl, + ); + }); + + it('redirect with relative Location resolves against current URL (still same-origin)', async () => { + const impl = sequenceFetch([ + { status: 302, headers: { location: '/relative/path' } }, + { + status: 200, + assertUrl: (url) => expect(url).toBe('https://api.github.com/relative/path'), + assertHeaders: (h) => expect(h['Authorization']).toBe(TEST_TOKEN), + }, + ]); + await fetchAuthenticated(ORIGIN_URL, AUTH_HEADERS, impl); + }); + + it('chain of redirects: first cross-origin strip propagates to subsequent hops', async () => { + // Once Authorization is stripped, it stays stripped — even if a subsequent + // hop is back to the original origin (a known token-laundering attack vector). + const impl = sequenceFetch([ + { status: 302, headers: { location: 'https://attacker.example.com/hop1' } }, + { + status: 302, + headers: { location: 'https://api.github.com/hop2' }, + assertHeaders: (h) => expect(h['Authorization']).toBeUndefined(), + }, + { + status: 200, + assertHeaders: (h) => expect(h['Authorization']).toBeUndefined(), + }, + ]); + await fetchAuthenticated('https://api.github.com/start', AUTH_HEADERS, impl); + }); + + it('exceeding maxRedirects returns the last 3xx response (does not throw)', async () => { + const impl = sequenceFetch([ + { status: 302, headers: { location: 'https://api.github.com/2' } }, + { status: 302, headers: { location: 'https://api.github.com/3' } }, + { status: 302, headers: { location: 'https://api.github.com/4' } }, + ]); + const response = await fetchAuthenticated( + 'https://api.github.com/1', + AUTH_HEADERS, + impl, + { maxRedirects: 2 }, + ); + expect(response.status).toBe(302); + }); + + it('redirect with no Location header returns the 3xx response', async () => { + const impl = sequenceFetch([{ status: 301 }]); + const response = await fetchAuthenticated(ORIGIN_URL, AUTH_HEADERS, impl); + expect(response.status).toBe(301); + }); +}); + +/** + * Helper: invoke fetchAuthenticated with the standard test args (ORIGIN_URL + + * AUTH_HEADERS) and a caller-supplied impl/sleep. Eliminates the repeated + * 4-arg call boilerplate across the retry tests. + */ +function callWithRetry( + impl: typeof fetch, + sleep: ReturnType, + overrides: { maxRetries?: number; maxRetryAfterMs?: number } = {}, +): Promise { + return fetchAuthenticated(ORIGIN_URL, AUTH_HEADERS, impl, { + maxRetries: 2, + sleep, + ...overrides, + }); +} + +describe('fetchAuthenticated — 429 + Retry-After (§5.2)', () => { + it('429 with Retry-After=2 → sleeps 2000ms, retries, returns 200', async () => { + const sleep = vi.fn(async () => undefined); + const impl = sequenceFetch([ + { status: 429, headers: { 'retry-after': '2' } }, + { status: 200 }, + ]); + const response = await callWithRetry(impl, sleep); + expect(response.status).toBe(200); + expect(sleep.mock.calls).toEqual([[2000]]); + }); + + it('429 without Retry-After → does not retry, returns the 429', async () => { + const sleep = vi.fn(async () => undefined); + const impl = sequenceFetch([{ status: 429 }]); + const response = await callWithRetry(impl, sleep, { maxRetries: 5 }); + expect(response.status).toBe(429); + expect(sleep).not.toHaveBeenCalled(); + }); + + it('Retry-After: 0 clamps up to the 250ms floor (good-neighbor defense)', async () => { + const sleep = vi.fn(async () => undefined); + const impl = sequenceFetch([ + { status: 429, headers: { 'retry-after': '0' } }, + { status: 200 }, + ]); + await callWithRetry(impl, sleep); + // Hostile host says "retry now"; we wait at least 250 ms anyway. + expect(sleep.mock.calls).toEqual([[250]]); + }); + + it('Retry-After exceeding maxRetryAfterMs cap is clamped (DoS defense)', async () => { + const sleep = vi.fn(async () => undefined); + const impl = sequenceFetch([ + { status: 429, headers: { 'retry-after': '3600' } }, // 1 hour + { status: 200 }, + ]); + await callWithRetry(impl, sleep, { maxRetryAfterMs: 60_000 }); + expect(sleep.mock.calls).toEqual([[60_000]]); + }); + + it('exhausting maxRetries on repeated 429 returns the final 429', async () => { + const sleep = vi.fn(async () => undefined); + const impl = sequenceFetch([ + { status: 429, headers: { 'retry-after': '1' } }, + { status: 429, headers: { 'retry-after': '1' } }, + { status: 429, headers: { 'retry-after': '1' } }, + ]); + const response = await callWithRetry(impl, sleep); + expect(response.status).toBe(429); + expect(sleep).toHaveBeenCalledTimes(2); + }); +}); + +describe('fetchAuthenticated — interaction', () => { + it('429 → Retry-After → redirect: each phase honored in order', async () => { + const sleep = vi.fn(async () => undefined); + const impl = sequenceFetch([ + { status: 429, headers: { 'retry-after': '1' } }, + { status: 302, headers: { location: 'https://api.github.com/redirected' } }, + { status: 200 }, + ]); + const response = await callWithRetry(impl, sleep); + expect(response.status).toBe(200); + expect(sleep.mock.calls).toEqual([[1000]]); + }); +}); diff --git a/packages/utils/src/link-auth/resolve-token.ts b/packages/utils/src/link-auth/resolve-token.ts index 05446ad9..54c6b795 100644 --- a/packages/utils/src/link-auth/resolve-token.ts +++ b/packages/utils/src/link-auth/resolve-token.ts @@ -11,7 +11,7 @@ * * Returns `undefined` if every source fails or yields an empty/whitespace * value — the caller's `resolveAuthenticatedUrl` translates that to the - * `unverified` outcome (LINK_AUTH_UNVERIFIED in slice 2). + * `unverified` outcome (surfaced as `LINK_AUTH_UNVERIFIED` by the validator). * * Per design issue #113 §4 (token vocabulary) and §6.1 (command execution, * `safeExecSync`-backed, `shell: false`). diff --git a/packages/utils/src/link-auth/resolve.ts b/packages/utils/src/link-auth/resolve.ts index e26851a7..045e4c57 100644 --- a/packages/utils/src/link-auth/resolve.ts +++ b/packages/utils/src/link-auth/resolve.ts @@ -53,7 +53,18 @@ export interface LinkAuthConfig { } export type ResolveOutcome = - | { readonly fetchUrl: string; readonly headers: Record } + | { + readonly fetchUrl: string; + readonly headers: Record; + /** + * The matched provider's `check` block, passed through so the post-fetch + * classifier (in `packages/resources`) can route status codes to outcomes + * without re-running `selectProvider`. Reading this from the engine — + * rather than asking the validator to re-derive it — keeps the + * provider-match decision in exactly one place. + */ + readonly check: ProviderCheck; + } | { readonly outcome: 'unsupported' } | { readonly outcome: 'unverified'; readonly reason: string }; @@ -91,5 +102,5 @@ export function resolveAuthenticatedUrl( headerContext['token'] = token; const headers = buildHeaders(provider.auth.headers, headerContext); - return { fetchUrl: rewrite.rewrittenUrl, headers }; + return { fetchUrl: rewrite.rewrittenUrl, headers, check: provider.check }; } From e688e64c68a2f4e94bc9c67cb48aea85db9a966d Mon Sep 17 00:00:00 2001 From: Ethan Dutton <46871249+ejdutton@users.noreply.github.com> Date: Wed, 17 Jun 2026 09:34:36 -0400 Subject: [PATCH 2/2] =?UTF-8?q?fix(resources):=20address=20#125=20review?= =?UTF-8?q?=20=E2=80=94=20passthrough=20schemas,=20memoize=20runCommand,?= =?UTF-8?q?=20fail-soft=20cache=20IO?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses jdutton's three review findings on PR #125: 1. Postel's Law: the adopter-facing linkAuth schemas (InlineProviderSchema, LinkAuthConfigSchema, and the five nested object schemas) were `.strict()` but parse external input. Switched all seven to `.passthrough()` to match the repo CLAUDE.md rule for adopter configs — forward-compatible / typo'd fields now degrade rather than crash `vat resources validate`. Adjusted the file header comment that misread Postel's Law, and updated four schema tests to assert the new passthrough semantics. The compile-time _KeysAgree drift check moved from `keyof z.infer<...>` to `keyof Schema.shape` so passthrough's index signature doesn't defeat it. Post-expansion validation still catches missing required fields and wrong types on declared fields (e.g. invalid notFoundMeaning enum value); typo-catching DX belongs in a separate lint pass. 2. Token memoization: ExternalLinkValidator now wraps the linkAuth deps' runCommand with a Map, so validating N URLs from the same host runs `gh auth token` (or any command-source token) at most once per validator instance. Treats *all* token sources as potentially expensive per the review. Engine's DEFAULT_RUN_COMMAND exported as `defaultRunCommand` so the wrapper calls through to a single source of truth (avoids a duplicate-implementation jscpd clone). Two new tests pin the memoization (single provider → 1 call across N URLs; distinct providers → separate calls). 3. Fail-soft cache IO: ExternalLinkCache.loadCache() previously rethrew anything other than ENOENT/SyntaxError; saveCache() had no try/catch. An EACCES/EROFS on the cache file aborted the whole validate run. Both paths now swallow IO errors — read returns empty cache, write no-ops while the in-memory cache stays authoritative for the rest of the run. Two POSIX-skipped tests using chmod simulate EACCES on read and write. CHANGELOG updated to reflect the final post-review behavior. Co-Authored-By: Claude Opus 4.7 (1M context) --- CHANGELOG.md | 3 +- packages/resources/src/external-link-cache.ts | 39 ++++++---- .../resources/src/external-link-validator.ts | 37 ++++++++- .../resources/src/link-auth-config-build.ts | 28 ++++--- packages/resources/src/schemas/link-auth.ts | 41 +++++----- .../test/external-link-cache.test.ts | 54 ++++++++++++- .../test/external-link-validator-auth.test.ts | 76 ++++++++++++++++++- .../resources/test/schemas/link-auth.test.ts | 22 ++++-- packages/utils/src/index.ts | 2 +- packages/utils/src/link-auth/resolve-token.ts | 9 ++- 10 files changed, 251 insertions(+), 60 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 70834b4a..83d5532f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Internal -- **linkAuth validator wiring + per-host outcome codes (issue #113, slice 2).** The slice-1 pure engine is now end-to-end: when an adopter sets `resources.linkAuth` in `vibe-agent-toolkit.config.yaml`, `vat resources validate` bypasses the anonymous `markdown-link-check` path for any URL whose host is claimed by a provider, issues an authenticated `fetch()` against the rewritten URL with the configured token, and classifies the response per design §7 into one of five new `CODE_REGISTRY` entries: `LINK_AUTH_DEAD` (404/410 from an honest-404 host — `error`-severity, the only such code in the slice; design §7 establishes that an authenticated 404 against e.g. SharePoint is high-confidence link rot, satisfying the rule-design corpus-evidence bar), `LINK_AUTH_DEAD_OR_UNAUTHORIZED` (404 from an ambiguous host like GitHub that masks `403`s — warn), `LINK_AUTH_FORBIDDEN` (`403` — warn), `LINK_AUTH_UNAUTHORIZED` (`401` — warn, promote to `error` on strict CI lanes), and `LINK_AUTH_UNVERIFIED` (no token resolved — warn, never cached per §6.3). New files: `packages/resources/src/link-auth-fetch.ts` (`fetchAuthenticated()` — bounded redirect loop with **cross-origin `Authorization` stripping (§8)** that is sticky across the rest of the chain to defeat token-laundering, **429/`Retry-After` honoring (§5.2)** parsing both delta-seconds and HTTP-date forms with a 60s DoS cap *and* a 250ms good-neighbor floor, all dependency-injectable via `fetchImpl`/`sleep`/`signal`); `packages/resources/src/link-auth-classify.ts` (pure `(status, providerCheck) → outcome+code` per §7's table); `packages/resources/src/link-auth-config-build.ts` (bridge from adopter config to engine — runs `expandMacro` on `{ use: , ...overrides }` entries and **post-expansion validates against `InlineProviderSchema`** so a typo'd override field that passthroughs through the macro schema surfaces as a config error instead of silent runtime drift, with a compile-time `_KeysAgree` assertion locking the schema's top-level field set to the engine's `Provider` interface). New cache architecture: a second `ExternalLinkCache` instance for auth-branch results, **keyed by the rewritten URL** (the original `blob/` URL 404s — caching it would poison results) and scoped to a per-OS-user subdirectory `cacheDir/auth-${sanitizedOsUser}/` so two users on a shared CI host never read each other's authenticated results (§6.3); the OS user resolves through `os.userInfo()` → `USER`/`USERNAME` env → `'default'` with a one-shot `console.warn` on the last fallback so the cross-user-leak risk is observable. Cache entries gain an explicit `version: 1` field; reads of any other version produce a miss, so slice 3's content-cache evolution can change the entry shape without misparsing pre-existing files. Doc-anchor coverage iterator test (`packages/agent-schema/test/docs/validation-codes.test.ts`) iterates `CODE_REGISTRY` and asserts each code has a matching `### \`CODE\`` heading in `docs/validation-codes.md` plus a convention-matching `entry.reference` — 126 assertions covering all 63 codes, future-proofs the per-code docs requirement. Five new doc sections under "Authenticated External Link Codes" in `docs/validation-codes.md`. Engine surface gains: `Provider.check` now flows through on the verified `ResolveOutcome` so the classifier can route per-provider `notFoundMeaning`; `ExternalLinkValidatorOptions` gains `linkAuthConfig`, `fetchImpl`, `linkAuthDeps`, `sleep`, and `osUser` (the first two are adopter-usable for corporate-proxy/custom-TLS injection, the rest test-only); `LinkValidationResult` gains a `code?: IssueCode` field that the `resource-registry.ts` consumer prefers over the existing status-code-to-`EXTERNAL_URL_*` mapping. **196 new tests across `link-auth-classify` (13), `link-auth-fetch` (19), `external-link-validator-auth` (25), `link-auth-config-build` (10), `validation-codes` (+126 doc-anchor iterator, +2 LINK_AUTH_* registry), and `external-link-cache` (+1 version-gate).** Security-load-bearing tests pin the cross-origin Authorization strip (case-insensitive, sticky across chains, with userinfo/relative-Location edge cases), the path-traversal sanitizer (table-driven over 9 pathological `osUser` inputs), the post-expansion validation rejecting typo'd macro overrides, the unverified-no-cache invariant, the cache-hit re-classification (cache hits re-run the classifier against the current provider so a `notFoundMeaning` flip between runs surfaces the new code, not the old one), and an Object.hasOwn-based prototype-pollution defense on the `{ use }` discriminator in `buildLinkAuthEngineConfig`. Slice 4 (cross-platform `.cmd`-shim system test, `VAT_LINKAUTH_ALLOW_COMMAND=0` opt-out, contributor docs) and slice 3 (content-fetch primitive + content cache) are downstream. +- **linkAuth validator wiring + per-host outcome codes (issue #113, slice 2).** The slice-1 pure engine is now end-to-end: when an adopter sets `resources.linkAuth` in `vibe-agent-toolkit.config.yaml`, `vat resources validate` bypasses the anonymous `markdown-link-check` path for any URL whose host is claimed by a provider, issues an authenticated `fetch()` against the rewritten URL with the configured token, and classifies the response per design §7 into one of five new `CODE_REGISTRY` entries: `LINK_AUTH_DEAD` (404/410 from an honest-404 host — `error`-severity, the only such code in the slice; design §7 establishes that an authenticated 404 against e.g. SharePoint is high-confidence link rot, satisfying the rule-design corpus-evidence bar), `LINK_AUTH_DEAD_OR_UNAUTHORIZED` (404 from an ambiguous host like GitHub that masks `403`s — warn), `LINK_AUTH_FORBIDDEN` (`403` — warn), `LINK_AUTH_UNAUTHORIZED` (`401` — warn, promote to `error` on strict CI lanes), and `LINK_AUTH_UNVERIFIED` (no token resolved — warn, never cached per §6.3). New files: `packages/resources/src/link-auth-fetch.ts` (`fetchAuthenticated()` — bounded redirect loop with **cross-origin `Authorization` stripping (§8)** that is sticky across the rest of the chain to defeat token-laundering, **429/`Retry-After` honoring (§5.2)** parsing both delta-seconds and HTTP-date forms with a 60s DoS cap *and* a 250ms good-neighbor floor, all dependency-injectable via `fetchImpl`/`sleep`/`signal`); `packages/resources/src/link-auth-classify.ts` (pure `(status, providerCheck) → outcome+code` per §7's table); `packages/resources/src/link-auth-config-build.ts` (bridge from adopter config to engine — runs `expandMacro` on `{ use: , ...overrides }` entries; the adopter schemas are passthrough per the repo's Postel's Law rule, so the post-expansion `InlineProviderSchema.safeParse` catches missing required fields and wrong types on declared fields but lets unknown extras through, matching how the rest of project-config treats adopter input; a compile-time `_KeysAgree` assertion locks the schema's top-level field set to the engine's `Provider` interface). New cache architecture: a second `ExternalLinkCache` instance for auth-branch results, **keyed by the rewritten URL** (the original `blob/` URL 404s — caching it would poison results) and scoped to a per-OS-user subdirectory `cacheDir/auth-${sanitizedOsUser}/` so two users on a shared CI host never read each other's authenticated results (§6.3); the OS user resolves through `os.userInfo()` → `USER`/`USERNAME` env → `'default'` with a one-shot `console.warn` on the last fallback so the cross-user-leak risk is observable. Cache entries gain an explicit `version: 1` field; reads of any other version produce a miss, so slice 3's content-cache evolution can change the entry shape without misparsing pre-existing files. Doc-anchor coverage iterator test (`packages/agent-schema/test/docs/validation-codes.test.ts`) iterates `CODE_REGISTRY` and asserts each code has a matching `### \`CODE\`` heading in `docs/validation-codes.md` plus a convention-matching `entry.reference` — 126 assertions covering all 63 codes, future-proofs the per-code docs requirement. Five new doc sections under "Authenticated External Link Codes" in `docs/validation-codes.md`. Engine surface gains: `Provider.check` now flows through on the verified `ResolveOutcome` so the classifier can route per-provider `notFoundMeaning`; `ExternalLinkValidatorOptions` gains `linkAuthConfig`, `fetchImpl`, `linkAuthDeps`, `sleep`, and `osUser` (the first two are adopter-usable for corporate-proxy/custom-TLS injection, the rest test-only); `LinkValidationResult` gains a `code?: IssueCode` field that the `resource-registry.ts` consumer prefers over the existing status-code-to-`EXTERNAL_URL_*` mapping. The validator memoizes `runCommand` results per unique argv for the duration of a `validate()` run, so validating N URLs from the same host runs `gh auth token` (and any other command-source token) at most once. **196 new tests across `link-auth-classify` (13), `link-auth-fetch` (19), `external-link-validator-auth` (25), `link-auth-config-build` (10), `validation-codes` (+126 doc-anchor iterator, +2 LINK_AUTH_* registry), and `external-link-cache` (+1 version-gate).** Security-load-bearing tests pin the cross-origin Authorization strip (case-insensitive, sticky across chains, with userinfo/relative-Location edge cases), the path-traversal sanitizer (table-driven over 9 pathological `osUser` inputs), the unverified-no-cache invariant, the cache-hit re-classification (cache hits re-run the classifier against the current provider so a `notFoundMeaning` flip between runs surfaces the new code, not the old one), the runCommand memoization (N URLs from the same provider → 1 command invocation), and an Object.hasOwn-based prototype-pollution defense on the `{ use }` discriminator in `buildLinkAuthEngineConfig`. Slice 4 (cross-platform `.cmd`-shim system test, `VAT_LINKAUTH_ALLOW_COMMAND=0` opt-out, contributor docs) and slice 3 (content-fetch primitive + content cache) are downstream. - **linkAuth pure engine foundation (issue #113, slice 1).** Adds a config-driven engine for authenticated external URL resolution, scoped to the pure-logic layer with no consumer wiring yet (the `ExternalLinkValidator` integration and the `LINK_AUTH_*` `CODE_REGISTRY` entries are slice 2; the content-fetch primitive is slice 3). New `link-auth/` module under `@vibe-agent-toolkit/utils` with eight files: `transforms.ts` (closed allowlist — `base64url`, `urlencode`, `lower` — with `Object.hasOwn`-based prototype-chain defense), `template.ts` (tiny `${name}` / `${transform(name)}` renderer, deliberately separate from the Handlebars renderer in `utils/template.ts`), `rewrite.ts` (ordered `when → vars → to` pipeline with fragment/query stripping per design §5.2), `build-headers.ts` (header rendering plus structural `Authorization` redaction), `select-provider.ts` (host-glob matching via picomatch with `excludeHost`), `expand-macro.ts` (YAML loader + deep-merge expander), `resolve-token.ts` (ordered env / `safeExecResult`-backed argv-command sources, first-non-empty wins, no shell), and `resolve.ts` (the public `resolveAuthenticatedUrl(url, config)` entry returning one of `{fetchUrl, headers}` / `{outcome: 'unsupported'}` / `{outcome: 'unverified', reason}`). Ships the `github` and `sharepoint` macros as a YAML data asset (`src/link-auth/macros.yaml`), with a new cross-platform `packages/dev-tools/src/copy-yaml-assets.ts` post-build step bundling `.yaml` into `dist/` — first YAML-asset shipping pattern in the utils package. Adds `yaml` as a utils dependency. Companion Zod schema in `@vibe-agent-toolkit/resources` (`src/schemas/link-auth.ts`) validates the `resources.linkAuth` config block (strict; accepts either `{ use: , ...overrides }` or full inline providers), wired as an optional field on `ResourcesConfigSchema`. 140 unit tests in utils + 29 schema tests in resources, all pure-logic with no network or filesystem dependencies; security-load-bearing tests pin the closed-allowlist guarantee, the `${__proto__}` bypass defense, the token-never-leaks-into-Authorization invariant, and `shell: false` literal-argv handling. - **Corpus seed expanded from 9 → 237 entries via a new committed importer at `packages/dev-tools/src/import-marketplace.ts` (`bun run import-marketplace [--allow-shrink]`).** The script fetches `.claude-plugin/marketplace.json` from `anthropics/claude-plugins-official` (205 of 209 raw entries kept) and `anthropics/knowledge-work-plugins` (30 of 60 — the knowledge-work catalog turns out to be ≈50% mirror entries of the official catalog) via `gh api`, maps each upstream entry to a `PluginEntry`, deduplicates by `source` URL (preserved VAT-owned entries always win; otherwise alphabetical-first-name wins within each duplicate cluster), and rewrites `corpus/seed.yaml`. Mapping rules: `bucket: official` uniformly (both catalogs are anthropics-curated marketplaces — `bucket` is the *reporting posture* per slice 1a, not code provenance); `confidence: first-party` for catalog-internal string sources and `github.com/anthropics/...` object sources, else `curated`; the `./partner-built/` knowledge-work convention overrides to `curated`; `maturity: production` for all entries. URL composition handles all five upstream source shapes (string, `git-subdir` ± `ref`, `url` ± `path`, `github`), throwing on unknown discriminators. The seven sample entries from slice 1a are regenerated from upstream manifests on every re-import. Re-import safety: the importer refuses to overwrite `corpus/seed.yaml` if either upstream catalog returned 0 plugins or the new entry count would drop more than 20% vs. the existing seed; `--allow-shrink` bypasses both gates for the rare case where shrinkage is real. The generated `seed.yaml` header dropped its earlier per-entry `validation:` claim (the importer throws on validation blocks today) and now states explicitly that entry `source` URLs pin a fragment ref (typically the default branch), not a per-entry commit SHA — the catalog SHAs in the header are this run's audit provenance. Issue #99 slice 1b — follows the schema change from PR #111 (slice 1a). - **Empirical compatibility harness (`packages/dev-tools/src/compat-empirical/`).** Per-#100 research scaffold for measuring skill compatibility across `claude-code`, `claude-cowork`, and `claude-chat`: a CLI (`predict`/`run`/`judge`/`report`/`all`) that joins VAT's static predictions with deterministic runtime observations and an LLM-judge semantic read into a reality-vs-prediction matrix — an evidence artifact for proposing detector improvements that each cite specific (skill, runtime) cells. Probe coverage: multi-prompt + repeat-N with adaptive N=3→N=5 extension, mandatory positive+negative prompt pairing per corpus entry, and negative-prompt agreement inversion so false-positive triggers surface as `vat-optimistic`. Evidence quality: the deterministic class is widened from 6 to 9 values (splitting `error` into `install-failed`/`runtime-error`, `not-invoked` into `not-invoked-engaged`/`not-invoked-empty`, adding `refused`), with a v2 judge prompt that adds a `refused` verdict. Report fidelity: coverage stats, per-bucket headline (own/official/community × ran/agree/optimistic/pessimistic/gray-zone), gray-zone (mixed-signal) and high-variance subsections, and per-attempt variance rendered inline (`runtime-error (2/3) / failed (3/3)`). Judge replay persists `judge-calls/---.json` artifacts that a new `re-judge` subcommand re-executes against an optionally different model or freshly-edited system prompt — without re-spending operator hours on the runtime side. Also landed: `git fetch --tags --force` before named-ref fetch (annotated tag refresh) and `setup()` teardown-first idempotency for the manual driver. No detector code or `RUNTIME_PROFILES` changes; lives entirely in the private `@vibe-agent-toolkit/dev-tools` package with no adopter-facing surface. Design: [the v2 harness design](./docs/research/2026-05-23-compat-empirical-harness-v2-design.md). Corpus authoring, the first real run, and the docs deliverable are the downstream work. @@ -29,6 +29,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - **`ExternalLinkValidator.clearCache()` and `getCacheStats()` now operate on both caches (issue #113).** Slice 2 introduced a second cache instance for authenticated-link results (per-OS-user scoping); the existing `clearCache()` / `getCacheStats()` methods continued to touch only the anonymous cache, so an adopter rotating a token would see stale `401`/`403` entries until the auth cache TTL expired. Both methods now clear/sum across both caches. +- **`ExternalLinkCache` IO errors degrade to a cache miss instead of aborting validation (issue #113).** `loadCache()` previously threw on anything other than `ENOENT` / `SyntaxError` (e.g. `EACCES` on a permissions-restricted cache file, `EROFS` on a read-only filesystem); `saveCache()` had no try/catch (write errors propagated). A failed read / write on the status-cache file would abort the whole `vat resources validate` run. Both paths are now fail-soft: a read failure returns an empty in-memory cache, a write failure no-ops while the in-memory cache stays authoritative for the remainder of the run. Cost of a bad cache entry: one extra fetch. Cost of a bad cache entry under the previous behavior: the whole run. - **Transformers.js integration tests now skip on Windows CI instead of flaking.** `transformers-embedding-provider.integration.test.ts` and the Transformers.js block of `comparison.integration.test.ts` skip on Windows (in addition to skipping when the optional `@xenova/transformers` dependency is absent), matching the existing `onnx-embedding-provider` test. These tests download a model over the network and load the `onnxruntime-node` native backend — both flaky in Windows CI. Such a failure was previously mislabeled `@xenova/transformers is not installed` by an over-broad `catch` in the provider's `loadPipeline` (the package was installed; the model download/inference is what failed), which is also why an availability-only guard did not prevent it. - **Config-first skill discovery now honors `..` in `skills.include` patterns.** `vat build`, `vat verify`, and `vat skills validate` all funnel through `discoverSkillsFromConfig`, which previously passed every include pattern to a single downward-only crawl rooted at `projectRoot` — so an include like `"../../docs/skills/*/SKILL.md"` (common in monorepos where SKILL.md sources live alongside, not inside, the package) silently matched zero skills. `vat audit` accepted the same config only because it has a separate filesystem-first walker. Each include pattern is now split into a literal base + glob remainder via `picomatch.scan`, patterns are grouped by their resolved absolute base, and the crawler runs once per base — making config-first discovery agree with audit. User-supplied excludes stay anchored to `projectRoot` so patterns like `docs/private/**` keep their original meaning, and a pattern resolving to a nonexistent base now silently produces zero matches. diff --git a/packages/resources/src/external-link-cache.ts b/packages/resources/src/external-link-cache.ts index e22918a6..18ddcec8 100644 --- a/packages/resources/src/external-link-cache.ts +++ b/packages/resources/src/external-link-cache.ts @@ -75,7 +75,13 @@ export class ExternalLinkCache { } /** - * Load cache from disk + * Load cache from disk. Fail-soft: any IO error (ENOENT, EACCES, EROFS, + * corrupted JSON, …) degrades to an empty cache instead of throwing. + * + * Why no error propagation: `vat resources validate` should keep running + * when the cache file isn't reachable (read-only filesystem, permission + * mismatch, full disk) — a missing cache costs an extra network round-trip, + * an exception costs the whole run. Per #125 review. */ private async loadCache(): Promise { if (this.cache !== null) { @@ -89,28 +95,35 @@ export class ExternalLinkCache { const data = await fs.readFile(this.cacheFile, 'utf-8'); this.cache = JSON.parse(data) as CacheData; return this.cache; - } catch (error) { - // Handle missing file or corrupted JSON - start with empty cache - if ((error as NodeJS.ErrnoException).code === 'ENOENT' || error instanceof SyntaxError) { - this.cache = {}; - return this.cache; - } - throw error; + } catch { + // All IO and parse errors degrade to an empty cache. Subsequent + // reads see the same empty cache (this.cache is set), so we don't + // re-spam mkdir/read on every lookup within the same run. + this.cache = {}; + return this.cache; } } /** - * Save cache to disk + * Save cache to disk. Fail-soft: any IO error becomes a no-op instead of + * throwing. Same rationale as `loadCache` — a non-persisted entry costs an + * extra fetch on the next run, an exception costs the whole current run. */ private async saveCache(): Promise { if (this.cache === null) { return; } - // eslint-disable-next-line security/detect-non-literal-fs-filename -- cacheDir is constructor parameter, controlled by caller - await fs.mkdir(this.cacheDir, { recursive: true }); - // eslint-disable-next-line security/detect-non-literal-fs-filename -- cacheFile is derived from cacheDir - await fs.writeFile(this.cacheFile, JSON.stringify(this.cache, null, 2), 'utf-8'); + try { + // eslint-disable-next-line security/detect-non-literal-fs-filename -- cacheDir is constructor parameter, controlled by caller + await fs.mkdir(this.cacheDir, { recursive: true }); + // eslint-disable-next-line security/detect-non-literal-fs-filename -- cacheFile is derived from cacheDir + await fs.writeFile(this.cacheFile, JSON.stringify(this.cache, null, 2), 'utf-8'); + } catch { + // No-op on IO failure. The in-memory cache (`this.cache`) is still + // authoritative for the current run; only the disk persistence is + // lost. + } } /** diff --git a/packages/resources/src/external-link-validator.ts b/packages/resources/src/external-link-validator.ts index b5ea4cde..e75aab2a 100644 --- a/packages/resources/src/external-link-validator.ts +++ b/packages/resources/src/external-link-validator.ts @@ -2,6 +2,7 @@ import { userInfo } from 'node:os'; import type { IssueCode } from '@vibe-agent-toolkit/agent-schema'; import { + defaultRunCommand, resolveAuthenticatedUrl, safePath, type LinkAuthConfig, @@ -49,6 +50,34 @@ function warnDefaultUserFallbackOnce(): void { ); } +/** + * Wrap a `LinkAuthDeps` so its `runCommand` (used by the engine's + * `resolveToken`) caches results per unique argv. Without this, validating N + * URLs from the same host re-runs the token command N times — `gh auth token` + * spawns a subprocess each call, and we treat *all* token sources as + * potentially expensive per the #125 review. + * + * The memo cache is keyed by JSON-stringified argv, so semantically-identical + * invocations share. Cache is per-instance — code that creates a fresh + * validator per run gets a fresh cache. + */ +function wrapLinkAuthDepsWithMemo(deps: LinkAuthDeps): LinkAuthDeps { + type RunCommandFn = NonNullable['runCommand']>; + type RunCommandResult = ReturnType; + const memo = new Map(); + const baseRunCommand: RunCommandFn = deps?.runCommand ?? defaultRunCommand; + const runCommand: RunCommandFn = (argv) => { + const key = JSON.stringify(argv); + const cached = memo.get(key); + if (cached !== undefined) return cached; + const fresh = baseRunCommand(argv); + memo.set(key, fresh); + return fresh; + }; + return { ...(deps ?? {}), runCommand }; +} + + /** * Make an OS username safe for use as a directory component. Replaces path * separators (`/`, `\`), the parent-directory shorthand (`..`), and other @@ -266,7 +295,13 @@ export class ExternalLinkValidator { this.linkAuthConfig = options.linkAuthConfig; this.fetchImpl = options.fetchImpl ?? globalThis.fetch; - this.linkAuthDeps = options.linkAuthDeps; + // Wrap the configured runCommand (or the engine's default) with a memo + // keyed by stringified argv so each unique token-resolution command runs + // at most once per validator instance. Validating N links to the same + // host previously re-resolved the token N times — including subprocess + // spawns for `command` sources. Per #125 review: treat all token + // resolvers as expensive; the memo's lifetime equals one validate() run. + this.linkAuthDeps = wrapLinkAuthDepsWithMemo(options.linkAuthDeps); this.sleep = options.sleep; this.cache = new ExternalLinkCache(cacheDir, this.options.cacheTtlHours); diff --git a/packages/resources/src/link-auth-config-build.ts b/packages/resources/src/link-auth-config-build.ts index a9444d68..c4a380a4 100644 --- a/packages/resources/src/link-auth-config-build.ts +++ b/packages/resources/src/link-auth-config-build.ts @@ -8,10 +8,13 @@ * - A full inline `{ match, rewrite, auth, token, check }` — used as-is * * This function walks each provider entry, runs macro expansion when needed, - * and re-validates each fully-expanded provider against the inline schema - * (the macro schema's `.passthrough()` accepts unknown override keys, which - * silently no-op at expansion time — post-expansion validation is where - * typo'd override fields surface as errors, per #113 §5). + * and re-validates each fully-expanded provider against `InlineProviderSchema`. + * Since the schemas are passthrough (per the repo Postel's Law rule for + * adopter input), unknown extra keys survive expansion silently — the engine + * ignores them at runtime, and a separate lint/warn pass is the right home + * for typo-catching DX. Post-expansion validation still catches the cases + * passthrough doesn't relax: missing required fields, and wrong types on + * declared fields (e.g. `notFoundMeaning: 'totally-invalid'`). * * Per design issue #113 §5 (macros are config, not a privileged code path) * and §4 (engine vocabulary). @@ -22,7 +25,6 @@ import { type LinkAuthConfig, type Provider, } from '@vibe-agent-toolkit/utils'; -import type { z } from 'zod'; import { InlineProviderSchema, type LinkAuthProjectConfig } from './schemas/link-auth.js'; @@ -37,7 +39,11 @@ import { InlineProviderSchema, type LinkAuthProjectConfig } from './schemas/link * The schema and engine type are kept in sync by review (per slice 1 design * decision); this guard makes that review easier. */ -type _SchemaKeys = keyof z.infer; +// Use `.shape` (Zod's declared-key record) rather than `keyof z.infer<...>` — +// passthrough() injects `string | number` into the inferred key set, which +// defeats the drift comparison. `.shape` is unaffected: it's the set of +// declared fields, exactly what we want to compare against the engine type. +type _SchemaKeys = keyof typeof InlineProviderSchema.shape; type _EngineKeys = keyof Provider; type _KeysAgree = [_SchemaKeys] extends [_EngineKeys] ? [_EngineKeys] extends [_SchemaKeys] @@ -73,11 +79,11 @@ function expandProviderEntry(entry: unknown, index: number): Provider { const { use, overrides } = splitMacroEntry(entry as Record); const expanded = expandMacro(use, overrides); - // Re-validate the fully-expanded shape. A macro override that uses a typo'd - // field name (e.g. `notFoundMeaningg`) is accepted by MacroProviderSchema's - // .passthrough(), survives expansion as a stray field on the merged object, - // and is caught here by InlineProviderSchema's .strict() — surfaced as a - // config error rather than as silently-wrong runtime behavior. + // Re-validate the fully-expanded shape. Unknown extra keys pass through + // (the engine ignores them); what we still catch are missing required + // fields and wrong types on declared fields — e.g. a macro override that + // sets `check.notFoundMeaning: 'bogus'` is rejected here even though the + // upstream macro schema's passthrough accepted it. const parsed = InlineProviderSchema.safeParse(expanded); if (!parsed.success) { throw new Error( diff --git a/packages/resources/src/schemas/link-auth.ts b/packages/resources/src/schemas/link-auth.ts index c2a757b1..18fcf6ca 100644 --- a/packages/resources/src/schemas/link-auth.ts +++ b/packages/resources/src/schemas/link-auth.ts @@ -3,25 +3,26 @@ * * Validates the shape of an adopter's linkAuth configuration: providers * (each either a `use: ` reference or a full inline provider) plus - * an optional cache config. Strict — typos in provider field names are - * errors, per Postel's law ("writing our output → conservative"). + * an optional cache config. **Passthrough** — per the repo Postel's Law + * rule, schemas that parse external data (adopter `vibe-agent-toolkit.config.yaml`) + * are liberal: unknown fields pass through rather than abort the parse, so + * an adopter with a forward-compatible field or a typo gets `vat resources + * validate` degrading gracefully instead of crashing. Typo-catching DX is + * a separate lint/warn pass, not a hard parse failure. * * Provider entries accept EITHER: * - `{ use: , ...overrides }` — reference a shipped macro by name; - * override fields are deep-merged on top at expansion time. Overrides - * are NOT strictly typed at this layer because they must match the - * macro's shape, which we cannot see until expansion. A typo in an - * override silently no-ops; the full provider shape is validated - * after expansion (`buildLinkAuthEngineConfig` validates the - * fully-expanded provider against `InlineProviderSchema`). - * - A full inline `{ match, rewrite, auth, token, check }` — every field - * required, every nested object strict. + * override fields are deep-merged on top at expansion time. + * - A full inline `{ match, rewrite, auth, token, check }` — every required + * field present, with declared-field types still enforced (passthrough + * only relaxes unknown-key handling, not type checking on known fields). * * Mirrors the runtime types in `@vibe-agent-toolkit/utils`'s `link-auth/` * module. Keep these aligned: a config that parses here must satisfy the * `Provider` / `LinkAuthConfig` interfaces over there. * - * Per design issue #113 §4 (vocabulary) and §5 (macros). + * Per design issue #113 §4 (vocabulary) and §5 (macros), and the repo + * CLAUDE.md Postel's Law rule for adopter-facing configs. */ import { z } from 'zod'; @@ -38,7 +39,7 @@ export const ProviderMatchSchema = z .optional() .describe('Globs that, if any match the hostname, exclude this provider'), }) - .strict() + .passthrough() .describe('Provider selection — match.host + optional excludeHost globs'); export const RewriteRuleSchema = z @@ -50,7 +51,7 @@ export const RewriteRuleSchema = z .describe('Computed variables, each a template that references captures from `when`'), to: z.string().min(1).describe('URL template — interpolates ${capture} and ${var}'), }) - .strict() + .passthrough() .describe('A single rewrite rule (one entry in a provider\'s ordered rewrite list)'); export const ProviderAuthSchema = z @@ -59,17 +60,17 @@ export const ProviderAuthSchema = z .record(z.string(), z.string()) .describe('Header name → value template. Values interpolate ${token} and any capture/var.'), }) - .strict() + .passthrough() .describe('Auth headers attached to the authenticated fetch'); export const TokenSourceSchema = z .union([ - z.object({ env: z.string().min(1) }).strict(), + z.object({ env: z.string().min(1) }).passthrough(), z .object({ command: z.union([z.string().min(1), z.array(z.string().min(1)).min(1)]), }) - .strict(), + .passthrough(), ]) .describe('A single token source — env var, or argv command (preferred), or convenience string'); @@ -86,7 +87,7 @@ export const ProviderCheckSchema = z 'Per-host: does 404 mean "dead" (honest 404 host like Graph) or "ambiguous" (masking host like GitHub)?', ), }) - .strict() + .passthrough() .describe('Status-to-outcome classifier (consumed by the resources layer, not the pure engine)'); // --------------------------------------------------------------------------- @@ -101,7 +102,7 @@ export const InlineProviderSchema = z token: z.array(TokenSourceSchema).describe('Ordered token sources — first non-empty wins'), check: ProviderCheckSchema, }) - .strict(); + .passthrough(); const MacroProviderSchema = z .object({ @@ -129,7 +130,7 @@ export const LinkAuthCacheSchema = z .optional() .describe('Content cache TTL in minutes (default 30 — used by slice 3 content-fetch)'), }) - .strict(); + .passthrough(); export const LinkAuthConfigSchema = z .object({ @@ -138,7 +139,7 @@ export const LinkAuthConfigSchema = z .array(ProviderEntrySchema) .describe('Ordered list of providers — first claiming-by-host wins'), }) - .strict() + .passthrough() .describe('`resources.linkAuth` — authenticated external link resolution config'); /** diff --git a/packages/resources/test/external-link-cache.test.ts b/packages/resources/test/external-link-cache.test.ts index d66f6629..72b69de1 100644 --- a/packages/resources/test/external-link-cache.test.ts +++ b/packages/resources/test/external-link-cache.test.ts @@ -8,6 +8,7 @@ import { ExternalLinkCache } from '../src/external-link-cache.js'; const EXAMPLE_URL = 'https://example.com'; const GITHUB_URL = 'https://github.com'; const BROKEN_URL = 'https://broken.com'; +const CACHE_FILE = 'external-links.json'; describe('ExternalLinkCache', () => { let tempDir: string; @@ -117,7 +118,7 @@ describe('ExternalLinkCache', () => { // Hand-craft a legacy cache file (pre-version) and confirm get() rejects it. // Forward-compat per #113 — keeps slice 3+ free to evolve CacheEntry without // risking misparse against pre-existing files. - const cacheFile = safePath.join(tempDir, 'external-links.json'); + const cacheFile = safePath.join(tempDir, CACHE_FILE); const url = 'https://legacy.example.com'; const crypto = await import('node:crypto'); const key = crypto.createHash('sha256').update(url).digest('hex'); @@ -131,7 +132,7 @@ describe('ExternalLinkCache', () => { it('should handle corrupted cache file gracefully', async () => { // Write invalid JSON to cache file - const cacheFile = safePath.join(tempDir, 'external-links.json'); + const cacheFile = safePath.join(tempDir, CACHE_FILE); // eslint-disable-next-line security/detect-non-literal-fs-filename -- tempDir from mkdtemp, safe await fs.writeFile(cacheFile, 'invalid json {{{'); @@ -145,6 +146,55 @@ describe('ExternalLinkCache', () => { expect(newResult).not.toBeNull(); }); + // chmod modes used to simulate IO failures. Named for documentation; the + // `sonarjs/file-permissions` warning fires on the chmod call sites (where + // the literal is "applied"), so the per-call eslint-disable comments at + // each `await fs.chmod(...)` are what actually silence it. + const MODE_NO_PERMS = 0o000; + const MODE_RO_OWNER = 0o500; + const MODE_RW_OWNER = 0o700; + const MODE_RW_FILE = 0o644; + + it.skipIf(process.platform === 'win32')( + 'treats EACCES on cache file as a cache miss (fail-soft IO; POSIX-only)', + async () => { + // Per #125 review: cache IO must degrade, not abort the whole run. + // Seed an entry, then revoke read permission on the cache file — + // the next get() call should return null rather than throw EACCES. + await cache.set(EXAMPLE_URL, 200, 'OK'); + const cacheFile = safePath.join(tempDir, CACHE_FILE); + // eslint-disable-next-line security/detect-non-literal-fs-filename -- test-only: revokes perms on self-created tempDir to simulate EACCES + await fs.chmod(cacheFile, MODE_NO_PERMS); + try { + // Fresh instance to force re-read from disk. + const newCache = new ExternalLinkCache(tempDir, 24); + const result = await newCache.get(EXAMPLE_URL); + expect(result).toBeNull(); + } finally { + // Restore perms so afterEach cleanup succeeds. + // eslint-disable-next-line security/detect-non-literal-fs-filename, sonarjs/file-permissions -- test-only: restore RW (0o644) on file inside self-created tempDir so cleanup runs + await fs.chmod(cacheFile, MODE_RW_FILE); + } + }, + ); + + it.skipIf(process.platform === 'win32')( + 'treats EACCES on cache directory as a no-op set (fail-soft IO; POSIX-only)', + async () => { + // Read-only parent dir → mkdir/writeFile fail with EACCES. set() + // must complete without throwing; the in-memory cache still holds + // the entry for the rest of this run, but the disk write is lost. + // eslint-disable-next-line security/detect-non-literal-fs-filename -- test-only: read-only mode on tempDir to simulate EACCES + await fs.chmod(tempDir, MODE_RO_OWNER); + try { + await expect(cache.set(EXAMPLE_URL, 200, 'OK')).resolves.toBeUndefined(); + } finally { + // eslint-disable-next-line security/detect-non-literal-fs-filename -- test-only: restore RW on tempDir for cleanup + await fs.chmod(tempDir, MODE_RW_OWNER); + } + }, + ); + it('should clear all cache entries', async () => { // Add some entries await cache.set(EXAMPLE_URL, 200, 'OK'); diff --git a/packages/resources/test/external-link-validator-auth.test.ts b/packages/resources/test/external-link-validator-auth.test.ts index 72e2f7c7..bd3cb655 100644 --- a/packages/resources/test/external-link-validator-auth.test.ts +++ b/packages/resources/test/external-link-validator-auth.test.ts @@ -13,6 +13,7 @@ const TEST_TOKEN = 'gh_test_token_abc'; const HOST = 'https://github.com/owner/repo/blob/main/file.md'; const REWRITTEN = 'https://api.github.com/repos/owner/repo/contents/file.md?ref=main'; const CACHE_FILE = 'external-links.json'; +const BEARER_TOKEN_TEMPLATE = 'Bearer ${token}'; /** * Path-derived `existsSync` check — wraps the lint disable in one place. The @@ -32,7 +33,7 @@ function buildProvider(notFoundMeaning: 'ambiguous' | 'dead' = 'ambiguous'): Pro to: 'https://api.github.com/repos/${owner}/${repo}/contents/${path}?ref=${ref}', }, ], - auth: { headers: { Authorization: 'Bearer ${token}', Accept: 'application/vnd.github+json' } }, + auth: { headers: { Authorization: BEARER_TOKEN_TEMPLATE, Accept: 'application/vnd.github+json' } }, token: [{ env: 'TEST_GH_TOKEN' }], check: { method: 'GET', aliveStatus: [200], notFoundMeaning }, }; @@ -366,6 +367,79 @@ describe('ExternalLinkValidator — auth cache scoping (#113 §6.3)', () => { }); }); +describe('ExternalLinkValidator — runCommand memoization (#125 review)', () => { + it('reuses a token-resolution command across N URLs from the same provider', async () => { + // A provider configured with a command-source token. Validating multiple + // URLs from the same host must run the command at most once per validator + // instance — `gh auth token` spawns a subprocess; doing it per-URL is the + // exact pessimization the review caught. + let runCommandCalls = 0; + const provider: Provider = { + match: { host: 'github.com' }, + rewrite: [ + { + when: String.raw`^https://github\.com/(?[^/]+)/(?[^/]+)/blob/(?[^/]+)/(?.+)$`, + to: 'https://api.github.com/repos/${owner}/${repo}/contents/${path}?ref=${ref}', + }, + ], + auth: { headers: { Authorization: BEARER_TOKEN_TEMPLATE } }, + token: [{ command: ['gh', 'auth', 'token'] }], + check: { method: 'GET', aliveStatus: [200], notFoundMeaning: 'ambiguous' }, + }; + + const validator = new ExternalLinkValidator(tempDir, { + linkAuthConfig: { providers: [provider] }, + linkAuthDeps: { + env: {}, + runCommand: (argv) => { + runCommandCalls++; + return { success: true, stdout: `tok-${argv.join('-')}` }; + }, + }, + fetchImpl: stubFetch(200), + }); + + await validator.validateLink('https://github.com/owner/repo/blob/main/a.md'); + await validator.validateLink('https://github.com/owner/repo/blob/main/b.md'); + await validator.validateLink('https://github.com/owner/repo/blob/main/c.md'); + + expect(runCommandCalls).toBe(1); + }); + + it('runs separate commands for distinct argv (different providers do not share)', async () => { + const calls: string[][] = []; + const providerA: Provider = { + match: { host: 'a.example.com' }, + rewrite: [{ when: '^.+$', to: 'https://api.a.example.com' }], + auth: { headers: { Authorization: BEARER_TOKEN_TEMPLATE } }, + token: [{ command: ['cmd-a'] }], + check: { method: 'GET', aliveStatus: [200], notFoundMeaning: 'dead' }, + }; + const providerB: Provider = { + ...providerA, + match: { host: 'b.example.com' }, + rewrite: [{ when: '^.+$', to: 'https://api.b.example.com' }], + token: [{ command: ['cmd-b'] }], + }; + const validator = new ExternalLinkValidator(tempDir, { + linkAuthConfig: { providers: [providerA, providerB] }, + linkAuthDeps: { + env: {}, + runCommand: (argv) => { + calls.push([...argv]); + return { success: true, stdout: 'tok' }; + }, + }, + fetchImpl: stubFetch(200), + }); + + await validator.validateLink('https://a.example.com/path'); + await validator.validateLink('https://b.example.com/path'); + + expect(calls).toEqual([['cmd-a'], ['cmd-b']]); + }); +}); + describe('ExternalLinkValidator — unsupported host falls through', () => { it('a URL no provider claims uses the anonymous markdown-link-check path (no code set)', async () => { // Wire a config with a provider that only claims github.com; validate an unrelated host. diff --git a/packages/resources/test/schemas/link-auth.test.ts b/packages/resources/test/schemas/link-auth.test.ts index b7107b15..c412b3e4 100644 --- a/packages/resources/test/schemas/link-auth.test.ts +++ b/packages/resources/test/schemas/link-auth.test.ts @@ -50,8 +50,11 @@ describe('TokenSourceSchema', () => { expect(TokenSourceSchema.safeParse({ command: [] }).success).toBe(false); }); - it('rejects an object with both env and command (strict union)', () => { - expect(TokenSourceSchema.safeParse({ env: 'X', command: 'y' }).success).toBe(false); + it('passthrough: an object with both env and command parses against the first matching branch (engine consumes one source per entry)', () => { + // With passthrough on both union branches, this object matches the env + // branch and the `command` field passes through silently — the engine's + // per-source dispatcher only consults the discriminator it knows. + expect(TokenSourceSchema.safeParse({ env: 'X', command: 'y' }).success).toBe(true); }); }); @@ -72,9 +75,11 @@ describe('RewriteRuleSchema', () => { ).toBe(true); }); - it('rejects an unknown sibling key (typo defense)', () => { + it('passthrough: an unknown sibling key parses (adopter input is read liberally)', () => { + // Per repo Postel's Law: reading external data → passthrough. Typo-catching + // DX belongs in a separate lint/warn pass, not a hard parse failure. const result = RewriteRuleSchema.safeParse({ when: '^.+$', to: 'x', tos: 'typo' }); - expect(result.success).toBe(false); + expect(result.success).toBe(true); }); it('rejects missing required fields', () => { @@ -94,9 +99,10 @@ describe('ProviderEntrySchema — inline provider', () => { expect(ProviderEntrySchema.safeParse(incomplete).success).toBe(false); }); - it('rejects an unknown sibling key in an inline provider (typo defense)', () => { + it('passthrough: an unknown sibling key on an inline provider parses (adopter input is read liberally)', () => { + // Same Postel rationale as RewriteRuleSchema's passthrough test. const withTypo = { ...GITHUB_INLINE_PROVIDER, tokens: GITHUB_INLINE_PROVIDER.token }; - expect(ProviderEntrySchema.safeParse(withTypo).success).toBe(false); + expect(ProviderEntrySchema.safeParse(withTypo).success).toBe(true); }); it('rejects an empty rewrite array (at least one rule required)', () => { @@ -160,9 +166,9 @@ describe('LinkAuthConfigSchema — top-level', () => { expect(LinkAuthConfigSchema.safeParse(config).success).toBe(true); }); - it('rejects unknown top-level keys', () => { + it('passthrough: unknown top-level keys parse (adopter input is read liberally)', () => { const config = { providers: [], cachees: { ttlMinutes: 30 } }; - expect(LinkAuthConfigSchema.safeParse(config).success).toBe(false); + expect(LinkAuthConfigSchema.safeParse(config).success).toBe(true); }); it('rejects a missing `providers` field', () => { diff --git a/packages/utils/src/index.ts b/packages/utils/src/index.ts index e875f0cc..e0afba03 100644 --- a/packages/utils/src/index.ts +++ b/packages/utils/src/index.ts @@ -57,5 +57,5 @@ export { } from './link-auth/resolve.js'; export type { ProviderMatch } from './link-auth/select-provider.js'; export type { RewriteRule } from './link-auth/rewrite.js'; -export type { TokenSource } from './link-auth/resolve-token.js'; +export { defaultRunCommand, type TokenSource } from './link-auth/resolve-token.js'; export { expandMacro, UnknownMacroError } from './link-auth/expand-macro.js'; diff --git a/packages/utils/src/link-auth/resolve-token.ts b/packages/utils/src/link-auth/resolve-token.ts index 54c6b795..2e52ebe9 100644 --- a/packages/utils/src/link-auth/resolve-token.ts +++ b/packages/utils/src/link-auth/resolve-token.ts @@ -37,7 +37,12 @@ export interface TokenResolutionDeps { readonly runCommand: (argv: readonly string[]) => { success: boolean; stdout: string }; } -const DEFAULT_RUN_COMMAND: TokenResolutionDeps['runCommand'] = (argv) => { +/** + * Default `runCommand` implementation — exported so callers that want to + * memoize per-validate-run can wrap it without duplicating the spawn logic. + * Forwards to `safeExecResult` (no shell, argv-based). + */ +export const defaultRunCommand: TokenResolutionDeps['runCommand'] = (argv) => { if (argv.length === 0) return { success: false, stdout: '' }; const [bin, ...args] = argv; if (bin === undefined) return { success: false, stdout: '' }; @@ -58,7 +63,7 @@ export function resolveToken( deps?: Partial, ): string | undefined { const env = deps?.env ?? process.env; - const runCommand = deps?.runCommand ?? DEFAULT_RUN_COMMAND; + const runCommand = deps?.runCommand ?? defaultRunCommand; for (const source of sources) { const value = tryResolveSource(source, env, runCommand);