feat: stele, a CLI that holds the credential so the agent never sees it - #1
Merged
Conversation
Two targets after cbx-cli: SteleKit holds the decisions and depends on nothing, the stele executable holds ArgumentParser and every print. That split is what lets the suite test behaviour as pure functions instead of spawning a process, and it is why CI needs no service container. The Makefile is cbx's with the names changed, keeping both hard-won details and their reasons: install-to-a-temp-name-then-rename (writing over a running binary is ETXTBSY, and agents reinstall while another invocation may be mid-publish), and the ZSHCOMP probe for oh-my-zsh's custom/completions with ~/.zfunc as the fallback. The version lives in SteleKit rather than in CommandConfiguration because `--version` and the User-Agent the server version-gates on must be one string. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018anMA6iMBG1Ekv7rGtSXXb
The logic layer, returning plain data and printing nothing: the executable target owns every `print`, which is what makes `--json` a rendering choice rather than a second path through the API and what lets these tests run without a process or a network. Token custody is structural rather than a convention. `Token`'s plaintext is reachable only through two `internal` members — the `Authorization` header and the credential file — so the executable has no expression that yields it; the three reflection escapes (`description`, `debugDescription`, `customMirror`) are closed; and the type conforms to neither `Encodable` nor `Decodable`, so it cannot fall into a `--json` shape by someone adding a field. `MintedToken` is the single deliberate exception, because a freshly minted credential has no other delivery path, and it still requires an explicit `.secret` at the call site. Server-supplied messages pass through `Redaction.scrub` before they can reach an error description, so the promise does not rest on the other repository's behaviour. The credential file is written `0600` inside a `0700` directory via an atomic rename, and a group- or world-accessible file is refused the way ssh refuses a private key. Host resolution has no environment-variable step anywhere in it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018anMA6iMBG1Ekv7rGtSXXb
Wires SteleKit up to ArgumentParser: auth login/status/logout, publish, update, skill, and admin clients create/list/revoke. The executable holds every `print` and the library holds every decision, so `--json` is a rendering choice rather than a second code path. Three things this commit is really about: The token enters through a TTY and nowhere else. `auth login` prompts with echo off, and refuses a non-TTY stdin instead of reading it — `echo $TOKEN | stele auth login` would put the credential back into the environment and the shell history the credential file exists to keep it out of, which would make every other precaution decorative. There is no --token flag for anyone to reach for. No command prints the token, and the enforcement is access control rather than care: Token's plaintext accessors are internal to SteleKit, so this target has no expression that yields one. `admin clients create` is the single exception, because the server keeps only a SHA-256 and cannot reissue what it minted; that goes through MintedToken.secret, spelled out at the call site, and appears in the --json payload too — omitting it there would make --json a quiet way to lose a credential you just created. Exit codes are the agent-facing API. Outcomes with different next steps get different codes — 2 "ask a human", 5 "pick another slug", 8 "reinstall and retry once", 9 "retryable" — rendered into `stele --help` from the same constants the mapping uses, so a code cannot leave its documentation behind. Two decisions the command line hands straight to the library land in SteleKit where they can be tested as pure functions: content type inferred from the file extension (a hint, not a copy of the server's allowlist), and `--expires-in 90d` parsed into seconds, with a bare number refused because its unit is only obvious to whoever typed it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018anMA6iMBG1Ekv7rGtSXXb
Follows cbx's strategy rather than spawning the binary: every decision is a pure function in SteleKit taking its world as a parameter — the home directory into CredentialStore, the transport into SteleClient — so the suite covers behaviour without a process, a server or a network. The centrepiece is TokenLeakTests. It renders values every way a program can render one — interpolation, String(reflecting:), dump and JSONEncoder, which are four different doors and a type can leave any one of them open — and asserts neither of two canary credentials comes out. Both error types are covered exhaustively through a switch that stops the file compiling when a case is added, and the errors are produced the way production produces them, through the real client from a hostile response, rather than by constructing an enum with a payload the test chose: whether the type *can* hold a token is not the question. That found three genuine leaks, fixed here at construction so no payload holds a credential in the first place: - SteleHost rejecting a value echoed it back. The likely way to land there is a human pasting the token at login's *host* prompt, one line above the token prompt, which put a live credential on the terminal and into any transcript. - The credential file's decode errors quote the file's own bytes — a key, a parser's description of what it choked on — and those bytes are credentials. - transportFailure repeated a reason from SteleTransport, which is a public protocol, so that string can come from a conformer this library did not write. Also pins the status table as identity rather than as prose (a status mapped to a plausible-but-wrong case still gives confident advice for the wrong problem, and the exit code comes off the case), and the request-construction rules the CLI now owns on the agent's behalf. The README follows cbx's structure, with install matching what the server's skill document tells an agent to run — clone, make install, make install-completions — and both traps named: ~/.local/bin on PATH, and LD_LIBRARY_PATH for a swiftly toolchain in a non-interactive shell, whose linker error looks nothing like a missing environment variable. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018anMA6iMBG1Ekv7rGtSXXb
…ould have caught it
Both suites were green and four contracts were broken. The unit tests assert
against a FakeTransport whose expectations were written from this repository's
code, so a field spelled differently from the server's is wrong in the client
and wrong in the test that checks the client, and the two agree with each other
right up to the first live request.
Corrected against the server's routes, which are the source of truth:
- `expiresIn`, not `expiresInSeconds`. The server ignores keys it does not
know, so the old spelling earned a cheerful 201 and a credential that never
expired — the worst failure shape available, because nothing reports it.
- `GET /admin/clients` returns `{"clients": [...]}`, not a bare array. Decoded
through an envelope struct and unwrapped so callers still see a list.
- Revocation is `DELETE /admin/clients/:name`, not
`POST /admin/clients/:name/revoke`.
- `Path.whoami`'s doc note now says the trap out loud: it sits under `/admin`
but is deliberately excluded from the admin scope group, because `auth login`
runs it holding a publish-only credential.
Prompt moves to SteleKit behind a `Console` seam and a typed `PromptError`, so
the custody rule is testable without a terminal. The refusal is unchanged and
now asserted directly: a non-TTY stdin is refused for the secret *and* the host
line, and is never read. `Exit` splits the two prompt failures that used to
share a code — `notATerminal` keeps `noCredential` ("stop and ask a person"),
while EOF at a real terminal is the generic "try again", since an agent that
saw `1` for a refused pipe would retry the pipe.
`scripts/integration-smoke.sh` drives the real binary against a real server and
walks the documented lifecycle end to end. It is the only thing that can catch
this class of bug; the README says plainly what it does not cover.
The token-leak suite is untouched.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018anMA6iMBG1Ekv7rGtSXXb
|
Warning Review limit reached
Next review available in: 58 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Free Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (29)
Note 🎁 Summarized by CodeRabbit FreeYour organization is on the Free plan. CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please upgrade your subscription to CodeRabbit Pro by visiting https://app.coderabbit.ai/login. Comment |
…pping on a bad duration Five findings from a review of the branch. The first is the only one that matters much, and it is the one the leak suite could not see. `SteleClient` documents that "a token is only ever sent to the deployment it was filed under". That was true of how URLs were built and false of what went out on the wire: `URLSession` follows a 3xx by itself and copies the request's headers — `Authorization` among them — onto the redirected request. Verified with two loopback servers: host A answers `302 Location: http://host-B/`, host B receives `Authorization: Bearer <token>`, and the caller sees an ordinary 200. Anything able to answer for the configured host could collect the credential that way. `URLSessionTransport` now builds its own session with a redirect policy and no longer accepts one, since a `session:` parameter is a way to pass a session without the policy. A 3xx is followed only within the same scheme, host and port; anything else stops and becomes `RedirectRefused`, which the client turns into `SteleError.redirected`. That is a separate case from `transportFailure`, whose message promises the credential was never sent — here it was sent, to the right host, which then pointed somewhere else. `TransportTests` is the first test in the suite to speak TCP, and it has to: redirect following is a decision `URLSession` makes rather than one this code makes, so a fake transport tests the seam and not the thing. Two real servers on loopback ports, differing only in port — a policy comparing hostnames alone would pass while leaving the same hole open. With the delegate removed the cross-origin test fails and the same-origin one still passes, so it is the policy under test and not redirect handling in general. The other four: - `--expires-in 999999999999999d` multiplied out past `Int`'s range, and `Int(Double)` out of range is a runtime trap, not an error — a crash and a backtrace where every other bad duration gets a sentence. `ExpiryDuration` now has a ceiling and checked arithmetic, with `.tooLong` as its own case because the syntax was right and the number was not. `SteleClient` clamps besides, since `createClient` takes a `TimeInterval` from any caller. - `Redaction.scrub` recognised a credential by the `stele_pat_` prefix, which is exactly what the token most likely to be pasted at the host prompt does not carry: `Token.init` accepts an unprefixed token on purpose, because until the server's shared upload token is demoted that is what an operator logs in with. `scrubbedEcho` withholds a whole typed value that is one unbroken run of credential characters. A mistyped host is still quoted back — a message that withholds the typo cannot be acted on. - `attributesOfItem` does not follow symlinks, so a credential file symlinked out of a dotfiles repository was judged by the link's own 0777 and refused forever: `chmod` follows the link and changes the target, the link stays 0777, and the printed remedy loops. Resolved before the stat, and only when the last component is actually a link. - Two spellings of one deployment in the credential file (`https://h` and `https://h:443`, or two casings) normalise onto one key, and `[String: Any]` has no order — so the credential used could differ between two runs over identical bytes. Refused now, naming both spellings. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SZwrF9ZtAUQHDGzCoJRMQi
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The initial implementation of
stele, the client for stele-pages. Pairs with ProJedi1234/stele-pages#19, which adds the per-client credential system this talks to.Why a CLI at all
Not ergonomics — custody. The server's skill document used to tell an agent to hold a bearer token and run
curl, which put a secret in the same context window as a document about to be published at a guessable URL. The only thing standing between them was a prose rule ("never write it into a page you publish"), which is a rule you only write down because the failure is plausible.Now the agent runs
stele publish page.htmland the token lives in a0600file it never reads. The credential enters once, through a human runningstele auth login.The rules that make that real
auth loginreads from a TTY and refuses a pipe, because argv is visible inpsand lands in shell history — and shell history is something an agent reads.sshrefuses one.auth status, not--json, not any error path.Token.secretis private, the type conforms to neitherEncodablenor round-trippableDecodable, anddescription/debugDescription/customMirrorare all closed. There are exactly four plaintext reads in the codebase: theAuthorizationheader, the0600file, and the two deliberate ones inadmin clients create, where the token exists for the only moment it ever will.--hostto override.TokenLeakTestsproduces every error case through the real code path from a hostile server that echoes credentials back, renders each four ways (interpolation,String(reflecting:),dump,JSONEncoder) and asserts the secret appears nowhere — with an anti-vacuity check that the canary really is present in the0600file.Structure
Follows
cbx-cli: a dependency-freeSteleKitthat returns plain data and prints nothing, and a thinsteleexecutable that owns all rendering. Tests target the library, so nothing needs a spawned process. TheMakefilekeeps two details worth not simplifying away — the install-to-temp-then-mvthat avoidsETXTBSYwhen overwriting a running binary, and the completion-directory probe that picks oh-my-zsh'scustom/completionswhen it exists and falls back to~/.zfunc.An honest note on how this was caught
The first version of this CLI passed its whole suite while being wrong about four things: it called a route the server never implemented, used the wrong verb for revoke, decoded a bare array where the server sends an envelope, and sent
expiresInSecondswhere the server readsexpiresIn— so--expires-inwas silently dropped and minted credentials that never expired.All four survived because the tests asserted against a fake transport built from this repo's own assumptions. Two green suites, zero integration.
scripts/integration-smoke.shis the fix: it drives the real binary against a real server and a real database through the whole lifecycle, and fails loudly. The expiry case is checked against a control credential minted without the flag, so a fabricated value can't pass.Verification
swift test— 75 tests in 12 suites, passingKnown gaps
auth login's pipe refusal but seeds the credential file rather than driving the interactive prompt; the interactive path was verified manually over a pty. Automating it needs a pty driver.Prompt's non-TTY guard now has coverage, but the executable target's test story is thinner than the library's.🤖 Generated with Claude Code
https://claude.ai/code/session_018anMA6iMBG1Ekv7rGtSXXb