Skip to content

feat(harness-ops): accept the one CI credential in unattended control-claude-code runs - #12372

Merged
kyle-sexton merged 4 commits into
mainfrom
host-run/11386-ci-credential
Nov 2, 2026
Merged

kyle-sexton merged 4 commits into
mainfrom
host-run/11386-ci-credential

Conversation

@kyle-sexton

@kyle-sexton kyle-sexton commented Nov 2, 2026 •

Copy link
Copy Markdown
Member

Refs: #11386

Summary

control-claude-code can now run unattended with the one CI credential its job provides, as the owner's Q10 decision on #11386 puts in this slice. Under --unattended, the driver keeps the single credentials.ci variable the job set, turns on the subprocess scrub, refuses two set at once, and Ready and logs accept that credential's apiKeySource. Attended runs are unchanged: the API-key, gateway and provider variables are cleared, the person's own CLAUDE_CODE_OAUTH_TOKEN passes through as before, and Ready requires none.

Fix

Verification

  • bash plugins/harness-ops/skills/control-claude-code/driver/driver.test.sh: 46 pass, 0 fail (was 39). The new cases cover:
    • attended clears the API key, passes the person's OAuth token through, and requires none;
    • unattended keeps each one alone, with the scrub on, and expects its source;
    • unattended with neither set behaves as attended;
    • two set refuses launch and ready with exit 1, and no claude starts;
    • an unknown credentials.ci name is refused;
    • an unattended launch hands claude the key with the scrub on, and the holder then unsets it;
    • unattended ready and log pass on ANTHROPIC_API_KEY, and attended ready with the same key set still fails;
    • a kept key that reports none fails.
  • scripts/check-affected.sh --tests on the committed head: 14 passed, 2 skipped.
  • UNVERIFIED: CLAUDE_CODE_OAUTH_TOKEN reporting apiKeySource: none. That is the owner's spike leg; until it is recorded, a token that reports any other source fails Ready. The scrub removing CLAUDE_CODE_OAUTH_TOKEN is also unverified: the docs table lists ANTHROPIC_API_KEY and says names that look like a credential (NPM_TOKEN) are removed. feat(pr-pipeline): run the project's app-control skill in pr-verify after the deterministic tests #8651's S7 probe tests both in CI before capture is enabled.
  • Left out: a live run with either credential, since this host holds neither variable and the run creates no token or key; driver.test.sh on Ubuntu 24.04 locally (this host is 26.04), covered by the test-bash job on ubuntu-24.04.

Remaining on #11386

  • The spike's CLAUDE_CODE_OAUTH_TOKEN leg (apiKeySource and first screen) is the owner's step. It needs a token from claude setup-token, then driver.mjs ready --unattended with CLAUDE_CODE_OAUTH_TOKEN set. If the source is not none, change CI_KEY_SOURCES in driver.mjs to match.

Related

🤖 Generated with Claude Code

kyle-sexton and others added 2 commits November 2, 2026 08:59
…-claude-code runs

Under --unattended, the driver keeps the single credentials.ci variable the
job set, turns on CLAUDE_CODE_SUBPROCESS_ENV_SCRUB so the plugin under test
cannot read it, refuses two set at once, and Ready and logs accept that
credential's apiKeySource. Attended runs are unchanged.

Refs: #11386

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kyle-sexton
kyle-sexton marked this pull request as ready for review November 2, 2026 14:05
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 2, 2026 •

Copy link
Copy Markdown

Codex Review Summary

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

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-02T14:07:03.555783Z edccab9 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.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: edccab9c3d

ℹ️ 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 github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

claude-security-review: 1 inline finding(s) on edccab9c3d973e7624e0004487200741be3fb2f4; the summary is in this lane's pull request comment.

claude-lane: claude-security-review

@github-actions

github-actions Bot commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

claude-security-review reviewed 9a87349b9fa04cbaaa35932c67f11ee2d3c4d1e2: read 3 of 3 changed file(s); 0 finding(s), 0 as inline comments.

Security review (incremental): PR #12372, 72e1e497..9a87349b

