exposure: git credentials in error detail, prod-honored test env overrides, unpinned CI installs #118

New issue

Closed

opened 2026-08-13 15:44:11 +00:00 by john · 1 comment

john commented

2026-08-13 15:44:11 +00:00

Owner

Three exposure surfaces, none of them a bypass, all of them worth closing.

1. Git credentials can reach the transcript

git-publish.mjs:161-163, same shape at :209 and planning.mjs:1917-1918:

} catch (e) {
  emit({ ok: false, reason: 'push-failed', detail: e && e.message ? e.message : String(e), warnings });
}

stdio: ['ignore','pipe','pipe'] puts git's stderr into e.message, and git prints the full remote URL on an auth or HTTP failure. A remote configured https://x-access-token:ghp_...@github.com/o/r.git puts the PAT on stdout, into the model's context, and into the transcript permanently. A leaked token forces a rotation.

Fix: strip ://<userinfo>@ from any git error text before it reaches the envelope. One helper, used at all three sites.

2. "Test-only" env overrides are honored in production

Each carries a comment saying hermetic test injection only. Nothing gates any of them:

route.mjs:70 CADENCE_ROUTE_TABLE - sets every review trigger's gate
config-merge.mjs:26 CADENCE_GLOBAL_CONFIG - supplies an entire config layer
config.mjs:36 CADENCE_CONFIG_SCHEMA
planning.mjs:128 CADENCE_PLUGIN_MANIFEST
worktree-base.mjs:56,65, self-verify.mjs:147

This contradicts the best-reasoned security comment in the repo. review-provider.mjs:445-460 refuses an env transport override precisely because env is attacker-settable, naming ".envrc in a cloned repo, a devcontainer env block". That reasoning applies here unchanged and was not applied.

Not theoretical: CADENCE_GLOBAL_CONFIG was the lever used to demonstrate #114 on a machine whose real global config would otherwise have shadowed it.

Fix: gate on a test sentinel, or document them as supported overrides and stop calling them test-only. Either is fine; the gap between the comment and the behaviour is not.

3. CI installs unpinned packages with lifecycle scripts enabled

.github/workflows/test.yml:

- run: npm install --no-save --no-package-lock typescript @types/node

Unpinned, no integrity check, postinstall enabled, on pull_request. Blast radius is genuinely small - permissions: contents: read, no secrets in the job, not pull_request_target - which is why this is INFO rather than a finding. Still: pin both versions and add --ignore-scripts.

Three exposure surfaces, none of them a bypass, all of them worth closing. ## 1. Git credentials can reach the transcript `git-publish.mjs:161-163`, same shape at `:209` and `planning.mjs:1917-1918`: ```js } catch (e) { emit({ ok: false, reason: 'push-failed', detail: e && e.message ? e.message : String(e), warnings }); } ``` `stdio: ['ignore','pipe','pipe']` puts git's stderr into `e.message`, and git prints the full remote URL on an auth or HTTP failure. A remote configured `https://x-access-token:ghp_...@github.com/o/r.git` puts the PAT on stdout, into the model's context, and into the transcript permanently. A leaked token forces a rotation. Fix: strip `://<userinfo>@` from any git error text before it reaches the envelope. One helper, used at all three sites. ## 2. "Test-only" env overrides are honored in production Each carries a comment saying hermetic test injection only. Nothing gates any of them: - `route.mjs:70` `CADENCE_ROUTE_TABLE` - sets every review trigger's gate - `config-merge.mjs:26` `CADENCE_GLOBAL_CONFIG` - supplies an entire config layer - `config.mjs:36` `CADENCE_CONFIG_SCHEMA` - `planning.mjs:128` `CADENCE_PLUGIN_MANIFEST` - `worktree-base.mjs:56,65`, `self-verify.mjs:147` This contradicts the best-reasoned security comment in the repo. `review-provider.mjs:445-460` refuses an env transport override precisely because env is attacker-settable, naming ".envrc in a cloned repo, a devcontainer env block". That reasoning applies here unchanged and was not applied. Not theoretical: `CADENCE_GLOBAL_CONFIG` was the lever used to demonstrate #114 on a machine whose real global config would otherwise have shadowed it. Fix: gate on a test sentinel, or document them as supported overrides and stop calling them test-only. Either is fine; the gap between the comment and the behaviour is not. ## 3. CI installs unpinned packages with lifecycle scripts enabled `.github/workflows/test.yml`: ```yaml - run: npm install --no-save --no-package-lock typescript @types/node ``` Unpinned, no integrity check, postinstall enabled, on `pull_request`. Blast radius is genuinely small - `permissions: contents: read`, no secrets in the job, not `pull_request_target` - which is why this is INFO rather than a finding. Still: pin both versions and add `--ignore-scripts`.

john added this to the v3.2.0 milestone

2026-08-13 15:44:11 +00:00

john added the

security

label

2026-08-13 15:44:11 +00:00

john closed this issue

2026-08-14 17:01:43 +00:00

john commented

2026-08-14 17:02:01 +00:00

Author

Owner

Verified through the phase UAT walk. The milestone audit traced 12/12 requirements with 0 broken and 36/36 acceptance criteria covered.

Fixed in v3.2.0 (PR #124). EXP-01, phase 2. redactUrl() strips userinfo from git failure detail at all four emit sites, each with its own test. The environment overrides commented "test injection only" are gated behind CADENCE_TEST_SEAM=1. Verified through the phase UAT walk. The milestone audit traced 12/12 requirements with 0 broken and 36/36 acceptance criteria covered.