[kyle-sexton]

Closes #760

Summary

pages-publish now reads its config only from the Key Vault secret pages-publish-config. The ~/.config/pages-publish/config file override is gone. That file could name any vault secret and any https endpoint, and Bash(pages-publish *) is pre-approved, so any process able to write it could send any vault secret to any host with no prompt. Key Vault writes are permission-gated and audited (kv-audit diagnostic logs). A local operator changes the config with az keyvault secret set on pages-publish-config. The ready review found a second config-forging path, a json.py planted in the working directory and picked up by vault-exec's python3 -c; it is closed here too.

Fix

• components/cloud-environment/pages-publish: load_config reads only Key Vault (still env -u VAULT_EXEC_VAULT -u PP_CONFIG, the empty-value check and parse_config). The passwd-home, getent, stat and owner/mode logic is removed, which also drops the macOS getent limitation. A missing vault-exec or a failed read exits 5. The header now says why config comes only from Key Vault, and no longer claims run-time variables are ignored: it names PATH, TMPDIR and BASH_ENV, and states the guarantee as "no config-file write changes where secrets go" (the two lower-severity notes in fix(cloud-environment): pages-publish config override can send any vault secret to any host #760).
• components/cloud-environment/vault-exec: the JSON parser runs as python3 -I -c, so neither the working directory nor PYTHON* variables reach its import path. Without it, a planted json.py could return any config value (attacker endpoint, any token secret name) and pages-publish would send that secret with no prompt.
• components/cloud-environment/setup.sh: removed the writer that built the config file from PAGES_PUBLISH_* variables, and the writer that wrote ~/.claude/rendered-views.md from RENDERED_VIEWS_MD. Both were dead code: claude.ai passes an environment's variables to the session, not to the setup script, and the rendered-views-sync SessionStart hook already supplies the rendered-views preference. The rendered-views-sync install and hook registration are unchanged.
• components/cloud-environment/README.md: config, exit-code, trusted-inputs and setup sections updated to match.

Verification

• New regression case in pages-publish.test.sh: a valid 0600 config naming a decoy endpoint and token secret, placed in the passwd home (stub getent), HOME and XDG_CONFIG_HOME, is ignored, and the upload uses the Key Vault endpoint and token. Run against the pre-fix script, it fails (curl got https://decoy.example.test/_upload and Bearer SENTINEL-decoy-token). The config-validation cases (http endpoint, missing key, unknown key, bad secret name, non-KEY=VALUE line, empty secret) now run against the Key Vault value.
• New case in vault-exec.test.sh: a json.py returning FORGED, planted in the working directory and on PYTHONPATH, is ignored. Run against the pre-fix vault-exec, it fails (got FORGED).
• bash harness/shell/run-tests.sh components/cloud-environment/pages-publish.test.sh components/cloud-environment/setup.test.sh components/cloud-environment/rendered-views-sync.test.sh components/cloud-environment/vault-exec.test.sh: 4 passed.
• shellcheck -x, shfmt -d, markdownlint-cli2, typos, and the lefthook pre-commit lanes (editorconfig, typos, markdownlint, gitleaks, shellcheck): clean.
• Fresh-context security review of the PR diff: findings above fixed. Not changed: a host set up before this PR keeps its old ~/.config/pages-publish/config, which nothing reads now (the decoy test proves it is ignored).

Related

• standards#759 (local install via the sync manifest) waits on this.
• standards#756 introduced the file override.
• fix(review): publish-hosted looks up repository visibility itself claude-code-plugins#6783.

🤖 Generated with Claude Code

https://claude.ai/code/session_01EQPrCTBQPb5b1XtbAH7tMA

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-10T21:31:10.783443Z	66190c2	Draft marked ready

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[claude]

Claude finished @kyle-sexton's task in 3m 51s —— View job

---

Code review (code-review lane)

• Gather context (diff, CLAUDE.md, REVIEW.md)
• Review changed files for correctness/maintainability
• Adversarially verify candidate findings
• Post findings

Scope note: this repo runs a dedicated security lane (pr-review-security.yml), so per REVIEW.md this lane omits security findings (vulnerabilities, trust-boundary, credential-exposure) and covers correctness, design, conventions, error handling, and tests only. The security substance of this fix (Key Vault-only config, python3 -I hardening) is that lane's to assess.

Findings: none. The removal of the ~/.config/pages-publish/config file-override path (load_config in pages-publish) and the dead write_pages_publish_config/pages_publish_config_path/write_rendered_views helpers in setup.sh leaves no stale references — I checked repo-wide for PAGES_PUBLISH_KEYS, pages_publish_config_path, write_pages_publish_config, write_rendered_views, RENDERED_VIEWS_MD, and getent/"passwd home" usage, and every remaining hit is either the still-used PAGES_PUBLISH_MARKER/PAGES_PUBLISH_URL or intentional test assertions that a run-time override is now ignored.

• components/cloud-environment/README.md tracks the code: no leftover mention of the file override, the macOS getent limitation, or the removed setup-time env vars.
• setup.test.sh's removed assertions line up exactly with the removed setup.sh code (operator-file ordering checks, the six PAGES_PUBLISH_* write/partial-set/line-break cases, RENDERED_VIEWS_MD).
• pages-publish.test.sh's rewritten config section plausibly covers the new contract: a valid 0600 decoy file in the passwd home and HOME/XDG_CONFIG_HOME is ignored, the Key Vault value is used instead, and the prior config-validation cases (bad mode/endpoint/key/secret-name, symlink, empty, missing vault-exec) are re-pointed at the Key Vault path. The new vault-exec.test.sh case plants a json.py on PYTHONPATH/cwd and asserts the real secret value still comes back, matching the python3 -I change.
• No dangling reference to the removed stat/getent owner-mode logic remains in pages-publish's header or exit-code table.

One non-blocking observation, not raised as a finding because it sits on a line this PR didn't touch: the header comment in pages-publish.test.sh ("Tests pages-publish against stubbed getent...") is now slightly stale since pages-publish no longer calls getent at all — the stub is only there to place a decoy file at a path the old code used to read. Pre-existing wording, harmless, so leaving it to the author's discretion rather than blocking on it.

Verification note: I was not able to execute harness/shell/run-tests.sh or the component test suites myself in this sandbox (the tool denied running those commands), so the "4 passed" / shellcheck / shfmt / markdownlint results in the PR description are author-claimed and unverified by me — I reviewed the test-file diffs statically instead of re-running them.

[claude]

Claude finished @kyle-sexton's task in 2m 22s —— View job

---

Security review in progress

• Gather context (PR diff, changed files)
• Read changed files (pages-publish, vault-exec, setup.sh, tests, README)
• Check instruction-surface deletions against protected classes — no standing instruction (CLAUDE.md/AGENTS.md/rules/hook allow/deny lists) is touched by this diff; claude-permissions.json's Bash(pages-publish *) pre-approval is unchanged, not deleted
• Hunt for security issues (fan out + adversarial verification) — two hunters dispatched
• Report findings

[github-actions]

claude-security-review has reviewed this pull request through 6e1552e; a later push is reviewed from there.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 66190c268d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[github-actions]

claude-review has reviewed this pull request through 6e1552e; a later push is reviewed from there.

[claude]

Claude finished @kyle-sexton's task in 2m 57s —— View job

---

Security review (incremental)

• Gather context — incremental diff scope: components/cloud-environment/README.md, setup.sh, setup.test.sh, changed since 66190c268da3cabdad9a08af3e022718a470ca48 (read from .claude-lane/incremental.diff)
• Invoke /review:security-review and follow its procedure (instruction-surface check, hunt, adversarial verification)
• Report findings

Findings: none.

This commit (6e1552e) is a follow-up addressing two Codex correctness comments on the prior commit, not new security-sensitive logic:

• setup.sh: SCRIPT_VERSION bumped to 2026-10-10.2, and a new log_retired_inputs function logs one WARN <name> is set but ignored line (naming the replacement, never the value) for RENDERED_VIEWS_MD and each PAGES_PUBLISH_* variable that's still set, called right after the start version= log line (setup.sh#L263-L277).
• README.md gets a "Retired setup inputs" section documenting this.
• setup.test.sh gets matching coverage, including an explicit assertion that the log never echoes a retired value.

Checked specifically for the things that would matter here:

• No secret leakage. log_retired_inputs uses ${RENDERED_VIEWS_MD+set} / ${!var+set} — bash's "is this set" test, which substitutes the literal word set and never reads the variable's actual contents. Every log line is a static string naming the variable, never ${!var}. setup.test.sh's own assert_not_contains … 'secret-value' case (setting PAGES_PUBLISH_PRIVATE_ENDPOINT='pp-secret-value') exercises this directly.
• No set -u edge case. The +set expansion form is exactly the idiomatic unbound-safe presence check; var is always assigned before indirection, so no attacker-controlled environment can trigger a crash or value leak through it.
• No injection surface. The six PAGES_PUBLISH_* suffixes iterated in the loop are a fixed, hardcoded list — not derived from environment content — so there's no way to expand the loop or inject into the log call via a crafted variable name.
• Instruction-surface lens. This diff only adds documentation and a warning log; it doesn't delete, narrow, or soften any standing instruction, so the protected-class check doesn't apply here (the register file itself isn't present in this checkout to consult, but that's moot since nothing is being removed).
• Restoring the old write_pages_publish_config/write_rendered_views behavior (what the Codex P1 comment suggested) would have reopened the config-file-override hole this PR's prior commit closed; logging a warning instead, as done here, correctly avoids that regression.

A review:lane-verifier hunter independently reviewed the same diff and reached the same conclusion.

[claude]

Claude finished @kyle-sexton's task in 1m 29s —— View job

---

Code review (code-review lane)

• Gather context (incremental diff, CLAUDE.md/AGENTS.md, REVIEW.md)
• Review changed files for correctness/maintainability
• Adversarially verify candidate findings
• Post findings

Scope: per this lane's incremental instructions, only the diff since 66190c2 in components/cloud-environment/{README.md,setup.sh,setup.test.sh} (.claude-lane/incremental.diff) — this is the fixup for the two Codex findings on the prior commit (silently-dropped retired inputs, stale SCRIPT_VERSION). Everything else in the PR was already reviewed at 66190c2.

Findings: none.

• SCRIPT_VERSION bump (2026-10-10.1 → .2) directly resolves Codex's P2.
• log_retired_inputs resolves Codex's P1 by making the now-ignored RENDERED_VIEWS_MD/PAGES_PUBLISH_* inputs observable (one WARN ... is set but ignored line naming the replacement) rather than restoring the removed write path — a reasonable resolution given the PR's premise that writing pages-publish's config from a settable variable is exactly the vulnerability being closed; silently dropping input was the real gap Codex flagged, not the absence of the old write-path.
  • ${VAR+set} / ${!var+set} correctly test "is set" (even if empty) under set -u without tripping unbound-variable errors.
  • No log line echoes a variable's value, consistent with the PR's "no secret value ever logged" guarantee.
  • Placed before the MELODIC_SETUP_LIBONLY early-return, so it's sourced for setup.test.sh like the other helpers.
  • Called right after log "start version=...", matching the new setup.test.sh assertion that checks that exact ordering in the file text.
• setup.test.sh's new cases (none-set → no log output; both set → each logs its own line, exactly 2 lines, no secret value substring) line up with the implementation; assert_contains/assert_not_contains/assert_eq are the existing harness/shell/lib.sh helpers, used consistently with the rest of the file.
• README addition's Key Vault secret names (pages-publish-config, rendered-views-md) match what pages-publish and rendered-views-sync actually read.

Verification note: I was not able to execute harness/shell/run-tests.sh in this sandbox (the tool denied running it), so I verified the new test logic by static reading against the implementation rather than by running it. The author's test-run/lint claims in the PR description remain author-claimed and unverified by me.
· branch