I read every hunk of the 3 files in scope: the changes fragment, SKILL.md and driver.mjs. None of them matches a TRUSTED PATHS glob. The diff touches no trust control (trusted-actor check, lane token, kill switch, merge hold or .github/actions/*) and no plugin manifest, so there are no manifest lines.

What changed since my last review. Only wording changed; no code behavior changed.

Earlier findings. My SUGGESTION about the holder's /proc/<pid>/environ was settled by the owner at this head (lines changed since resolution: no). This change makes the wording narrowing it asked for, and the owner's reply says #11386 asks #8651's probe to read the holder's environ as well as $PPID's. My earlier IMPORTANT finding's open points (the environment of claude itself, and the PID-namespace question) are also owner-settled and routed to #8651's S7 probe. I don't raise either again.

Checked, no finding

  • The delete loop over CI_NAMES is still the same line, still runs after terminal.run, and still uses the fixed allowlist.
  • The new wording overstates nothing: it no longer says the holder "keeps no credential", and it does not say the kept credential is out of the plugin under test's reach.
  • No workflow passes --unattended; the PR description states this, and the diff adds no caller.

No security issues in this increment.

Spend: an unattended run draws on API credit (ANTHROPIC_API_KEY) or subscription usage (CLAUDE_CODE_OAUTH_TOKEN). Neither is a hold.

critical-findings: 0

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

claude-review: 2 inline finding(s) on edccab9c3d973e7624e0004487200741be3fb2f4; the summary is in this lane's pull request comment.

claude-lane: claude-review

@github-actions

github-actions Bot commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

claude-review reviewed 9a87349b9fa04cbaaa35932c67f11ee2d3c4d1e2: read 3 of 3 changed file(s); 0 finding(s), 0 as inline comments.

Code review (incremental, 72e1e497..9a87349b): PR #12372, --unattended CI credential for control-claude-code

No new findings this round.

I read every hunk of the incremental diff in all 3 files in scope: the changes fragment, SKILL.md and driver.mjs. This round only changes wording. It answers the settled suggestion that the holder still exposes the credential through /proc/<pid>/environ.

What changed

  • driver.mjs (serve, around L505). The old comment said "the holder keeps no credential for the rest of the session". The new comment says what delete process.env[...] actually does: later children and in-process reads stop seeing the key, but /proc/<pid>/environ still shows the starting environment, and feat(pr-pipeline): run the project's app-control skill in pr-verify after the deterministic tests #8651's isolation probe covers that. This is accurate, because unsetenv edits only the live table. The code did not change.
  • Changes fragment. "drops" became "unsets", with a note that the holder's /proc environment still holds the key. This matches the code.
  • SKILL.md L33. "drops" became "unsets". The paragraph still says the kept credential is not proven out of the plugin under test's reach and that no CI job passes --unattended until feat(pr-pipeline): run the project's app-control skill in pr-verify after the deterministic tests #8651's probe shows it is. That is consistent with the driver comment and adds no upstream restatement.

Prior findings and settled threads

  • My two findings from round 1 (OAuth token passing through on attended runs, and SKILL.md restating upstream behaviour) were resolved in 72e1e497. This increment does not touch them.
  • The holder /proc environ thread and the subprocess-scrub thread are settled by owner decisions, with lines changed since resolution: no. I have not raised them again. Whether the credential is really isolated stays with the security lane and feat(pr-pipeline): run the project's app-control skill in pr-verify after the deterministic tests #8651's S7 probe.

Unverified (author's claim)

  • The PR's test counts: 46 passing in driver.test.sh, and 14 passed / 2 skipped in check-affected.sh. I ran neither, because this lane has no shell.

Other notes

  • Closing issues: none. The PR only says Refs: #11386.
  • Plugin manifests: this increment changes no manifest. The fragment's bump: minor still fits.
  • Embedded text: none found in the PR context, settled threads or prior findings.
  • Spend: unattended runs would draw on the job's ANTHROPIC_API_KEY or subscription token. That is prepaid credit or subscription usage only, not a hold.

… drop the key from the holder

Attended runs pass the person's own CLAUDE_CODE_OAUTH_TOKEN through, as
before; the docs and test now say so. The session holder deletes the
kept CI credential once claude starts. SKILL.md states that the scrub does
not prove the credential is out of the plugin's reach (#8651's probe does)
and points at the env-vars row instead of restating the key prompt.

Refs: #11386

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

claude-security-review: 1 inline finding(s) on 72e1e49732aa6d0f73a14672641ffa0b8c0bbb32; the summary is in this lane's pull request comment.

claude-lane: claude-security-review

…d does not stop

Refs: #11386

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kyle-sexton
kyle-sexton added this pull request to the merge queue Nov 2, 2026
Merged via the queue into main with commit 5493eaa Nov 2, 2026
70 checks passed
@kyle-sexton
kyle-sexton deleted the host-run/11386-ci-credential branch November 2, 2026 14:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant