Skip to content

feat(harness-ops): resolve account.mjs values from the driven session's holder - #13802

Merged
kyle-sexton merged 7 commits into
mainfrom
feat/control-claude-code-driven-session-account
Nov 6, 2026
Merged

kyle-sexton merged 7 commits into
mainfrom
feat/control-claude-code-driven-session-account

Conversation

@kyle-sexton

Copy link
Copy Markdown
Member

Refs: #11364

session: App Driven PR Verify

Summary

Drive's mask post-check resolves control-claude-code's values: source by running cli/account.mjs. Today that runs claude auth status in the caller's process. In the CI capture the caller is the outer leg, which never reads the driven session's config, so the answer describes the wrong process.

cli/account.mjs now asks the running session holder, which already runs claude auth status in the driven session's own launch environment to mask captures (maskedFrame). The holder serves a new account request from that status, cached once per session.

Output:

  • the email and organization;
  • else none: API-key auth has no signed-in account, only when that session is on key auth and its launch kept ANTHROPIC_API_KEY;
  • else exit 1 with empty stdout. That covers no session, several sessions (unless --session names one), or any other reply.

Fix

  • driver/driver.mjs: the holder's account request, accountLines, and drivenAccount().
  • cli/account.mjs: now a thin wrapper around drivenAccount(). It accepts only the none: line alone or single-line values.
  • cli/README.md and a changelog fragment (bump: minor).
  • The ## Mask values: line in SKILL.md is unchanged, so no skill body changes.

Verification

  • node --test plugins/harness-ops/skills/control-claude-code/driver/driver.test.mjs: 89 pass. 8 new account cases fail on the old implementation.

Remaining

  • Merge order: this branch builds on PR 13762 and merges after it.
  • CI allowlist: the companion workflow PR adds the exact node <abs>/cli/account.mjs rule to the capture's Bash allowlist.
  • Values after Cleanup: values resolve only while a session runs. Drive's post-check rewrites the report again after Cleanup, so drive must reuse the values it already resolved. That is an app-control follow-up after PR 13765 merges, tracked on feat(harness-ops): verify PR acceptance criteria by driving Claude Code (screenshots, video) before merge #11364.
  • Needs a live CI run: that claude auth status reports api_key inside the sibling sandbox, and that drive types the exact allowed command.

Related

🤖 Generated with Claude Code

kyle-sexton and others added 6 commits November 6, 2026 12:00
When `claude auth status --json` reports authMethod api_key with no
email and no organization, cli/account.mjs prints
`none: API-key auth has no signed-in account` and exits 0, so drive's
mask post-check can resolve the values: source to no values. Every
other case keeps its exit 1.

The spawn-and-parse moves into an exported authStatus helper;
accountValues still returns the array the driver masks with at
capture. Adds account.mjs tests against a stub claude and a key-auth
/status fixture, rewritten from a real 2.1.319 screen with every value
invented, with a maskCells test over it.

Refs: #13757

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

Drive runs cli/account.mjs from the outer capture leg, whose scrubbed
shell holds no credential, so `claude auth status` reports loggedIn
false and authMethod none there, never api_key. Treat that state as no
account to mask. A missing login that names any auth method still exits 1.

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

account.mjs may not read the config the driven session signs in with, so a
missing login is no proof there is nothing to mask. Only explicit API-key
auth prints none:. CI gained nothing from the no-login case anyway: the
capture's narrowed Bash allowlist does not run cli/account.mjs.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
cli/account.mjs ran `claude auth status` in the caller's environment.
In the pr-verify capture the caller is the outer leg and the driven
session runs in the sibling sandbox, so the values source described the
wrong account. The session holder now answers an `account` request by
running `claude auth status --json` in that session's own launch
environment, the same status its capture masks read. account.mjs asks
the one session running (or `--session <name>`) and checks the reply:
the `none:` line alone, or one-line values; anything else, no session,
or several sessions exits 1 with nothing on stdout. The `none:` line
also needs the launch to have kept ANTHROPIC_API_KEY.

Refs: #11364

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Resolve the control-claude-code conflicts with #13762's squash in this
branch's favor: it builds on #13762 and supersedes account.mjs, the holder
and their tests.

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

Copy link
Copy Markdown
Codex Review Summary: ❌ Failed

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

Review Status Commit Review trigger
📝 Code Review ❌ Failed 2026-11-06T19:10:37.273488Z
We couldn’t complete this review. Comment @codex review to try again.
d44d729 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

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@github-actions

github-actions Bot commented Nov 6, 2026

Copy link
Copy Markdown
Contributor

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

