From fb53a9e4da46eda2224203db8a9b5a5a92e9009b Mon Sep 17 00:00:00 2001 From: Ethan Dutton <46871249+ejdutton@users.noreply.github.com> Date: Mon, 6 Jul 2026 09:20:15 -0400 Subject: [PATCH 1/4] docs(changelog): rewrite linkAuth entries to be adopter-focused (0.1.39) Replace three implementation-dense slice entries (covering internal file names, test counts, and design section refs) with one clear entry that answers "what can I do and how do I configure it": built-in github/sharepoint macros, the five LINK_AUTH_* codes and their meanings, per-user CI-safe cache scoping, and the fetchAuthenticated() content-fetch primitive. Also removes the stale "not yet wired" Internal entry for slice 1. Co-Authored-By: Claude Sonnet 4.6 --- CHANGELOG.md | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 15c1b561..45e39219 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,9 +31,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **Skill-authoring guidance: portable bundled-script paths.** The `vibe-agent-toolkit:vat-skill-authoring` skill now documents how to reference bundled scripts/assets portably (relative to the skill directory, never `CLAUDE_PLUGIN_ROOT`/absolute/env-var anchors), and `vibe-agent-toolkit:vat-skill-review` carries the matching pre-publication checklist item. - **Skill-review guidance: reserved words `claude`/`anthropic` in skill names.** The `vibe-agent-toolkit:vat-skill-review` skill's Naming section now carries the reserved-word rule as a canonical `[A]` item — Anthropic's authoring guidance states a skill `name` "Cannot contain reserved words: 'anthropic', 'claude'", and Claude Code refuses to load a non-certified skill named that way, so it fails at install/validation, not just review (`[RESERVED_WORD_IN_NAME]`). Surfaced by dogfooding the skill against its own eval suite (the reviewer was noting the prefix as "redundant" but missing the install-blocking consequence). The rule directs the reviewer to surface that consequence when reviewing such a name and to include the warning when advising on naming. - **`NON_PORTABLE_COMMAND` validation code (default `warning`) — a portability check family.** `vat skills validate` / `vat audit` now flag a skill document that tells an agent to run a GNU/Linux-only shell command, scanning the `SKILL.md` body **and every reachable bundled markdown doc** (agents copy invocations from reference files too). It's a family of sub-checks under one code — `timeout`, `grep-pcre` (`grep -P`), `sed-i-no-backup` (`sed -i` with no suffix), `readlink-f`, and `date-d` (GNU `date -d`) — each finding names the variant and carries a tailored fix, and a single `validation.allow` entry silences the whole family for a file. Patterns match commands in command position only (not bare prose), so `grep -E`/`sed -i.bak` and nouns like "the request will timeout" are not flagged. Promotes a former manual `vat skill review` checklist line into an automated check. See [`NON_PORTABLE_COMMAND`](docs/validation-codes.md#non_portable_command). -- **linkAuth content-fetch primitive + content cache (issue #113, slice 3).** Ships the public `fetchAuthenticated(url, config, options) → { bytes, metadata, cached } | { outcome: 'unsupported' | 'unverified' }` primitive (`packages/resources/src/link-auth-content-fetch.ts`), per design §6.2 — *sibling to* the slice-2 health-check path, both reading from the same engine config and rewrite pipeline. No consumer wiring (asset-references, bundling) lands in this slice; the primitive ships standalone so future callers can adopt it without reworking the contract. **Two-mode headers (§6.2):** `Provider` gains an optional `fetch: { headers }` block alongside `auth: { headers }`; `resolveAuthenticatedUrl` now dual-expands both header sets against the same context (URL captures + resolved token), surfacing them on the success outcome as `headers` (auth, for health-check) and `fetchHeaders` (fetch, for content retrieval). The primitive merges `fetchHeaders` over `headers` so fetch-mode overrides on conflict — the canonical case being GitHub, where `Accept: application/vnd.github+json` returns 200 for any size but omits bytes >1 MiB (good for health-check) while `Accept: application/vnd.github.raw` streams the bytes inline (required for content). Adopter schema (`InlineProviderSchema`) gains a parallel `ProviderFetchSchema` (passthrough, like the rest of the adopter linkAuth tree per the repo Postel's Law rule), and the compile-time `_KeysAgree` drift check picks up the new field automatically. The resolved-token-wins precedence (URL-capture-named `token` cannot beat the resolved value) and the null-prototype hardening apply to `fetchHeaders` too. **`ContentCache` (§6.3, `packages/resources/src/content-cache.ts`):** new persistence class for the content-fetch primitive — distinct from slice 2's `ExternalLinkCache` (which is a status cache). Per-entry layout: `.json` (metadata: status, content-type, etag, last-modified, fetchedAt, rewrittenUrl, `version: 1`) + `.bin` (raw bytes), under a caller-supplied `cacheDir` (the validator-style `/content/auth-${osUser}/` scoping is the caller's responsibility — the class only knows about the directory it was given, mirroring §6.3's "cross-user isolation = OS user, not cache key"). 30-minute default TTL (`§6.3`), tunable via constructor and via the threaded-through `resources.linkAuth.cache.ttlMinutes` adopter config (`buildLinkAuthEngineConfig` now copies the adopter `cache` block onto the engine `LinkAuthConfig`, which previously dropped it silently). Write order is `.bin` first, then `.json` as the commit marker — a partial-write crash leaves either no entry or `.bin` ahead of `.json` (reads as a miss), never `.json` ahead of `.bin` (which would serve stale bytes under new metadata). On-disk metadata fields are whitelisted via a single `pickMetadata()` helper used by both `set()` (strip smuggled fields before write) and `get()` (strip the on-disk `version` before return), so token-bearing fields a caller might smuggle through structural typing cannot land on disk — defense in depth on top of the closed `ContentMetadata` interface. TTL boundary is `>`, not `>=` — entries are valid at exactly the TTL, expire at TTL + 1 ms; tests pin both boundary cases. Fail-soft IO per #125 review: `EACCES` / `EROFS` / corrupted JSON degrade to a miss (read) or no-op (write), never throw. Forward-compat `version: 1` mirrors `ExternalLinkCache`. **Primitive behavior:** the four outcome branches — `unsupported` and `unverified` short-circuit with no fetch and **no cache touch** (§6.3: never cache `unverified`, since the result flips the moment a token appears); cache-hit returns `{ bytes, metadata, cached: true }` with no fetch; otherwise fetch via `authTransport` (cross-origin auth strip + 429 retry inherited from slice 2), read the body binary-clean via `arrayBuffer()`, build metadata from response headers (content-type / etag / last-modified default to `null` when absent), write through to the cache if supplied. `forceRefresh: true` bypasses cache reads but still writes through. `AbortSignal` propagates to the transport. The token value is interpolated into request headers in-memory and never flows into `ContentMetadata`; an end-to-end test reads every file in the cache directory after a fetch and asserts the literal token string is absent. **`wrapLinkAuthDepsWithMemo` lifted to its own module** (`packages/resources/src/link-auth-deps-memo.ts`) and exported from the resources barrel — slice 2 originally housed it private inside `external-link-validator.ts`, but the standalone primitive needs the same memoization, and the lift centralizes the implementation so jscpd cannot flag a clone. Validators and primitive callers iterating many URLs from the same provider wrap their `deps` once and reuse, so `gh auth token` / any `command`-source resolver runs at most once across the iteration. **File rename for slice 2's transport:** `packages/resources/src/link-auth-fetch.ts` → `link-auth-transport.ts`, and the exported function `fetchAuthenticated` → `authTransport` (with `AuthFetchOptions` → `AuthTransportOptions`) — frees the `fetchAuthenticated` name for the spec-documented primitive and aligns the filename with the symbol's role as the lower-level auth-safe HTTP wrapper. **47 new tests:** `link-auth-content-fetch` (15 covering short-circuits, header merge with fetch.headers override, cache hit/miss/forceRefresh, unverified-never-cached, binary-clean round-trip, signal pass-through, token-never-persisted), `content-cache` (14 covering round-trip, binary safety, distinct-URL isolation, overwrite, TTL boundary at `=` and `=+1ms`, version-mismatch eviction, corrupted-JSON tolerance, POSIX-skipped `EACCES` fail-soft on read and write, and the whitelist-on-write check), `link-auth-deps-memo` (5 covering single-source memo, distinct-argv independence, default-runCommand fallback, deps pass-through, undefined-deps handling), and 13 augmenting tests on the slice 2 surface for the new `provider.fetch` block (engine dual-expansion, schema acceptance/rejection, cache field propagation). The slice 3 primitive does not wire into any existing CLI command — `--refresh` / `--no-cache` ships with the first consumer slice. -- **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. +- **Authenticated link checking for private GitHub and SharePoint URLs (`resources.linkAuth`, issue #113).** Add a `resources.linkAuth` block to `vibe-agent-toolkit.config.yaml` and `vat resources validate` will authenticate requests to configured hosts instead of fetching anonymously — fixing the long-standing problem where private GitHub repository links and SharePoint pages always appear dead. Two built-in macros ship ready to use: `use: github` (token via `gh auth token`) and `use: sharepoint` (token from the `SHAREPOINT_TOKEN` environment variable); full inline providers are supported for any other private host. Authenticated responses surface as five new validation codes rather than the generic `EXTERNAL_URL_*` set: `LINK_AUTH_DEAD` (error — confirmed dead link on a host that does not mask unauthorized responses as 404, e.g. SharePoint), `LINK_AUTH_DEAD_OR_UNAUTHORIZED` (warn — 404 on a host like GitHub that may mask 403 as 404), `LINK_AUTH_FORBIDDEN` (warn — 403, token accepted but insufficient access), `LINK_AUTH_UNAUTHORIZED` (warn — 401, token missing or rejected), and `LINK_AUTH_UNVERIFIED` (warn — no token resolved; the link was skipped). Authenticated results cache per OS user under `/auth-${user}/external-links.json`, so two runners on the same CI host cannot read each other's cache entries. Also ships `fetchAuthenticated(url, config)` as a new public export from `@vibe-agent-toolkit/resources` for retrieving the *bytes* of a private URL — useful when you need the file content, not just whether the link resolves. Pair it with the new optional `provider.fetch.headers` block to send different request headers for content retrieval than for link checking (e.g. `Accept: application/vnd.github.raw` to stream raw bytes inline versus `Accept: application/vnd.github+json` for the metadata-only link-health check). - **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. - **Cowork driver spike.** Added [`docs/contributing/cowork-driver-spike.md`](docs/contributing/cowork-driver-spike.md) — a time-boxed investigation (per §4a of the harness v2 design) of whether `claude-cowork` can be driven programmatically by the empirical compat harness today. Verdict: **not feasible**; cowork is a Claude Desktop app product with no public API/CLI surface. The `claude-cowork` runtime stays on `scripted-assisted` until Anthropic ships a Cowork CLI mode, Sessions API, or documented filesystem-import path. Adjacent finding (not a cowork replacement): the public-beta Skills API (`POST /v1/skills` + `container.skills[]` on `/v1/messages`) supports a fully-automatable *new* runtime — captured in the spike doc as a potential follow-up, gated on a separate design decision. @@ -95,7 +93,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **Skill-test eval fixtures excluded from the remaining link/structure validators (CI hygiene, no adopter-facing change).** The intentionally-broken eval fixtures (`resources/skills/evals/**` — non-portable SKILL.md samples, a fake plugin for `vat audit`) are test input, not real docs/code. They were already excluded from the repo-root resource validation, ESLint, and repo-structure checks; now also from the `vat-development-agents` package config (so `vat verify`'s resources phase stops failing on the fixtures' deliberate `LINK_BROKEN_FILE`s) and the `project-validation` dogfooding system test (hardcoded exclude list). Every exclusion site cross-references the others. - **Eval fixtures hold clean, realistic code — incidental smells removed.** Two fixtures carried code-quality issues unrelated to what their eval tests: the `release-notifier-plugin` notifier script (a payload that only needs to *exist* so `vat audit` can flag the skill's local-script dependency) now validates its `--changelog` path instead of opening it blind, and the `vat-knowledge-resources` starter config dropped a redundant `TODO` comment (the eval's prompt already states the task). Fixtures that are themselves the *subject under review* (e.g. the vat-agent-authoring analyzer the eval asks an agent to improve) keep their VAT-domain flaws by design. - **Unified `resolveSkillSource` skill-source resolver (#132, foundation).** A `skill-source/` module in `@vibe-agent-toolkit/agent-skills` that materializes a typed source union (`workspace` / `npm` / `url(+sha256)` / `path` / `vendored`) to a hardened, content-addressed staged directory through a per-user, `0700`, uid-checked fetch cache. The git-URL parser moved from `@vibe-agent-toolkit/cli` to `@vibe-agent-toolkit/utils`. No user-facing CLI surface yet — this is the resolver consumed by `vat skill test`. -- **Authenticated external-URL resolution foundation (issue #113, slice 1).** A pure `link-auth/` engine in `@vibe-agent-toolkit/utils` (host-glob provider selection, ordered token sources with no shell, header rendering with `Authorization` redaction, `github`/`sharepoint` macros) plus a strict `resources.linkAuth` config schema in `@vibe-agent-toolkit/resources`. Not yet wired into validation — consumer integration and the `LINK_AUTH_*` codes land in later slices — so there is no user-facing behavior yet. - **`corpus/seed.yaml` is now generated from the upstream Anthropic marketplaces (issue #99, slice 1b).** A committed importer (`bun run import-marketplace [--allow-shrink]`) fetches the `claude-plugins-official` and `knowledge-work-plugins` catalogs, deduplicates by `source` URL, and rewrites the seed — replacing the previously hand-maintained list. Re-import is guarded against accidental shrinkage (refuses to overwrite on a 0-plugin fetch or a >20% drop unless `--allow-shrink`); current entry counts and audit provenance live in the generated seed header. - **Empirical compatibility harness (issue #100).** A research scaffold (`packages/dev-tools/src/compat-empirical/`) for measuring skill compatibility across `claude-code`, `claude-cowork`, and `claude-chat` — it joins VAT's static predictions with deterministic runtime observations and an LLM-judge read into a reality-vs-prediction matrix, as evidence for future detector improvements. Lives entirely in the private `dev-tools` package with no adopter-facing surface; no detector or `RUNTIME_PROFILES` changes. [Design](./docs/research/2026-05-23-compat-empirical-harness-v2-design.md). - **Cowork driver spike.** [`docs/contributing/cowork-driver-spike.md`](docs/contributing/cowork-driver-spike.md) records a time-boxed finding that `claude-cowork` cannot currently be driven programmatically (no public API/CLI surface), so it stays on `scripted-assisted` in the compat harness. Notes the public-beta Skills API as a separate, fully-automatable runtime worth a future follow-up. From 3336a108b0119256ea57b943fb8f530648980e4d Mon Sep 17 00:00:00 2001 From: Ethan Dutton <46871249+ejdutton@users.noreply.github.com> Date: Mon, 6 Jul 2026 09:59:17 -0400 Subject: [PATCH 2/4] =?UTF-8?q?feat(link-auth):=20slice=204=20=E2=80=94=20?= =?UTF-8?q?GIT=5F*=20scrubbing,=20ALLOW=5FCOMMAND=20opt-out,=20system=20te?= =?UTF-8?q?st,=20contributor=20docs=20(issue=20#113)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Strip GIT_* env vars in defaultRunCommand so `gh auth token` works from inside git pre-commit hooks where GIT_DIR/GIT_WORK_TREE are set - Add allowCommand field to TokenResolutionDeps (default: env var VAT_LINKAUTH_ALLOW_COMMAND !== '0'); command sources are skipped when false — escape hatch for security-sensitive CI environments - Add cross-platform system test exercising real git/gh binaries through resolveToken (covers .cmd shim path on Windows) - Add contributor guide at docs/contributing/vat-linkauth-contributing.md - Bump version to 0.1.40-rc.1 Co-Authored-By: Claude Sonnet 4.6 --- CHANGELOG.md | 9 + bun.lock | 46 ++--- .../contributing/vat-linkauth-contributing.md | 192 ++++++++++++++++++ package.json | 2 +- packages/agent-config/package.json | 2 +- packages/agent-runtime/package.json | 2 +- packages/agent-schema/package.json | 2 +- packages/agent-skills/package.json | 2 +- packages/claude-marketplace/package.json | 2 +- packages/cli/package.json | 2 +- packages/dev-tools/package.json | 2 +- packages/discovery/package.json | 2 +- packages/gateway-mcp/package.json | 2 +- packages/rag-lancedb/package.json | 2 +- packages/rag/package.json | 2 +- packages/resource-compiler/package.json | 2 +- packages/resources/package.json | 2 +- .../runtime-claude-agent-sdk/package.json | 2 +- packages/runtime-langchain/package.json | 2 +- packages/runtime-openai/package.json | 2 +- packages/runtime-vercel-ai-sdk/package.json | 2 +- packages/test-agents/package.json | 2 +- packages/transports/package.json | 2 +- packages/utils/package.json | 2 +- packages/utils/src/link-auth/resolve-token.ts | 26 ++- .../test/link-auth/resolve-token.test.ts | 42 ++++ .../link-auth-token-dispatch.system.test.ts | 98 +++++++++ packages/vat-development-agents/package.json | 2 +- packages/vat-example-cat-agents/package.json | 2 +- packages/vibe-agent-toolkit/package.json | 2 +- 30 files changed, 413 insertions(+), 48 deletions(-) create mode 100644 docs/contributing/vat-linkauth-contributing.md create mode 100644 packages/utils/test/system/link-auth-token-dispatch.system.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 45e39219..cb9d279a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- **`VAT_LINKAUTH_ALLOW_COMMAND=0` opt-out for token command sources (issue #113 §6.2).** Set `VAT_LINKAUTH_ALLOW_COMMAND=0` in your environment to skip all `{ command: ... }` token sources at runtime — only `{ env: ... }` sources are tried. Useful in security-sensitive CI environments or policies that prohibit arbitrary child-process execution from the link validator. The opt-out can also be set programmatically via `allowCommand: false` in the injected `TokenResolutionDeps`. +- **Contributor guide for the linkAuth engine** at [`docs/contributing/vat-linkauth-contributing.md`](docs/contributing/vat-linkauth-contributing.md): engine vocabulary, how to add a new provider macro, token resolution mechanics, the GIT_* scrubbing rationale, testing requirements, and code style notes. + +### Fixed + +- **`gh auth token` (and other token commands) no longer fail when `vat resources validate` runs from a git pre-commit hook (issue #113 §6.1).** Git sets `GIT_DIR`, `GIT_WORK_TREE`, `GIT_INDEX_FILE`, and related vars before invoking hooks; these poison any tool that internally shells out to git, including `gh auth token`. The default token-command runner now strips all `GIT_*` vars from the environment before spawning, so authenticated link checking works correctly in both hook and non-hook contexts. + ## [0.1.39] - 2026-07-03 ### Added diff --git a/bun.lock b/bun.lock index ef4589c2..b6dc7239 100644 --- a/bun.lock +++ b/bun.lock @@ -43,7 +43,7 @@ }, "packages/agent-config": { "name": "@vibe-agent-toolkit/agent-config", - "version": "0.1.39", + "version": "0.1.40-rc.1", "dependencies": { "@vibe-agent-toolkit/agent-schema": "workspace:*", "@vibe-agent-toolkit/utils": "workspace:*", @@ -57,7 +57,7 @@ }, "packages/agent-runtime": { "name": "@vibe-agent-toolkit/agent-runtime", - "version": "0.1.39", + "version": "0.1.40-rc.1", "dependencies": { "@vibe-agent-toolkit/agent-schema": "workspace:*", "@vibe-agent-toolkit/utils": "workspace:*", @@ -72,7 +72,7 @@ }, "packages/agent-schema": { "name": "@vibe-agent-toolkit/agent-schema", - "version": "0.1.39", + "version": "0.1.40-rc.1", "dependencies": { "picomatch": "^4.0.3", "zod": "^3.23.8", @@ -88,7 +88,7 @@ }, "packages/agent-skills": { "name": "@vibe-agent-toolkit/agent-skills", - "version": "0.1.39", + "version": "0.1.40-rc.1", "dependencies": { "@vibe-agent-toolkit/agent-config": "workspace:*", "@vibe-agent-toolkit/agent-schema": "workspace:*", @@ -114,7 +114,7 @@ }, "packages/claude-marketplace": { "name": "@vibe-agent-toolkit/claude-marketplace", - "version": "0.1.39", + "version": "0.1.40-rc.1", "dependencies": { "@vibe-agent-toolkit/agent-schema": "workspace:*", "@vibe-agent-toolkit/agent-skills": "workspace:*", @@ -132,7 +132,7 @@ }, "packages/cli": { "name": "@vibe-agent-toolkit/cli", - "version": "0.1.39", + "version": "0.1.40-rc.1", "bin": { "vat": "./dist/bin/vat.js", }, @@ -169,7 +169,7 @@ }, "packages/dev-tools": { "name": "@vibe-agent-toolkit/dev-tools", - "version": "0.1.39", + "version": "0.1.40-rc.1", "dependencies": { "@vibe-agent-toolkit/agent-skills": "workspace:*", "@vibe-agent-toolkit/claude-marketplace": "workspace:*", @@ -192,7 +192,7 @@ }, "packages/discovery": { "name": "@vibe-agent-toolkit/discovery", - "version": "0.1.39", + "version": "0.1.40-rc.1", "dependencies": { "@vibe-agent-toolkit/utils": "workspace:*", "picomatch": "^4.0.2", @@ -203,7 +203,7 @@ }, "packages/gateway-mcp": { "name": "@vibe-agent-toolkit/gateway-mcp", - "version": "0.1.39", + "version": "0.1.40-rc.1", "dependencies": { "@modelcontextprotocol/sdk": "^1.0.4", "@vibe-agent-toolkit/agent-schema": "workspace:*", @@ -219,7 +219,7 @@ }, "packages/rag": { "name": "@vibe-agent-toolkit/rag", - "version": "0.1.39", + "version": "0.1.40-rc.1", "dependencies": { "@vibe-agent-toolkit/resources": "workspace:*", "@vibe-agent-toolkit/utils": "workspace:*", @@ -240,7 +240,7 @@ }, "packages/rag-lancedb": { "name": "@vibe-agent-toolkit/rag-lancedb", - "version": "0.1.39", + "version": "0.1.40-rc.1", "dependencies": { "@lancedb/lancedb": "^0.23.0", "@vibe-agent-toolkit/rag": "workspace:*", @@ -257,7 +257,7 @@ }, "packages/resource-compiler": { "name": "@vibe-agent-toolkit/resource-compiler", - "version": "0.1.39", + "version": "0.1.40-rc.1", "bin": { "vat-compile-resources": "./bin/vat-compile-resources", }, @@ -278,7 +278,7 @@ }, "packages/resources": { "name": "@vibe-agent-toolkit/resources", - "version": "0.1.39", + "version": "0.1.40-rc.1", "dependencies": { "@vibe-agent-toolkit/agent-schema": "workspace:*", "@vibe-agent-toolkit/utils": "workspace:*", @@ -309,7 +309,7 @@ }, "packages/runtime-claude-agent-sdk": { "name": "@vibe-agent-toolkit/runtime-claude-agent-sdk", - "version": "0.1.39", + "version": "0.1.40-rc.1", "dependencies": { "@anthropic-ai/claude-agent-sdk": "^0.1.5", "@anthropic-ai/sdk": "^0.37.0", @@ -328,7 +328,7 @@ }, "packages/runtime-langchain": { "name": "@vibe-agent-toolkit/runtime-langchain", - "version": "0.1.39", + "version": "0.1.40-rc.1", "dependencies": { "@langchain/core": "^0.3.29", "@vibe-agent-toolkit/agent-runtime": "workspace:*", @@ -346,7 +346,7 @@ }, "packages/runtime-openai": { "name": "@vibe-agent-toolkit/runtime-openai", - "version": "0.1.39", + "version": "0.1.40-rc.1", "dependencies": { "@vibe-agent-toolkit/agent-runtime": "workspace:*", "openai": "^4.77.3", @@ -364,7 +364,7 @@ }, "packages/runtime-vercel-ai-sdk": { "name": "@vibe-agent-toolkit/runtime-vercel-ai-sdk", - "version": "0.1.39", + "version": "0.1.40-rc.1", "dependencies": { "@ai-sdk/provider": "^3.0.4", "@ai-sdk/provider-utils": "^4.0.8", @@ -385,7 +385,7 @@ }, "packages/test-agents": { "name": "@vibe-agent-toolkit/test-agents", - "version": "0.1.39", + "version": "0.1.40-rc.1", "dependencies": { "@vibe-agent-toolkit/agent-runtime": "workspace:*", "zod": "^3.24.1", @@ -398,7 +398,7 @@ }, "packages/transports": { "name": "@vibe-agent-toolkit/transports", - "version": "0.1.39", + "version": "0.1.40-rc.1", "dependencies": { "@vibe-agent-toolkit/agent-runtime": "workspace:*", "ws": "^8.18.0", @@ -412,7 +412,7 @@ }, "packages/utils": { "name": "@vibe-agent-toolkit/utils", - "version": "0.1.39", + "version": "0.1.40-rc.1", "dependencies": { "handlebars": "^4.7.8", "ignore": "^7.0.5", @@ -434,7 +434,7 @@ }, "packages/vat-development-agents": { "name": "@vibe-agent-toolkit/vat-development-agents", - "version": "0.1.39", + "version": "0.1.40-rc.1", "dependencies": { "@vibe-agent-toolkit/agent-schema": "workspace:*", "@vibe-agent-toolkit/cli": "workspace:*", @@ -449,7 +449,7 @@ }, "packages/vat-example-cat-agents": { "name": "@vibe-agent-toolkit/vat-example-cat-agents", - "version": "0.1.39", + "version": "0.1.40-rc.1", "dependencies": { "@vibe-agent-toolkit/agent-runtime": "workspace:*", "@vibe-agent-toolkit/agent-schema": "workspace:*", @@ -475,7 +475,7 @@ }, "packages/vibe-agent-toolkit": { "name": "vibe-agent-toolkit", - "version": "0.1.39", + "version": "0.1.40-rc.1", "bin": { "vat": "./bin/vat", }, diff --git a/docs/contributing/vat-linkauth-contributing.md b/docs/contributing/vat-linkauth-contributing.md new file mode 100644 index 00000000..a4949215 --- /dev/null +++ b/docs/contributing/vat-linkauth-contributing.md @@ -0,0 +1,192 @@ +# Contributor Guide: linkAuth Engine + +This guide is for developers working on the linkAuth feature inside VAT itself. For +adopter documentation (how to configure `resources.linkAuth`), see the main README and +`vibe-agent-toolkit.config.yaml` reference. + +Design: issue #113 in `jdutton/vibe-agent-toolkit`. + +## Architecture overview + +The linkAuth pipeline has three layers: + +``` +Adopter YAML config + ↓ + Engine (packages/utils/src/link-auth/) + ↓ builds LinkAuthConfig, resolves providers, runs rewrites + Validator (packages/resources/src/link-auth-validator.ts) + ↓ calls engine, classifies results as LINK_AUTH_* codes + vat resources validate +``` + +### Engine vocabulary (packages/utils/src/link-auth/) + +| File | Responsibility | +|---|---| +| `macros.yaml` | Shipped provider macros (`github`, `sharepoint`) — YAML entries, not TS | +| `expand-macro.ts` | Loads `macros.yaml`, applies adopter deep-merge overrides | +| `resolve.ts` | Engine entry point: `resolveAuthenticatedUrl(url, config, deps)` — picks provider, runs rewrites, resolves token, plans the fetch | +| `select-provider.ts` | Match a URL against configured providers (host + `excludeHost` rules) | +| `rewrite.ts` | URL rewrite rules (regex `when` + `to` with `${var}` substitution) | +| `resolve-token.ts` | Token source resolution (`env:` + `command:` sources, GIT_* scrubbing, `VAT_LINKAUTH_ALLOW_COMMAND` opt-out) | +| `build-headers.ts` | Build request headers, substitute `${token}` into templates | +| `template.ts` | Generic `${…}` template substitution with allowlist enforcement | +| `transforms.ts` | Transform functions callable inside templates (e.g. `base64url`) — the safety allowlist for template calls | + +### Validator wiring (packages/resources/src/) + +| File | Responsibility | +|---|---| +| `schemas/link-auth.ts` | Zod schema for the `resources.linkAuth` YAML block | +| `link-auth-config-build.ts` | `buildLinkAuthEngineConfig()` — turns the adopter config into an engine-ready `LinkAuthConfig` | +| `link-auth-content-fetch.ts` | Orchestrates: call engine → fetch via `authTransport` → return response for classification | +| `link-auth-classify.ts` | Maps HTTP outcomes to `LINK_AUTH_*` validation codes per the provider's `check` block | +| `link-auth-transport.ts` | `authTransport` — HTTP fetch primitive. Cross-origin auth strip, `Retry-After`, timeout, signal propagation | +| `external-link-validator.ts` | Validator entry point invoked by `vat resources validate` | + +### Content cache + +`ExternalLinkCache` (packages/resources/src/external-link-cache.ts) stores auth results +under `/auth-${sanitizedOsUser}/external-links.json`. Cache entries carry +`version: 1`; a version mismatch triggers a re-fetch. Do not cache derived LINK_AUTH_* +codes — only the raw `statusCode`; re-classify on every cache hit under the current +provider's `check` block. + +## Adding a new built-in provider macro + +A macro is a shorthand that expands to a full inline provider. The two shipped macros +(`github`, `sharepoint`) live in `packages/utils/src/link-auth/macros.yaml` — as YAML +entries, not TypeScript. The macro loader (`expand-macro.ts`) reads the file once at +module init and applies adopter deep-merge overrides at runtime. + +To add a new macro `myprovider`: + +1. **Add the entry** to `packages/utils/src/link-auth/macros.yaml`: + ```yaml + myprovider: + match: + host: myprovider.example + rewrite: + - when: '^https://myprovider\.example/(?.+)$' + to: 'https://api.myprovider.example/v1/${path}' + auth: + headers: + Authorization: 'Bearer ${token}' + token: + - env: MYPROVIDER_TOKEN + check: + method: GET + aliveStatus: [200] + notFoundMeaning: dead + ``` + + Note: `use:` in the adopter YAML is validated by `expand-macro.ts` at runtime + (throws `UnknownMacroError`), not by an enum in the Zod schema. Adding a + macro is a **YAML-only** change — no code edit to `schemas/link-auth.ts` is + required. + +2. **Write unit tests** for the expansion in + `packages/utils/test/link-auth/expand-macro.test.ts`. Cover: base expansion, + at least one adopter override, and — if applicable — the "no zero-config + token source" case (see the `sharepoint` tests for the pattern). + +3. **Write an integration test** in `packages/resources/test/` that verifies a + roundtrip through `buildLinkAuthEngineConfig` with `use: 'myprovider'`. + +4. **Document** the macro in the main README and in the linkAuth section of + `packages/vat-development-agents/resources/skills/vat-knowledge-resources.md` + (the skill that surfaces linkAuth config to agents). + +Per issue #113 §10: the design explicitly limits shipped macros to hosts where auth is +universally needed and the pattern is stable. Do not add macros speculatively — open an +issue and wait for adopter demand. + +## Token resolution (resolve-token.ts) + +### Source priority + +Sources are tried in order; the first non-empty value wins: + +```yaml +token: + - env: GITHUB_TOKEN # read process.env.GITHUB_TOKEN + - command: gh auth token # spawn gh, trim stdout +``` + +### Command execution + +Commands run via `safeExecResult` with `shell: false` (argv-based spawn, no shell). The +string form `command: "gh auth token"` is whitespace-tokenized; shell operators (`|`, +`&&`) become literal argv elements — they are **not** pipes. + +### GIT_* environment scrubbing + +`defaultRunCommand` strips all `GIT_*` env vars before spawning. This matters because +`vat resources validate` is often run from a git pre-commit hook, where git sets +`GIT_DIR`, `GIT_WORK_TREE`, `GIT_INDEX_FILE`, and other vars. These interfere with any +tool that internally shells out to git (most notably `gh auth token`). Without scrubbing, +`gh auth token` fails inside a pre-commit hook. + +If you add a new default command runner or wrap `defaultRunCommand`, preserve the +scrubbing. + +### VAT_LINKAUTH_ALLOW_COMMAND=0 + +Set this env var (or pass `allowCommand: false` in `TokenResolutionDeps`) to skip all +`{ command: ... }` sources at runtime. Only `{ env: ... }` sources are tried. Useful in +security-sensitive environments or when the CI policy prohibits arbitrary child-process +execution from the validator. + +This is an escape hatch, not a security boundary. Operators who need a hard block should +not configure `command:` sources in the first place. + +## Testing requirements + +### Unit tests + +Every new source type, macro, rewrite rule, or token-resolution behaviour needs a unit +test in `packages/utils/test/link-auth/`. Use injected `deps` — never depend on ambient +`process.env` state or real network calls. + +Test the **transform allowlist** (`packages/utils/src/link-auth/transforms.ts` — the +map of names to transform functions, callable inside `${…}` templates) for any new +allowed transform. The allowlist protects against arbitrary function invocation via +config; a bypassed transform is a security issue. + +Test the **header template** expansion (`${token}` substitution). Verify that a token +containing `}` or other special characters does not escape the template. + +### Integration tests + +`packages/utils/test/integration/` — test that the engine wires together correctly with +the real Zod schema and the real macro expansions. No real network calls; inject a mock +transport. + +### System tests + +`packages/utils/test/system/link-auth-token-dispatch.system.test.ts` — exercises real +binaries (`git`, `gh`) through `resolveToken` with no injected deps. This is the +cross-platform canary: on Windows, binaries are `.cmd` shims and dispatch goes through +`shouldUseShell` in `safe-exec.ts`. Keep this test in sync when you change how +`defaultRunCommand` spawns processes. + +### What the system test does NOT cover + +- Real authenticated HTTP requests (too flaky for CI; test with a mock transport) +- The full validator pipeline (covered by resources integration tests) +- Cache persistence (covered by resources unit tests via `ExternalLinkCache`) + +## Code style + +Follow the project-wide conventions in `CLAUDE.md`. A few linkAuth-specific notes: + +- **No shell execution.** The entire pipeline deliberately avoids `shell: true`. Do not + introduce shell strings anywhere in the engine. +- **Fail-soft I/O.** Cache reads and writes use fail-soft error handling. Never let a + cache I/O failure propagate to the adopter as a hard error. +- **Re-classify on cache hit.** Never cache the derived `LINK_AUTH_*` code; cache only + `statusCode`. The adopter's `check` block may be updated between runs. +- **No adopter-visible field renames without a version bump.** The cache `version: 1` + field gates reads. If you change the cache entry shape, bump the version constant and + update the cache-miss path. diff --git a/package.json b/package.json index a2374bcb..1b5432ed 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "vibe-agent-toolkit", - "version": "0.1.39", + "version": "0.1.40-rc.1", "type": "module", "private": true, "description": "Toolkit for testing and packaging portable AI agents across various LLMs, frameworks, and deployment targets", diff --git a/packages/agent-config/package.json b/packages/agent-config/package.json index a0d15203..59f787a4 100644 --- a/packages/agent-config/package.json +++ b/packages/agent-config/package.json @@ -1,6 +1,6 @@ { "name": "@vibe-agent-toolkit/agent-config", - "version": "0.1.39", + "version": "0.1.40-rc.1", "description": "Agent manifest loading and validation", "type": "module", "main": "./dist/index.js", diff --git a/packages/agent-runtime/package.json b/packages/agent-runtime/package.json index 61bc25e4..74c6779d 100644 --- a/packages/agent-runtime/package.json +++ b/packages/agent-runtime/package.json @@ -1,6 +1,6 @@ { "name": "@vibe-agent-toolkit/agent-runtime", - "version": "0.1.39", + "version": "0.1.40-rc.1", "type": "module", "description": "Runtime framework for building and executing portable AI agents", "keywords": [ diff --git a/packages/agent-schema/package.json b/packages/agent-schema/package.json index 32c9007d..80a90c1a 100644 --- a/packages/agent-schema/package.json +++ b/packages/agent-schema/package.json @@ -1,6 +1,6 @@ { "name": "@vibe-agent-toolkit/agent-schema", - "version": "0.1.39", + "version": "0.1.40-rc.1", "description": "JSON Schema definitions and TypeScript types for VAT agent manifest format", "type": "module", "main": "./dist/index.js", diff --git a/packages/agent-skills/package.json b/packages/agent-skills/package.json index efc88b82..7564f0db 100644 --- a/packages/agent-skills/package.json +++ b/packages/agent-skills/package.json @@ -1,6 +1,6 @@ { "name": "@vibe-agent-toolkit/agent-skills", - "version": "0.1.39", + "version": "0.1.40-rc.1", "description": "Build, validate, and package agent skills in the Agent Skills format", "type": "module", "main": "./dist/index.js", diff --git a/packages/claude-marketplace/package.json b/packages/claude-marketplace/package.json index 406109f4..f4933476 100644 --- a/packages/claude-marketplace/package.json +++ b/packages/claude-marketplace/package.json @@ -1,6 +1,6 @@ { "name": "@vibe-agent-toolkit/claude-marketplace", - "version": "0.1.39", + "version": "0.1.40-rc.1", "description": "Claude plugin marketplace tools: compatibility analysis, provenance tracking, enterprise config", "type": "module", "main": "./dist/index.js", diff --git a/packages/cli/package.json b/packages/cli/package.json index 87aaa712..450cb307 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -1,6 +1,6 @@ { "name": "@vibe-agent-toolkit/cli", - "version": "0.1.39", + "version": "0.1.40-rc.1", "description": "Command-line interface for vibe-agent-toolkit", "type": "module", "bin": { diff --git a/packages/dev-tools/package.json b/packages/dev-tools/package.json index 522ab65e..74502b4c 100644 --- a/packages/dev-tools/package.json +++ b/packages/dev-tools/package.json @@ -1,6 +1,6 @@ { "name": "@vibe-agent-toolkit/dev-tools", - "version": "0.1.39", + "version": "0.1.40-rc.1", "private": true, "type": "module", "description": "Development tools for the monorepo", diff --git a/packages/discovery/package.json b/packages/discovery/package.json index 396e9469..641cc940 100644 --- a/packages/discovery/package.json +++ b/packages/discovery/package.json @@ -1,6 +1,6 @@ { "name": "@vibe-agent-toolkit/discovery", - "version": "0.1.39", + "version": "0.1.40-rc.1", "description": "Intelligent file discovery for VAT agents and Agent Skills", "type": "module", "main": "./dist/index.js", diff --git a/packages/gateway-mcp/package.json b/packages/gateway-mcp/package.json index 5e611ac4..900c3846 100644 --- a/packages/gateway-mcp/package.json +++ b/packages/gateway-mcp/package.json @@ -1,6 +1,6 @@ { "name": "@vibe-agent-toolkit/gateway-mcp", - "version": "0.1.39", + "version": "0.1.40-rc.1", "description": "MCP Gateway for exposing VAT agents through Model Context Protocol", "type": "module", "main": "./dist/index.js", diff --git a/packages/rag-lancedb/package.json b/packages/rag-lancedb/package.json index 0ece6038..22b31f12 100644 --- a/packages/rag-lancedb/package.json +++ b/packages/rag-lancedb/package.json @@ -1,6 +1,6 @@ { "name": "@vibe-agent-toolkit/rag-lancedb", - "version": "0.1.39", + "version": "0.1.40-rc.1", "type": "module", "description": "LanceDB implementation of RAG interfaces for vibe-agent-toolkit", "keywords": [ diff --git a/packages/rag/package.json b/packages/rag/package.json index a03b0ff0..8ebdfb3c 100644 --- a/packages/rag/package.json +++ b/packages/rag/package.json @@ -1,6 +1,6 @@ { "name": "@vibe-agent-toolkit/rag", - "version": "0.1.39", + "version": "0.1.40-rc.1", "type": "module", "description": "Abstract RAG (Retrieval-Augmented Generation) interfaces and shared implementations", "keywords": [ diff --git a/packages/resource-compiler/package.json b/packages/resource-compiler/package.json index 642c63b5..0af1c50c 100644 --- a/packages/resource-compiler/package.json +++ b/packages/resource-compiler/package.json @@ -1,6 +1,6 @@ { "name": "@vibe-agent-toolkit/resource-compiler", - "version": "0.1.39", + "version": "0.1.40-rc.1", "description": "Compile markdown resources to TypeScript with full IDE support", "type": "module", "main": "./dist/index.js", diff --git a/packages/resources/package.json b/packages/resources/package.json index fd7bf04b..12338508 100644 --- a/packages/resources/package.json +++ b/packages/resources/package.json @@ -1,6 +1,6 @@ { "name": "@vibe-agent-toolkit/resources", - "version": "0.1.39", + "version": "0.1.40-rc.1", "description": "Markdown resource parsing, validation, and link integrity checking", "type": "module", "main": "./dist/index.js", diff --git a/packages/runtime-claude-agent-sdk/package.json b/packages/runtime-claude-agent-sdk/package.json index 56015ae3..543d670c 100644 --- a/packages/runtime-claude-agent-sdk/package.json +++ b/packages/runtime-claude-agent-sdk/package.json @@ -1,6 +1,6 @@ { "name": "@vibe-agent-toolkit/runtime-claude-agent-sdk", - "version": "0.1.39", + "version": "0.1.40-rc.1", "description": "Claude Agent SDK runtime adapter for VAT agents", "type": "module", "exports": { diff --git a/packages/runtime-langchain/package.json b/packages/runtime-langchain/package.json index 231b52ad..f400f87d 100644 --- a/packages/runtime-langchain/package.json +++ b/packages/runtime-langchain/package.json @@ -1,6 +1,6 @@ { "name": "@vibe-agent-toolkit/runtime-langchain", - "version": "0.1.39", + "version": "0.1.40-rc.1", "description": "LangChain.js runtime adapter for VAT agents", "type": "module", "exports": { diff --git a/packages/runtime-openai/package.json b/packages/runtime-openai/package.json index 87e72944..fb6c55aa 100644 --- a/packages/runtime-openai/package.json +++ b/packages/runtime-openai/package.json @@ -1,6 +1,6 @@ { "name": "@vibe-agent-toolkit/runtime-openai", - "version": "0.1.39", + "version": "0.1.40-rc.1", "description": "OpenAI SDK runtime adapter for VAT agents", "type": "module", "exports": { diff --git a/packages/runtime-vercel-ai-sdk/package.json b/packages/runtime-vercel-ai-sdk/package.json index 1805a315..e4aed007 100644 --- a/packages/runtime-vercel-ai-sdk/package.json +++ b/packages/runtime-vercel-ai-sdk/package.json @@ -1,6 +1,6 @@ { "name": "@vibe-agent-toolkit/runtime-vercel-ai-sdk", - "version": "0.1.39", + "version": "0.1.40-rc.1", "description": "Vercel AI SDK runtime adapter for VAT agents", "type": "module", "exports": { diff --git a/packages/test-agents/package.json b/packages/test-agents/package.json index b9183941..ce6f2e49 100644 --- a/packages/test-agents/package.json +++ b/packages/test-agents/package.json @@ -1,6 +1,6 @@ { "name": "@vibe-agent-toolkit/test-agents", - "version": "0.1.39", + "version": "0.1.40-rc.1", "private": true, "description": "Simple test agents for validating runtime adapters (internal use only)", "type": "module", diff --git a/packages/transports/package.json b/packages/transports/package.json index 4cf0da1d..70d17b86 100644 --- a/packages/transports/package.json +++ b/packages/transports/package.json @@ -1,6 +1,6 @@ { "name": "@vibe-agent-toolkit/transports", - "version": "0.1.39", + "version": "0.1.40-rc.1", "description": "Transport adapters for VAT conversational agents", "type": "module", "main": "./dist/index.js", diff --git a/packages/utils/package.json b/packages/utils/package.json index 2bba2cdd..e784c3ba 100644 --- a/packages/utils/package.json +++ b/packages/utils/package.json @@ -1,6 +1,6 @@ { "name": "@vibe-agent-toolkit/utils", - "version": "0.1.39", + "version": "0.1.40-rc.1", "type": "module", "description": "Core utility functions with no external dependencies", "keywords": [ diff --git a/packages/utils/src/link-auth/resolve-token.ts b/packages/utils/src/link-auth/resolve-token.ts index 2e52ebe9..e2949e1f 100644 --- a/packages/utils/src/link-auth/resolve-token.ts +++ b/packages/utils/src/link-auth/resolve-token.ts @@ -9,6 +9,10 @@ * argv. **Not** passed through a shell — operators (`|`, `&&`, `$(...)`) * become literal argv elements, per the design's §6.1 sharp-edge note. * + * Command sources can be disabled at runtime with `VAT_LINKAUTH_ALLOW_COMMAND=0` + * (or by passing `allowCommand: false` in deps). Useful in security-sensitive + * environments where arbitrary command execution is undesirable. + * * Returns `undefined` if every source fails or yields an empty/whitespace * value — the caller's `resolveAuthenticatedUrl` translates that to the * `unverified` outcome (surfaced as `LINK_AUTH_UNVERIFIED` by the validator). @@ -35,18 +39,36 @@ export interface TokenResolutionDeps { * propagated by `resolveToken` — they indicate operator-level bugs. */ readonly runCommand: (argv: readonly string[]) => { success: boolean; stdout: string }; + + /** + * Whether `{ command: ... }` sources are allowed. Defaults to + * `process.env['VAT_LINKAUTH_ALLOW_COMMAND'] !== '0'`. Set to `false` (or + * export `VAT_LINKAUTH_ALLOW_COMMAND=0`) to skip all command sources and rely + * solely on env-var sources — useful in locked-down CI or security reviews. + */ + readonly allowCommand: boolean; } /** * 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). + * + * Strips `GIT_*` env vars before spawning so token commands like `gh auth token` + * work correctly from inside git pre-commit hooks, where git sets `GIT_DIR`, + * `GIT_WORK_TREE`, and similar vars that confuse nested git calls. */ 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: '' }; - const result = safeExecResult(bin, [...args], { encoding: 'utf8' }); + // Case-insensitive on the key so Windows env vars (which are case-insensitive + // at the OS level, though `process.env` preserves original case) can't sneak + // through as e.g. `Git_Dir`. + const env = Object.fromEntries( + Object.entries(process.env).filter(([k]) => !k.toUpperCase().startsWith('GIT_')), + ) as NodeJS.ProcessEnv; + const result = safeExecResult(bin, [...args], { encoding: 'utf8', env }); const stdout = typeof result.stdout === 'string' ? result.stdout : result.stdout.toString('utf8'); return { success: result.success, stdout }; }; @@ -64,8 +86,10 @@ export function resolveToken( ): string | undefined { const env = deps?.env ?? process.env; const runCommand = deps?.runCommand ?? defaultRunCommand; + const allowCommand = deps?.allowCommand ?? (process.env['VAT_LINKAUTH_ALLOW_COMMAND'] !== '0'); for (const source of sources) { + if (!allowCommand && 'command' in source) continue; const value = tryResolveSource(source, env, runCommand); if (value !== undefined && value.length > 0) return value; } diff --git a/packages/utils/test/link-auth/resolve-token.test.ts b/packages/utils/test/link-auth/resolve-token.test.ts index f07ec640..3e96dace 100644 --- a/packages/utils/test/link-auth/resolve-token.test.ts +++ b/packages/utils/test/link-auth/resolve-token.test.ts @@ -12,6 +12,7 @@ function makeDeps(overrides?: Partial): TokenResolutionDeps return { env: overrides?.env ?? {}, runCommand: overrides?.runCommand ?? (() => ({ success: false, stdout: '' })), + allowCommand: overrides?.allowCommand ?? true, }; } @@ -188,3 +189,44 @@ describe('resolveToken — no token leakage in serialized errors', () => { // graceful handling, wrap at the call site. }); }); + +describe('resolveToken — allowCommand opt-out', () => { + it('skips command sources when allowCommand is false', () => { + const runCommand = vi.fn(() => ({ success: true, stdout: 'should-not-run' })); + const deps = makeDeps({ allowCommand: false, runCommand }); + expect(resolveToken([{ command: ['gh', 'auth', 'token'] }], deps)).toBeUndefined(); + expect(runCommand).not.toHaveBeenCalled(); + }); + + it('still resolves env sources when allowCommand is false', () => { + const deps = makeDeps({ allowCommand: false, env: { MY_TOKEN: 'env-wins' } }); + expect(resolveToken([{ command: ['gh', 'auth', 'token'] }, { env: 'MY_TOKEN' }], deps)).toBe('env-wins'); + }); + + it('allows command sources when allowCommand is true (default)', () => { + const runCommand = vi.fn(() => ({ success: true, stdout: 'cmd-token' })); + const deps = makeDeps({ allowCommand: true, runCommand }); + expect(resolveToken([{ command: ['gh', 'auth', 'token'] }], deps)).toBe('cmd-token'); + expect(runCommand).toHaveBeenCalledWith(['gh', 'auth', 'token']); + }); + + it('skips all command sources in a mixed list when allowCommand is false', () => { + const runCommand = vi.fn(() => ({ success: true, stdout: 'cmd-token' })); + const deps = makeDeps({ + allowCommand: false, + runCommand, + env: { FALLBACK_TOKEN: 'fallback' }, + }); + const result = resolveToken( + [{ command: ['gh', 'auth', 'token'] }, { command: ['az', 'account', 'get-access-token'] }, { env: 'FALLBACK_TOKEN' }], + deps, + ); + expect(result).toBe('fallback'); + expect(runCommand).not.toHaveBeenCalled(); + }); + + // Note: the `VAT_LINKAUTH_ALLOW_COMMAND` env-var-at-call-time behaviour is + // covered by the system test (link-auth-token-dispatch.system.test.ts), where + // real process.env interaction is safe. Unit tests here inject `allowCommand` + // directly to avoid ambient-state pollution. +}); diff --git a/packages/utils/test/system/link-auth-token-dispatch.system.test.ts b/packages/utils/test/system/link-auth-token-dispatch.system.test.ts new file mode 100644 index 00000000..e7b0c301 --- /dev/null +++ b/packages/utils/test/system/link-auth-token-dispatch.system.test.ts @@ -0,0 +1,98 @@ +/** + * System test: token command dispatch via the real OS process layer. + * + * Exercises `resolveToken` with actual binaries — `git --version` (always + * available in CI) and `gh --version` (skipped when not installed). On Windows, + * both binaries are `.cmd` shims and the spawn path goes through `shouldUseShell` + * in `safe-exec.ts`. This test exists precisely to catch cross-platform dispatch + * regressions that unit tests (with injected `runCommand`) cannot catch. + * + * Also verifies GIT_* env scrubbing behaviorally (by inspecting what the child + * process sees) and the `VAT_LINKAUTH_ALLOW_COMMAND=0` opt-out via real + * `process.env` interaction. + */ +import { describe, it, expect } from 'vitest'; + +import { resolveToken } from '../../src/link-auth/resolve-token.js'; +import { isToolAvailable } from '../../src/safe-exec.js'; + +describe('System Test: linkAuth token command dispatch', () => { + it('resolves a token via real git binary (git --version)', () => { + // git is required in CI — if it's absent the test harness cannot even clone + // the repo, so a hard assertion is correct here. + const result = resolveToken([{ command: ['git', '--version'] }]); + expect(result).toBeDefined(); + expect(result).toMatch(/^git version /); + }); + + it.skipIf(!isToolAvailable('gh'))('resolves a token via real gh binary (gh --version)', () => { + const result = resolveToken([{ command: ['gh', '--version'] }]); + expect(result).toBeDefined(); + // gh version output: "gh version X.Y.Z (YYYY-MM-DD)\n..." + expect(result).toMatch(/^gh version /); + }); + + it('GIT_* env vars are stripped before spawning — child process cannot see them', () => { + // Whitebox check: spawn a node child that echoes GIT_DIR back via + // stdout.write (no color codes, unlike `node -p` which colorizes undefined). + // If scrubbing regressed, the child prints the poison; scrubbing means it + // prints the sentinel. This is what unblocks `gh auth token` inside a + // pre-commit hook, where git pre-sets GIT_DIR / GIT_WORK_TREE / etc. + const savedGitDir = process.env['GIT_DIR']; + process.env['GIT_DIR'] = 'POISONED_GIT_DIR_VALUE'; + try { + const result = resolveToken([ + { command: ['node', '-e', "process.stdout.write(process.env.GIT_DIR ?? 'GIT_DIR_NOT_SET')"] }, + ]); + expect(result).toBe('GIT_DIR_NOT_SET'); + } finally { + if (savedGitDir === undefined) { + delete process.env['GIT_DIR']; + } else { + process.env['GIT_DIR'] = savedGitDir; + } + } + }); + + it('VAT_LINKAUTH_ALLOW_COMMAND=0 skips command sources via real process.env', () => { + // Verifies the runtime env-var read in resolveToken. Unit tests can pin the + // `allowCommand` prop directly but cannot cover the ambient env pathway. + const saved = process.env['VAT_LINKAUTH_ALLOW_COMMAND']; + process.env['VAT_LINKAUTH_ALLOW_COMMAND'] = '0'; + try { + // git --version would resolve if allowCommand defaulted to true; with the + // opt-out set to '0' the source is skipped and resolveToken returns + // undefined (no other sources supplied). + const result = resolveToken([{ command: ['git', '--version'] }]); + expect(result).toBeUndefined(); + } finally { + if (saved === undefined) { + delete process.env['VAT_LINKAUTH_ALLOW_COMMAND']; + } else { + process.env['VAT_LINKAUTH_ALLOW_COMMAND'] = saved; + } + } + }); + + it('VAT_LINKAUTH_ALLOW_COMMAND unset (default) allows command sources', () => { + // Complement of the previous test — pins the default-allow behavior against + // real process.env, so a regression that flipped the default to opt-in + // would be caught here. + const saved = process.env['VAT_LINKAUTH_ALLOW_COMMAND']; + delete process.env['VAT_LINKAUTH_ALLOW_COMMAND']; + try { + const result = resolveToken([{ command: ['git', '--version'] }]); + expect(result).toMatch(/^git version /); + } finally { + if (saved !== undefined) { + process.env['VAT_LINKAUTH_ALLOW_COMMAND'] = saved; + } + } + }); + + it('returns undefined for env-only sources when env var is absent', () => { + // Sanity check: the env path works correctly via the real dep defaults too + const result = resolveToken([{ env: '__VAT_LINKAUTH_TEST_ABSENT__' }]); + expect(result).toBeUndefined(); + }); +}); diff --git a/packages/vat-development-agents/package.json b/packages/vat-development-agents/package.json index 214adc35..9a1ed6dd 100644 --- a/packages/vat-development-agents/package.json +++ b/packages/vat-development-agents/package.json @@ -1,6 +1,6 @@ { "name": "@vibe-agent-toolkit/vat-development-agents", - "version": "0.1.39", + "version": "0.1.40-rc.1", "description": "VAT development agents - dogfooding the vibe-agent-toolkit", "type": "module", "keywords": [ diff --git a/packages/vat-example-cat-agents/package.json b/packages/vat-example-cat-agents/package.json index 1f7acab1..559f340c 100644 --- a/packages/vat-example-cat-agents/package.json +++ b/packages/vat-example-cat-agents/package.json @@ -1,6 +1,6 @@ { "name": "@vibe-agent-toolkit/vat-example-cat-agents", - "version": "0.1.39", + "version": "0.1.40-rc.1", "description": "Example agents: 8 quirky cat agents demonstrating VAT patterns", "type": "module", "exports": { diff --git a/packages/vibe-agent-toolkit/package.json b/packages/vibe-agent-toolkit/package.json index f8555ed8..202f9c9d 100644 --- a/packages/vibe-agent-toolkit/package.json +++ b/packages/vibe-agent-toolkit/package.json @@ -1,6 +1,6 @@ { "name": "vibe-agent-toolkit", - "version": "0.1.39", + "version": "0.1.40-rc.1", "description": "Modular toolkit for building, testing, and deploying portable AI agents", "type": "module", "bin": { From 02fb01c1bbc53be11267b6e6926c9ea12cf04300 Mon Sep 17 00:00:00 2001 From: Ethan Dutton <46871249+ejdutton@users.noreply.github.com> Date: Mon, 6 Jul 2026 12:17:41 -0400 Subject: [PATCH 3/4] refactor(link-auth): apply slice-4 code review fixes (issue #113) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit No behavior change to shipping features (GIT_* scrubbing, ALLOW_COMMAND opt-out). Quality pass on tests, docs, and CHANGELOG framing after two independent agent review passes plus a duplication check. Key deltas: - System test now behaviorally verifies GIT_* scrubbing (whitebox `node -e` echo of child env) — the previous `git --version` case was a false-positive that would pass even with scrubbing removed. Confirmed by regression run. - Extend scrub test with a mixed-case var (Git_Index_File) to exercise the toUpperCase() branch on Windows. - Add positive-case env-forwarding test — guards against a future over-scrub that would silently pass the scrub tests but break PATH/HOME forwarding. - Case-insensitive GIT_* filter (toUpperCase().startsWith) for Windows defense. - Contributor doc: fix file paths (macros.yaml not provider-macros.ts, transforms.ts not transform-allowlist.ts, integration test location) and switch macro-adding example from TS to YAML to match how macros actually ship. - Delete duplicate `allowCommand: true` unit test — makeDeps defaults it, so every command-source test above already covers the allow-true path. - Rename misleading "resolves a token via real git binary" tests to "dispatches to real git binary (cross-platform smoke)" — git --version returns version text, not a token. - Move contributor-guide CHANGELOG entry from ### Added to ### Internal to match prior 0.1.39 convention for developer-facing docs. Co-Authored-By: Claude Opus 4.7 --- CHANGELOG.md | 5 +- .../contributing/vat-linkauth-contributing.md | 7 ++- packages/utils/src/link-auth/resolve-token.ts | 3 + .../test/link-auth/resolve-token.test.ts | 8 +-- .../link-auth-token-dispatch.system.test.ts | 62 +++++++++++++------ 5 files changed, 56 insertions(+), 29 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cb9d279a..a370c6ea 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,12 +10,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - **`VAT_LINKAUTH_ALLOW_COMMAND=0` opt-out for token command sources (issue #113 §6.2).** Set `VAT_LINKAUTH_ALLOW_COMMAND=0` in your environment to skip all `{ command: ... }` token sources at runtime — only `{ env: ... }` sources are tried. Useful in security-sensitive CI environments or policies that prohibit arbitrary child-process execution from the link validator. The opt-out can also be set programmatically via `allowCommand: false` in the injected `TokenResolutionDeps`. -- **Contributor guide for the linkAuth engine** at [`docs/contributing/vat-linkauth-contributing.md`](docs/contributing/vat-linkauth-contributing.md): engine vocabulary, how to add a new provider macro, token resolution mechanics, the GIT_* scrubbing rationale, testing requirements, and code style notes. ### Fixed - **`gh auth token` (and other token commands) no longer fail when `vat resources validate` runs from a git pre-commit hook (issue #113 §6.1).** Git sets `GIT_DIR`, `GIT_WORK_TREE`, `GIT_INDEX_FILE`, and related vars before invoking hooks; these poison any tool that internally shells out to git, including `gh auth token`. The default token-command runner now strips all `GIT_*` vars from the environment before spawning, so authenticated link checking works correctly in both hook and non-hook contexts. +### Internal + +- **Contributor guide for the linkAuth engine** at [`docs/contributing/vat-linkauth-contributing.md`](docs/contributing/vat-linkauth-contributing.md): engine vocabulary, how to add a new provider macro, token resolution mechanics, the GIT_* scrubbing rationale, testing requirements, and code style notes. + ## [0.1.39] - 2026-07-03 ### Added diff --git a/docs/contributing/vat-linkauth-contributing.md b/docs/contributing/vat-linkauth-contributing.md index a4949215..ed38b9de 100644 --- a/docs/contributing/vat-linkauth-contributing.md +++ b/docs/contributing/vat-linkauth-contributing.md @@ -159,9 +159,10 @@ containing `}` or other special characters does not escape the template. ### Integration tests -`packages/utils/test/integration/` — test that the engine wires together correctly with -the real Zod schema and the real macro expansions. No real network calls; inject a mock -transport. +`packages/resources/test/integration/linkauth-cross-slice.integration.test.ts` — verifies +the engine wires together with the real Zod schema, real macro expansions, and the +resources-side validator layer. No real network calls; inject a mock transport when +adding new cases. ### System tests diff --git a/packages/utils/src/link-auth/resolve-token.ts b/packages/utils/src/link-auth/resolve-token.ts index e2949e1f..c738612b 100644 --- a/packages/utils/src/link-auth/resolve-token.ts +++ b/packages/utils/src/link-auth/resolve-token.ts @@ -65,6 +65,9 @@ export const defaultRunCommand: TokenResolutionDeps['runCommand'] = (argv) => { // Case-insensitive on the key so Windows env vars (which are case-insensitive // at the OS level, though `process.env` preserves original case) can't sneak // through as e.g. `Git_Dir`. + // Case-insensitive on the key so Windows env vars (which are case-insensitive + // at the OS level, though `process.env` preserves original case) can't sneak + // through as e.g. `Git_Dir`. const env = Object.fromEntries( Object.entries(process.env).filter(([k]) => !k.toUpperCase().startsWith('GIT_')), ) as NodeJS.ProcessEnv; diff --git a/packages/utils/test/link-auth/resolve-token.test.ts b/packages/utils/test/link-auth/resolve-token.test.ts index 3e96dace..67b2d1d7 100644 --- a/packages/utils/test/link-auth/resolve-token.test.ts +++ b/packages/utils/test/link-auth/resolve-token.test.ts @@ -203,12 +203,8 @@ describe('resolveToken — allowCommand opt-out', () => { expect(resolveToken([{ command: ['gh', 'auth', 'token'] }, { env: 'MY_TOKEN' }], deps)).toBe('env-wins'); }); - it('allows command sources when allowCommand is true (default)', () => { - const runCommand = vi.fn(() => ({ success: true, stdout: 'cmd-token' })); - const deps = makeDeps({ allowCommand: true, runCommand }); - expect(resolveToken([{ command: ['gh', 'auth', 'token'] }], deps)).toBe('cmd-token'); - expect(runCommand).toHaveBeenCalledWith(['gh', 'auth', 'token']); - }); + // Note: `allowCommand: true` is `makeDeps`'s default, so every command-source + // test above already exercises the allow-true path. No dedicated test needed. it('skips all command sources in a mixed list when allowCommand is false', () => { const runCommand = vi.fn(() => ({ success: true, stdout: 'cmd-token' })); diff --git a/packages/utils/test/system/link-auth-token-dispatch.system.test.ts b/packages/utils/test/system/link-auth-token-dispatch.system.test.ts index e7b0c301..e4ccd5e9 100644 --- a/packages/utils/test/system/link-auth-token-dispatch.system.test.ts +++ b/packages/utils/test/system/link-auth-token-dispatch.system.test.ts @@ -17,40 +17,64 @@ import { resolveToken } from '../../src/link-auth/resolve-token.js'; import { isToolAvailable } from '../../src/safe-exec.js'; describe('System Test: linkAuth token command dispatch', () => { - it('resolves a token via real git binary (git --version)', () => { - // git is required in CI — if it's absent the test harness cannot even clone - // the repo, so a hard assertion is correct here. + it('dispatches to real git binary and returns trimmed stdout (cross-platform smoke)', () => { + // `git --version` returns a version string, not a token, but it exercises + // the full spawn → trim → return path against a real binary. On Windows, + // `git` is a `.cmd` shim so this covers the `shouldUseShell` path in + // safe-exec.ts. git is required in CI (harness cloning depends on it), so + // a hard assertion is correct here. const result = resolveToken([{ command: ['git', '--version'] }]); expect(result).toBeDefined(); expect(result).toMatch(/^git version /); }); - it.skipIf(!isToolAvailable('gh'))('resolves a token via real gh binary (gh --version)', () => { + it.skipIf(!isToolAvailable('gh'))('dispatches to real gh binary and returns trimmed stdout (cross-platform smoke)', () => { const result = resolveToken([{ command: ['gh', '--version'] }]); expect(result).toBeDefined(); // gh version output: "gh version X.Y.Z (YYYY-MM-DD)\n..." expect(result).toMatch(/^gh version /); }); - it('GIT_* env vars are stripped before spawning — child process cannot see them', () => { - // Whitebox check: spawn a node child that echoes GIT_DIR back via - // stdout.write (no color codes, unlike `node -p` which colorizes undefined). - // If scrubbing regressed, the child prints the poison; scrubbing means it - // prints the sentinel. This is what unblocks `gh auth token` inside a - // pre-commit hook, where git pre-sets GIT_DIR / GIT_WORK_TREE / etc. + it('GIT_* env vars are stripped before spawning — child process cannot see them (case-insensitive)', () => { + // Whitebox check: spawn a node child that echoes both an uppercase and a + // mixed-case GIT_* var back via stdout.write. If scrubbing regressed, the + // child prints the poison; scrubbing means it prints the sentinels. Two + // vars in one test to exercise both the canonical `GIT_DIR` and the + // case-insensitive branch (`Git_Index_File` — matters on Windows where env + // names are case-insensitive at the OS level). const savedGitDir = process.env['GIT_DIR']; + const savedMixed = process.env['Git_Index_File']; process.env['GIT_DIR'] = 'POISONED_GIT_DIR_VALUE'; + process.env['Git_Index_File'] = 'POISONED_MIXED_CASE_VALUE'; try { - const result = resolveToken([ - { command: ['node', '-e', "process.stdout.write(process.env.GIT_DIR ?? 'GIT_DIR_NOT_SET')"] }, - ]); - expect(result).toBe('GIT_DIR_NOT_SET'); + const script = + "process.stdout.write((process.env.GIT_DIR ?? 'UNSET') + '|' + (process.env.Git_Index_File ?? 'UNSET'))"; + const result = resolveToken([{ command: ['node', '-e', script] }]); + expect(result).toBe('UNSET|UNSET'); } finally { - if (savedGitDir === undefined) { - delete process.env['GIT_DIR']; - } else { - process.env['GIT_DIR'] = savedGitDir; - } + if (savedGitDir === undefined) delete process.env['GIT_DIR']; + else process.env['GIT_DIR'] = savedGitDir; + if (savedMixed === undefined) delete process.env['Git_Index_File']; + else process.env['Git_Index_File'] = savedMixed; + } + }); + + it('non-GIT_* env vars ARE forwarded to the child — scrub is scoped, not indiscriminate', () => { + // Positive-case complement of the scrub test: if a future regression + // over-scrubbed (e.g. stripped everything, or every var starting with any + // uppercase letter), the previous test would still pass but real-world + // tools that need PATH / HOME / auth-provider vars would break silently. + // Assert a synthetic non-GIT_* var reaches the child. + const sentinel = '__VAT_LINKAUTH_ENV_FORWARD_PROBE__'; + const savedProbe = process.env[sentinel]; + process.env[sentinel] = 'reached-child'; + try { + const script = `process.stdout.write(process.env['${sentinel}'] ?? 'MISSING')`; + const result = resolveToken([{ command: ['node', '-e', script] }]); + expect(result).toBe('reached-child'); + } finally { + if (savedProbe === undefined) delete process.env[sentinel]; + else process.env[sentinel] = savedProbe; } }); From 74ea7e198f9d12d9986b3a2151c538caca4750b4 Mon Sep 17 00:00:00 2001 From: Ethan Dutton <46871249+ejdutton@users.noreply.github.com> Date: Mon, 6 Jul 2026 13:34:54 -0400 Subject: [PATCH 4/4] refactor(link-auth): extract scrubGitEnv, close patch-coverage gap on defaultRunCommand MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extracts the case-insensitive GIT_* env filter from defaultRunCommand into an exported pure helper (`scrubGitEnv`). Adds: - Unit tests for scrubGitEnv (5 cases: uppercase strip, mixed-case strip for Windows defense, non-GIT preservation, substring-not-matched guard, empty input) - Unit tests for defaultRunCommand itself via vi.mock on safe-exec (empty-argv short-circuit, main spawn path with env-scrub assertion, Buffer→string stdout coercion) Also removes an accumulated-edit duplicate comment on the case-insensitive filter (was written twice due to earlier edit history). Codecov context: the prior slice-4 patch was flagged at 33% coverage because defaultRunCommand's env-scrub lines were only exercised by system tests (real spawn), which codecov doesn't measure. Extracting the pure helper lets the filter be tested directly; vi.mock covers the remaining spawn composition. No behavior change to shipping features. Co-Authored-By: Claude Opus 4.7 --- packages/utils/src/link-auth/resolve-token.ts | 37 ++++--- .../test/link-auth/resolve-token.test.ts | 101 +++++++++++++++++- 2 files changed, 122 insertions(+), 16 deletions(-) diff --git a/packages/utils/src/link-auth/resolve-token.ts b/packages/utils/src/link-auth/resolve-token.ts index c738612b..14c39392 100644 --- a/packages/utils/src/link-auth/resolve-token.ts +++ b/packages/utils/src/link-auth/resolve-token.ts @@ -49,29 +49,36 @@ export interface TokenResolutionDeps { readonly allowCommand: boolean; } +/** + * Return a copy of the given env with all `GIT_*` keys removed. Case-insensitive + * on the key so Windows env vars (which are case-insensitive at the OS level, + * though `process.env` preserves original case) can't sneak through as e.g. + * `Git_Dir`. + * + * Exported for unit testing and for callers assembling their own `runCommand` + * who want the exact same scrub `defaultRunCommand` applies. + * + * Rationale: `vat resources validate` is often invoked from git pre-commit + * hooks, which pre-set `GIT_DIR` / `GIT_WORK_TREE` / `GIT_INDEX_FILE`. These + * poison any nested tool that shells out to git, notably `gh auth token`. + */ +export function scrubGitEnv(source: NodeJS.ProcessEnv): NodeJS.ProcessEnv { + return Object.fromEntries( + Object.entries(source).filter(([k]) => !k.toUpperCase().startsWith('GIT_')), + ) as NodeJS.ProcessEnv; +} + /** * 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). - * - * Strips `GIT_*` env vars before spawning so token commands like `gh auth token` - * work correctly from inside git pre-commit hooks, where git sets `GIT_DIR`, - * `GIT_WORK_TREE`, and similar vars that confuse nested git calls. + * Forwards to `safeExecResult` (no shell, argv-based), with `GIT_*` vars + * stripped from the child env — see {@link scrubGitEnv}. */ 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: '' }; - // Case-insensitive on the key so Windows env vars (which are case-insensitive - // at the OS level, though `process.env` preserves original case) can't sneak - // through as e.g. `Git_Dir`. - // Case-insensitive on the key so Windows env vars (which are case-insensitive - // at the OS level, though `process.env` preserves original case) can't sneak - // through as e.g. `Git_Dir`. - const env = Object.fromEntries( - Object.entries(process.env).filter(([k]) => !k.toUpperCase().startsWith('GIT_')), - ) as NodeJS.ProcessEnv; - const result = safeExecResult(bin, [...args], { encoding: 'utf8', env }); + const result = safeExecResult(bin, [...args], { encoding: 'utf8', env: scrubGitEnv(process.env) }); const stdout = typeof result.stdout === 'string' ? result.stdout : result.stdout.toString('utf8'); return { success: result.success, stdout }; }; diff --git a/packages/utils/test/link-auth/resolve-token.test.ts b/packages/utils/test/link-auth/resolve-token.test.ts index 67b2d1d7..51d52699 100644 --- a/packages/utils/test/link-auth/resolve-token.test.ts +++ b/packages/utils/test/link-auth/resolve-token.test.ts @@ -1,9 +1,22 @@ -import { describe, expect, it, vi } from 'vitest'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; import { + defaultRunCommand, resolveToken, + scrubGitEnv, type TokenResolutionDeps, } from '../../src/link-auth/resolve-token.js'; +import { safeExecResult } from '../../src/safe-exec.js'; + +// Mock safeExecResult so we can unit-test defaultRunCommand without spawning. +// Existing tests below inject their own `runCommand` via makeDeps, so they never +// reach the mocked module — this mock only affects the `defaultRunCommand` +// describe block at the bottom. +vi.mock('../../src/safe-exec.js', () => ({ + safeExecResult: vi.fn(), +})); + +const mockedSafeExecResult = vi.mocked(safeExecResult); const FALLBACK_ENV = 'FALLBACK'; const FALLBACK_VALUE = 'fallback-value'; @@ -226,3 +239,89 @@ describe('resolveToken — allowCommand opt-out', () => { // real process.env interaction is safe. Unit tests here inject `allowCommand` // directly to avoid ambient-state pollution. }); + +describe('scrubGitEnv', () => { + it('strips uppercase GIT_* keys', () => { + expect(scrubGitEnv({ GIT_DIR: 'x', PATH: '/bin' })).toEqual({ PATH: '/bin' }); + }); + + it('strips mixed-case GIT_* keys (Windows case-insensitivity defense)', () => { + // Windows treats env-var names case-insensitively at the OS level. Even + // though Node's `process.env` preserves the original case, a shell or user + // could set `Git_Dir` and reasonably expect it to behave the same as + // `GIT_DIR`. Scrub must match all case variants. + expect(scrubGitEnv({ Git_Dir: 'x', git_index_file: 'y', HOME: '/h' })).toEqual({ HOME: '/h' }); + }); + + it('preserves non-GIT vars', () => { + expect(scrubGitEnv({ PATH: '/bin', NODE_ENV: 'test', HOME: '/h' })).toEqual({ + PATH: '/bin', + NODE_ENV: 'test', + HOME: '/h', + }); + }); + + it('only strips keys that START with GIT_ — MY_GIT_TOKEN survives', () => { + // Guards against sweeping too broadly. A user var like MY_GIT_TOKEN must + // NOT be scrubbed just because "GIT_" appears in the middle of its name. + expect(scrubGitEnv({ MY_GIT_TOKEN: 'keep', GIT_TOKEN: 'strip' })).toEqual({ + MY_GIT_TOKEN: 'keep', + }); + }); + + it('returns an empty object for empty input', () => { + expect(scrubGitEnv({})).toEqual({}); + }); +}); + +describe('defaultRunCommand', () => { + beforeEach(() => { + mockedSafeExecResult.mockReset(); + }); + + it('returns { success: false, stdout: "" } for empty argv (no spawn attempted)', () => { + const result = defaultRunCommand([]); + expect(result).toEqual({ success: false, stdout: '' }); + expect(mockedSafeExecResult).not.toHaveBeenCalled(); + }); + + it('spawns via safeExecResult, forwards GIT_*-scrubbed env, and returns spawn result', () => { + mockedSafeExecResult.mockReturnValue({ + success: true, + stdout: 'output-value', + } as unknown as ReturnType); + + process.env['__TEST_FORWARD_VAR__'] = 'reached'; + process.env['GIT_DIR'] = 'should-be-stripped'; + try { + const result = defaultRunCommand(['echo', 'x', 'y']); + expect(result).toEqual({ success: true, stdout: 'output-value' }); + + expect(mockedSafeExecResult).toHaveBeenCalledTimes(1); + const [bin, args, opts] = mockedSafeExecResult.mock.calls[0] as [ + string, + string[], + { env?: NodeJS.ProcessEnv; encoding?: string }, + ]; + expect(bin).toBe('echo'); + expect(args).toEqual(['x', 'y']); + expect(opts.encoding).toBe('utf8'); + expect(opts.env?.['__TEST_FORWARD_VAR__']).toBe('reached'); + expect(opts.env?.['GIT_DIR']).toBeUndefined(); + } finally { + delete process.env['__TEST_FORWARD_VAR__']; + delete process.env['GIT_DIR']; + } + }); + + it('coerces Buffer stdout from safeExecResult to a utf8 string', () => { + // When encoding isn't set to 'utf8' by the caller, safeExecResult may + // return a Buffer. defaultRunCommand normalizes to string. + mockedSafeExecResult.mockReturnValue({ + success: true, + stdout: Buffer.from('buffered-value\n'), + } as unknown as ReturnType); + const result = defaultRunCommand(['some-bin']); + expect(result.stdout).toBe('buffered-value\n'); + }); +});