feat(link-auth): slice 4 — GIT_* scrubbing, ALLOW_COMMAND opt-out, system test, contributor docs (issue #113)

[ejdutton]

Summary

• GIT_ env scrubbing in defaultRunCommand*: strips all GIT_DIR, GIT_WORK_TREE, and related vars before spawning token commands, so gh auth token works correctly from inside git pre-commit hooks (where git pre-sets these vars and confuses nested git calls)
• VAT_LINKAUTH_ALLOW_COMMAND=0 opt-out: new allowCommand field in TokenResolutionDeps; defaults to process.env['VAT_LINKAUTH_ALLOW_COMMAND'] !== '0'; command sources are silently skipped when false — escape hatch for security-sensitive CI environments
• Cross-platform system test at packages/utils/test/system/link-auth-token-dispatch.system.test.ts: exercises real git and gh binaries through resolveToken with no injected deps; includes a whitebox GIT_DIR scrubbing check and a VAT_LINKAUTH_ALLOW_COMMAND=0 real-process-env check
• Contributor guide at docs/contributing/vat-linkauth-contributing.md: engine vocabulary, how to add a new provider macro, token resolution mechanics, GIT_* scrubbing rationale, testing requirements, code style notes
• Version bumped to 0.1.40-rc.1

Test plan

• bun run test:unit — 26 unit tests for resolve-token.ts (6 new for allowCommand)
• bun run test:system — 6 new system tests in link-auth-token-dispatch.system.test.ts (all green locally, including GIT_DIR scrubbing whitebox check and VAT_LINKAUTH_ALLOW_COMMAND=0 real-env check)
• CI matrix: Ubuntu + Windows — the Windows path exercises .cmd shim dispatch through shouldUseShell in safe-exec.ts
• Manual: run vat resources validate inside a git pre-commit hook on a repo with resources.linkAuth.providers[*].use: github configured; gh auth token should resolve correctly

Closes #113 (slice 4 of 4).

🤖 Generated with Claude Code

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.84%. Comparing base (f70d472) to head (74ea7e1).

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #138      +/-   ##
==========================================
+ Coverage   83.80%   83.84%   +0.03%     
==========================================
  Files         272      272              
  Lines       21198    21205       +7     
  Branches     4393     4400       +7     
==========================================
+ Hits        17765    17779      +14     
+ Misses       3433     3426       -7     

Files with missing lines	Coverage Δ
packages/utils/src/link-auth/resolve-token.ts	100.00% <100.00%> (+17.07%)	⬆️

Files with missing lines	Coverage Δ
packages/utils/src/link-auth/resolve-token.ts	100.00% <100.00%> (+17.07%)	⬆️

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[jdutton]

Independent review follow-up (post-merge)

Reviewed this PR in an isolated worktree after merge, plus a broader look at the linkAuth workstream (#113 slices 1–4) prompted by a downstream bundler report about macros.yaml becoming a dangling asset reference when @vibe-agent-toolkit/utils is wildcard-re-exported and bundled. None of this blocks anything already shipped — filing as concrete follow-up items.

1. [Bug, Med] allowCommand opt-out reads ambient process.env instead of the resolved deps.env

packages/utils/src/link-auth/resolve-token.ts:97-99:

const env = deps?.env ?? process.env;
const runCommand = deps?.runCommand ?? defaultRunCommand;
const allowCommand = deps?.allowCommand ?? (process.env['VAT_LINKAUTH_ALLOW_COMMAND'] !== '0');

Every other injectable input in this function goes through the resolved env local — but the allowCommand default reaches around it to process.env directly. A caller that supplies a curated deps.env (e.g. a sandboxed/scrubbed env map for testing or embedding) and sets VAT_LINKAUTH_ALLOW_COMMAND=0 in that map gets silently ignored — command sources still run, because the check consults real ambient process.env instead. For a control whose whole purpose is "environments where arbitrary command execution is undesirable," failing open here is the wrong direction.

Telling sign in the tests: the unit tests can only reach this flag via deps.allowCommand directly (resolve-token.test.ts); the env-var pathway is only exercised by the system test poking real process.env (link-auth-token-dispatch.system.test.ts) — because there's no way to unit-test "does it respect deps.env" when it doesn't.

Fix: read the flag from the resolved env, not process.env:

const allowCommand = deps?.allowCommand ?? (env['VAT_LINKAUTH_ALLOW_COMMAND'] !== '0');

2. [Packaging, Low] utils package doesn't declare sideEffects: false

packages/utils/package.json has no sideEffects field. Bundlers that consume this package must conservatively assume every module has side effects and can't tree-shake unused ones — including the linkAuth/macro-expansion module, which is dead weight for the vast majority of consumers who only need e.g. path/fs/process helpers.

I checked: no module under packages/utils/src has a real import-time side effect (no top-level I/O, no global mutation, no side-effect-only imports). It should be safe to add:

"sideEffects": false

This is also the direct fix for the downstream report: macros.yaml is loaded via new URL('./macros.yaml', import.meta.url) in expand-macro.ts:22 — a pattern bundlers recognize and bake into their output as a sibling-asset dependency. Combined with no sideEffects flag, a bundler-based consumer that re-exports the whole utils barrel (even via a wildcard re-export it doesn't control, e.g. export * from '@vibe-agent-toolkit/utils') can't drop the unused macro-expansion module, so it ends up shipping a bundled reference to a macros.yaml sibling file that was never carried into the bundle. Not a runtime bug today (the file read is lazy, and it only fires if expandMacro is actually called) — but it does produce exactly this kind of dangling-reference finding in tooling that statically audits bundle output. sideEffects: false lets a tree-shaking bundler drop the whole module (and its asset reference) for any consumer that never calls expandMacro.

3. [Hygiene, Low] Move the macrosPath computation inside getMacros()

packages/utils/src/link-auth/expand-macro.ts:22:

const macrosPath = fileURLToPath(new URL('./macros.yaml', import.meta.url));

This runs at module top level — i.e. merely importing expand-macro.ts computes the asset URL, even if expandMacro() is never called. Purely cosmetic today (no I/O happens until getMacros() reads the file), but moving this line inside getMacros() makes the module's "side-effect-free at import time" property literally true rather than true-in-practice, which reinforces item #2 and gives bundler static analysis one less top-level new URL(...) reference to trip over.

4. [Enhancement] Add ./fs and ./process subpath exports to @vibe-agent-toolkit/utils

Cost is low given moduleResolution: NodeNext is already configured — this doesn't need a build-system change, just:

• two new barrel files (e.g. src/fs.ts, src/process.ts) that re-export a curated subset — tsc --build compiles them into dist/fs.js / dist/process.js automatically
• two new entries in the exports map in packages/utils/package.json:

"exports": {
  ".": { "types": "./dist/index.d.ts", "import": "./dist/index.js" },
  "./fs": { "types": "./dist/fs.d.ts", "import": "./dist/fs.js" },
  "./process": { "types": "./dist/process.d.ts", "import": "./dist/process.js" }
}

Fully additive — the existing "." barrel is untouched, nothing internal needs to migrate.

Recommended contents, based on what our own packages/dev-tools actually imports from utils today (a reasonable proxy for what a "give me cross-platform primitives" consumer needs):

• @vibe-agent-toolkit/utils/process — safeExecSync, safeExecResult, isToolAvailable, CommandExecutionError
• @vibe-agent-toolkit/utils/fs — safePath, toForwardSlash, mkdirSyncReal, normalizedTmpdir, and the rest of path-utils/fs-utils

This gives narrow entry points that a consumer can import (or re-export) without pulling in linkAuth, git, glob, template, or skill-testing machinery. Leave other subpaths (git-, skill-, linkAuth, etc.) alone for now — add on demand when a real consumer needs one; no need to carve up the whole barrel speculatively.

Note: discussed relocating the linkAuth engine + macros.yaml out of utils entirely (its only current consumer is packages/resources) as an alternative/complementary fix. Decided to leave it in utils for now — deferring, not rejecting; may be useful to expose from utils to another consumer later. If subpath exports + sideEffects: false don't fully resolve bundler friction in practice, revisit the relocation.

5. [Nit] Stale package description

packages/utils/package.json: "description": "Core utility functions with no external dependencies" — false; the package has carried npm dependencies (handlebars, ignore, picomatch, which, yaml) since before this PR. Worth fixing next time this file is touched.

6. [Nit] No test asserting GITHUB_TOKEN/GH_TOKEN survive the GIT_* scrub

resolve-token.test.ts / scrubGitEnv tests cover MY_GIT_TOKEN surviving (good — guards against over-matching on substring), but nothing explicitly asserts that GITHUB_TOKEN and GH_TOKEN — the exact vars gh auth token needs to function — survive the scrub. The logic is already correct (neither starts with GIT_), but an explicit assertion would document the intent and guard against a future refactor of the prefix-match logic.

---

Everything else in this PR looked solid: the Object.hasOwn __proto__ defense, no-shell argv handling, per-OS-user cache isolation, and the test suite in general — both scrub directions are verified behaviorally (spawning a real child process and inspecting what it sees), the Windows .cmd/shouldUseShell path is explicitly exercised, and the case-insensitive scrub rationale is well documented. Nice work — the above are follow-ups, not blockers.