Security review: PR #13802 (head d44d729)

No security issues found. I read every hunk of all 5 changed files. No changed file matches a trusted-path glob, and the PR changes no plugin manifest.

What I checked

  • New account verb on the session holder (driver/driver.mjs). The holder already ran claude auth status --json in the session's launch environment to build the masks. The new verb only returns that cached result (email and organization), or the fixed KEY_AUTH_NONE line. The socket lives in runtimeDir()/ccd. privateDir (driver.mjs:221-230) refuses that directory unless it is owned by the current user and has mode 0700. On Windows, endpoint uses a random pipe suffix stored in a 0600 file. So only the same local user can reach the verb. It exposes nothing that user's own claude auth status doesn't already show them.
  • liveSessions discovery. It calls privateDir(..., { create: false }), which runs the same ownership and mode check when the directory exists. Names are filtered through SESSION before request(), and request()'s endpoint() checks the name again. No path injection from directory entries.
  • What the client accepts from the holder. drivenAccount fails closed. It accepts only the none: line on its own, or a non-empty array of single-line values with no none: prefix. Anything else exits 1 with empty stdout. A broken or fake holder therefore can't make the mask post-check read "nothing to hide".
  • accountLines and keySource. The none: result now also requires launch.keySource === 'ANTHROPIC_API_KEY'. This only makes the masking check stricter.
  • authStatus default env removed. Every remaining caller passes ctx.env (driver.mjs:662). No call without an argument is left that would inherit the caller's full process.env. The status output is still never echoed into error messages.
  • cli/account.mjs. It uses parseArgs with one string option. The session name is checked by checkSession (a UsageError maps to exit 2). It prints account values to stdout, as the old version did. No new sink.
  • Tests and changelog. The tests use fake sandboxes and dummy keys only. The changelog is prose.

Not checked here

  • The companion workflow PR that adds node <abs>/cli/account.mjs to the CI capture's Bash allowlist is outside this diff, so this review does not cover it.

critical-findings: 0

@github-actions

github-actions Bot commented Nov 6, 2026

Copy link
Copy Markdown
Contributor

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

Code review: PR #13802, the driven session's account from its holder

No findings. I read every hunk of all 5 changed files. I also read the unchanged code the changed lines call: buildLaunch, privateDir, runtimeDir, endpoint, request, serve, maskedFrame, the error classes, and the test helpers (withEnv, withPath, placeEngine, netForHolders).

What I checked:

  • Holder account verb. ctx.env is launch.env, which is a separate object from process.env. So when serve deletes the CI names from process.env after start, authStatus(ctx.env) still runs with the kept ANTHROPIC_API_KEY. accountLines gets the keySource that buildLaunch computed, and sessionStatus caches the status only after a successful read. A failed read is retried on the next call and is never cached as empty.
  • maskedFrame. It now goes through the shared sessionStatus cache and still masks with statusValues, the same values as the removed accountValues(ctx.env).
  • drivenAccount / liveSessions. Leftover .sock or .pipe endpoints whose holder is gone fail the ping (ECONNREFUSED/ENOENT) and are left out. Names that don't match SESSION are filtered out before endpoint can throw. The reply check accepts only the exact KEY_AUTH_NONE line on its own, or non-empty single-line values that don't start with none:. That fails closed on every forged shape the new test tries.
  • cli/account.mjs exit codes. A bad or empty --session, an unknown flag or a positional argument exits 2: parseArgs throws, or checkSession raises UsageError with code 2. NoSession (code 3), Unsafe (5) and holder errors exit 1. This matches the README row.
  • Removed exports. Nothing else in plugins/ imports accountValues or calls authStatus() with no argument. The spawnSync import removed from the test file has no remaining uses.
  • Tests. The new cases give the caller a claude that always disagrees with the holder's. A value read in the caller's environment would therefore fail each case. I did not run them: this lane has no shell. The "89 pass" result is the author's claim and unverified.

Notes, not counted as findings:

This is a whole review, not blocking-only: the prior-findings file shows no earlier head reviewed.

@kyle-sexton
kyle-sexton added this pull request to the merge queue Nov 6, 2026
Merged via the queue into main with commit c0ede3a Nov 6, 2026
83 checks passed
@kyle-sexton
kyle-sexton deleted the feat/control-claude-code-driven-session-account branch November 6, 2026 20:29
@github-actions github-actions Bot added the merge-path:owner-agent Post-merge accounting record (README.md, Merge paths) label Nov 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-path:owner-agent Post-merge accounting record (README.md, Merge paths)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant