feat(secrets): credential recipes as targets, and the broker that mints them - #452
Closed
raphaelvigee wants to merge 14 commits into
Closed
raphaelvigee wants to merge 14 commits into
raphaelvigee wants to merge 14 commits into
Conversation
…ts them A target names the credential it needs by address. The *recipe* is built, hashed and shared like any other dependency; the *value* is minted at run time and never persisted. This is phase 1: the declaration and the broker. Consumer wiring, sandbox delivery, `cache.subject_scoped`, the `oidc` provider and `heph auth` follow. The whole design turns on one split. A `secret()` declaration has an *identity* half (role, audience, registry, machine, profile, shape) that belongs in every consumer's cache key, and an *acquisition* half (provider, helper, protocol, runner, timeout, ttl) that must not be — otherwise CI saying `oidc` and a laptop saying `exec` never share a cache entry for any consumer, which is `pass_env`'s disease moved one level up. That split is structural rather than a flag anyone must remember: a consumer's `hashin` folds in its inputs' *hashouts*, so the driver writes only the identity half into `secret.json` and the broker reads the rest from the spec. A field that never becomes an artifact has no hashout to contribute. `crates/e2e/tests/secret.rs` pins both directions on a real consumer's `hashin`. Three defects the review board caught, all reproduced before fixing: - **Two descriptors shared one cache key.** Every `Identity` field is optional, so `//creds:prod` and `//creds:staging` — differing only in which env var they read — emitted byte-identical artifacts. A consumer of one was a cache hit on the other's, served output built with the wrong credential, winner decided by scheduling order. The descriptor's address is now in the hashed bytes. Cost, stated deliberately: moving a descriptor re-keys its consumers. - **The redacting tee leaked at chunk boundaries.** Holding back a partial prefix could truncate a *longer* match and let a shorter pattern win, so a split emitted nine bytes of a live credential and masked it under the wrong name. Matches straddling the hold are now carried whole. - **6-7 byte values were neither masked nor reported.** The length filter ran per encoding, so a short value's base64 survived while its raw form — the form actually printed — went out verbatim with no warning. Also from review: RFC 3339 parsing moved to `chrono` (already a workspace dep) after the hand-rolled version was found rolling Feb 30th forward three days and reading a missing offset as UTC — expiries wrong in the direction that strands a build mid-target; `credential_process` no longer denies unknown fields, since it is AWS's schema and not ours; `env` pointers are normalized so three spellings of one field are one key; live values are bounded across re-mints; and the emitted bytes are frozen by a golden test, because they are a cache key and serde_json's layout is not a contract. An `exec` helper now has a 60s deadline, overridable per entry. Closing stdin only stops a *stdin* prompt — a macOS keychain dialog, a Touch ID prompt or a blocked network call reads none, and hangs a build nobody is watching. The deadline arrives as a cancellation so the helper is killed and reaped rather than orphaned. Per-platform: no cfg splits; a hanging helper was the one macOS-leaning hazard and the deadline is uniform across all three targets. Compatibility: no proto or ABI change — the driver is compiled into the host and crosses no plugin seam. `secret.json` is a new format at version 1, checked by exact match, local-cache-only. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RLvHrjGHVTciRH4DU3jcwR
… as standards
Two corrections to the descriptor schema, both made now because it is about to
freeze into a format other heph versions read back.
**The acquisition half is a `oneof`.** It was a flat bag of options where
`provider = "static_env"` and `helper = [...]` could be written in the same
breath and only a hand-written rule caught it. `Source` is now a tagged union
discriminated by `provider`, so each provider owns its own fields: `helper` on
a `static_env` is an unknown key, an `exec` with no `protocol` is a missing
required field caught at parse, and a `timeout` on an `oidc` cannot be spelled
at all. Five cross-field rules did not move — they stopped being expressible.
Three remain, and they are the ones a union genuinely cannot carry: a non-empty
argv, an `oidc` with something to trade its assertion for, and durations that
parse. `var`/`vars` collapsed into one field with two spellings, taking the
mutual-exclusion rule with it.
**Exchanges are grants, not vendors.** `aws | gcp | gcp_sa_key | github_app |
r2_temp` privileged whichever vendors were in front of the author — a GitHub App
token was a first-class concept while an internal IdP was inexpressible — and
put vendor names in a frozen format, so the next vendor meant a schema change.
What is general is the grant: `token_exchange` (RFC 8693), `jwt_bearer`
(RFC 7523), `client_credentials` (RFC 6749 §4.4), plus `aws_sts`, which stays
because it is how one of the three clouds federates and it predates RFC 8693.
Everything else is `http` — a URL, headers, a body, and JSON pointers — so a
GitHub App installation token and a Cloudflare R2 temporary credential are
described in the BUILD file and need no name in heph. `exchange` is a pipeline,
because GCP federation is RFC 8693 followed by an impersonation call.
The same argument reaches the hashed half, where it matters more: `app_id`,
`install` and `impersonate` were named identity fields, i.e. vendor names inside
the one format frozen into every consumer's cache key. They become entries in
`params`, hashed like the rest of the identity, so a new vendor re-keys nothing.
`Protocol::Engflow` becomes `CredentialHelper` for the same reason — the spec is
Bazel's `--credential_helper` and the helpers that speak it are not EngFlow's.
Spec-parsing support added for all of it:
- `#[derive(SpecOneOf)]` — a tagged union over struct variants, with a
per-variant unknown-key check, a required-field check, and a schema that is a
union of one struct type per variant so an editor can offer the right keys.
- `FromSpecMap` + `#[spec(flatten)]` — the inline form (`provider = "exec"` on
the target) and the `acquire` list now share one parser, rather than the
inline form needing a second, hand-maintained key list that would drift.
`Option<T>` is absent exactly when the tag key is.
- `#[spec(alias = "…")]` — a second accepted spelling, both at once an error.
This is what keeps `var = "TOKEN"` working alongside `vars = {…}`.
- `FromSpecValue for BTreeMap<String, String>`, for a map whose iteration order
is observable because it reaches a hash.
No behavioural change to the cache-key contract: the identity/acquisition split
and the e2e tests that pin it in both directions are unchanged.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RLvHrjGHVTciRH4DU3jcwR
…ays so The design document described the whole feature; only part of it is merged. A document read as a promise and tried once is a document distrusted afterwards, so the line between the two is now drawn explicitly — and drawn by a test rather than by prose. `crates/e2e/tests/secret_examples.rs` executes every declaration the docs publish. Each goes through the real BUILD-file evaluator and the real declaration parser; the ones that can mint today go through the real broker with real helper subprocesses. `static_env` mints, all four `exec` protocols mint (including `docker_credential`, whose bare-URL stdin is proved by echoing it back), two-route selection picks the right entry per environment, and the standards-first exchanges parse — including the vendor-REST one that shows a GitHub App token needs no vendor support in heph at all. The unbuilt half is pinned too: `oidc` declares and validates and then reports that no provider is registered for it. That assertion fails the day the provider lands, which is the reminder to update the docs with it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RLvHrjGHVTciRH4DU3jcwR
…ound
Two problems, and the first was a bug in what shipped: `stale_at` answered
"is this token still valid?" and was then used to answer "is it worth handing
to a target that is about to start?" Those are different questions. A
credential one second inside the 60s skew margin passed the first and failed
every target that runs for longer than a second — nondeterministically, since
whether it bit depended on where in a credential's life the scheduler happened
to start the target.
So the margin splits in two. `REFRESH_MARGIN` stays what it always meant: clock
skew, nothing else. `MIN_HANDOUT_LIFETIME` is new — the usable life a credential
must have left to be given to something about to run. A handout now re-mints a
still-valid credential with too little left rather than passing the problem on.
A regression test pins it, and mutation-testing the old rule back in place shows
it handing over a credential with 30 seconds of usable life.
The naive form of that rule is worse than the bug, so `Expiry` now records
`issued_at`: a credential whose *whole* lifetime is shorter than the headroom
cannot be refreshed into a longer one, and re-minting it on every handout would
be a mint per target buying nothing. That case warns once and says what to do
about it.
Second, a run that lasts hours refreshed only when a consumer asked, which puts
the mint latency on whichever target is first past the line. `Broker::refresh_due`
renews ahead of demand and `spawn_refresher` runs it on a tick, so by the time a
target asks the value is warm and the handout is a clone. The sweep holds a weak
reference (a forgotten guard leaks a sleeping task rather than pinning every
live credential for the process lifetime), skips slots that are mid-mint rather
than queueing behind a slow helper, and keeps the existing value when a refresh
fails — a background task has no target to blame and no user watching, so the
consumer that actually needs it reports the error instead.
The sweep logic is a plain async fn the tests call directly; only the loop is
spawned, and there is a test that it renews and that dropping its guard stops it.
None of this addresses a credential expiring *inside* one long-running target.
Nothing outside that process can replace a value it has already read; that needs
a process credential the tool re-reads, and is called out as separate work.
Also from review of the exchange model: the three OAuth grants now take an
`issuer` and discover the token endpoint from
`{issuer}/.well-known/openid-configuration`, rather than requiring a literal
one. That is the standard, it is what an administrator can actually tell you,
and it removes an inconsistency with `heph auth login`, which already resolves
its own endpoints that way. Discovery also makes a diagnostic possible:
`grant_types_supported` lets an unsupported grant fail by name before the
request instead of arriving as a bare `400 unsupported_grant_type`. `endpoint`
remains for a server that publishes no metadata; exactly one of the two.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RLvHrjGHVTciRH4DU3jcwR
…ub the sandbox
The point at which the feature does something. A target names a credential and
gets one; nothing durable holds it afterwards.
target(name = "push", driver = "bash",
secrets = {"ecr": "//infra/creds:ecr"},
run = ["aws ecr get-login-password | ..."])
The split of work keeps the plugin seam narrow: **the host does everything
except place the environment.** It reads the declarations, checks the slots,
mints, writes the files, and scrubs them. What crosses to the driver is a map of
environment variables — mostly paths — plus the values, for redaction only.
No ABI change. A credential reference is an annotated `Input`, the channel
scratch already uses, so no proto message moved and a third-party driver
participates without recompiling.
All seven shapes render. `file` writes 0600 under `<sandbox>/secrets/`; the rest
write a synthetic `$HOME` — which is not a convenience, since `git` reads
`$HOME/.netrc` through libcurl and honours no override, so a pointer variable
alone would leave every private clone unauthenticated. Merged files are written
sorted, and the slot collisions were already rejected from declarations alone,
so a renderer appends without arbitrating. Region and endpoint go in the AWS
*profile*, never a scalar variable: no single `AWS_REGION`-shaped variable
satisfies boto3, the JS SDK and the Java SDK at once.
The redactor is finally plugged in, one stream at a time — stdout and stderr
interleave in one loop, and a shared matcher state would corrupt a match
spanning the other stream's chunk. Held tails are flushed at EOF, or output
sharing a prefix with a live credential and arriving last is never written at
all.
Credential variables are exempt from `filter_long_env` eviction. A long token is
exactly the entry the "evict the longest" rule picks first, and the result would
be an authentication failure with no stated cause, in the one subsystem where a
mysterious 401 is most expensive to debug. If they genuinely do not fit, that is
now a hard error saying so.
**The scrub runs on both paths, and it took a test to notice.** It was written
for the failure path — a failing target's sandbox is deliberately kept as the
diagnostic, so credentials in it survive until that target's next run. But on
*success* the rendered file simply stayed there too: the sandbox teardown is
queued rather than immediate, and "nothing durable is written" has to mean it.
So credentials come off the moment the process is done with them, either way.
Nothing needs them by then — they render outside `ws/`, so no output glob can
collect one and `cache_locally` never reads them.
The scrub walks the sandbox rather than a recorded file list, so it also catches
what a tool wrote into the synthetic home on its own.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RLvHrjGHVTciRH4DU3jcwR
**Transitive.** A credential is usually a property of a *dependency*: whatever
pulls a private module needs the GitHub credential because of what it depends
on, not because of anything it does. Restating `secrets = {…}` on every consumer
drifts when hand-written and is impossible when it is not — the Go plugin emits
targets per directory, thousands of them, and nobody authors those.
So `Sandbox` gains a `secrets` map, contributed by `transitive = {"secrets":
{…}}` and turned into inputs by `apply_transitive` with the same
`hashed: true, runtime: false` shape a declared one produces. The identity has
to reach a target's key whether the author wrote it or inherited it.
Three rules, all tested end to end. The target's own declaration wins — which
resembles the silent overwrite the slot rule refuses and is not, because here
one of the two parties is written in the target itself, and it is the escape
hatch when a dependency's choice is wrong for one consumer. Two deps needing the
same descriptor dedupe silently. Two deps supplying one *name* from different
declarations fail, naming both chains: that is worse than a slot collision,
because the name is what the command references and neither party appears at the
failing target's call site.
`skip_serializing_if` on the new field is load-bearing rather than tidy:
`Sandbox` serializes into `hashin`, so a field that always emitted would re-key
every target with any transitive sandbox at all, on upgrade, for nothing.
**Logs are no longer uploaded to the shared cache.** They were, and it was a
disclosure with nothing on the other side of the ledger: `artifact_is_needed`
returns false for a log, so a remote hit never fetches one. Every log a fleet
produced was pushed and read by nobody — while a tool that printed an
authenticated URL published it org-wide. Redaction masks what heph knows about,
but is best-effort by construction. Logs are still packed locally, which is
where the failure renderer reads them.
**`heph inspect def` no longer prints `pass_env` values.** They are snapshotted
from the host at parse time, so printing the def verbatim published whatever the
shell that ran the build held — an API token as readily as a `TERM`. The names
survive, since they are what a reader of that command is after; the values are
host state that was never theirs to see. Masked at the print rather than in the
def, so the wire form a driver round-trips is untouched. Even innocuous values
are masked: this command cannot tell which is which, and guessing would be the
leak.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RLvHrjGHVTciRH4DU3jcwR
**`allow`** makes which targets may hold a credential a declared, reviewed fact. Access control without a new ACL system: which credentials exist is CODEOWNERS on the declaring package, and which targets may use one is a line in the same file, written as an ordinary target query (`"//svc/... + label(deploy)"`). It is evaluated on the **effective** set — what a target holds after `apply_transitive`, not what it declared. Anything else lets a dependency launder a credential past its own policy: the consumer names nothing, and the check that was supposed to stop it never sees the edge. The failure is legitimate even when the consumer wrote nothing, so the message carries the chain; without that a reader is told their target may not hold a credential they have never heard of. `allow` is unhashed. It decides whether a build is permitted, not what it computes — a target that passes produces exactly what it would have without one, so folding it into a key would invalidate every consumer for a policy edit that changed no output. Pinned by a test. **`cache.subject_scoped`** keys a target by who ran the build, for the minority whose result genuinely differs per person. It belongs to the consumer, not the descriptor: whether a result varies by who produced it is a property of what the target computes, and the same credential routinely feeds both kinds — one descriptor serves `go list`, whose module graph is exactly what that token could see, and `gh release upload`, whose result does not vary at all. It keys on the *run's* identity, not a post-exchange subject: `hashin` is computed before `execute`, so hashing the latter would force a subject-scoped target to mint before its own cache key existed, and a warm build would mint once per target to discover it had nothing to do. Resolved once per run, and only when a target asks. Unset it writes nothing into the hash, so every target that does not opt in hashes byte-identically to before. The subject is injectable through `Config` rather than read only from the environment, because detection reads process-global state that a test cannot set without racing every other test in its binary — and a cache-key input deserves a test that is not a coin flip. **`heph auth`** groups the credential surface under one noun. `show` reports what a target would hold, where each shape writes, and which declaration owns each entry, without minting — it is the visibility half of the design's bargain, since "the author configures" is only fair to ask of someone who can see what they are configuring, and it flags the credential-bearing-and-remotely-cached combination that warrants a decision. `check` mints and immediately drops, deduped by descriptor; on a warm workspace it is the only thing that ever validates the credential path, since a cache hit mints nothing. Both take `--json`. There is deliberately no `token` subcommand. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RLvHrjGHVTciRH4DU3jcwR
…audit trail
**`oidc`.** Two halves, configured independently, which is what lets one
descriptor work in CI and on a laptop. The assertion is ambient — GitHub Actions
hands a job an endpoint and a bearer token and will mint an ID token for any
audience; nothing is configured, it is either there or it is not. The exchange
is an ordered pipeline of standard grants, each consuming what the last
produced, so GCP's two hops are a list rather than a special case.
Detection requires *both* request variables, because that is exactly what a
missing `permissions: id-token: write` looks like — the variables are simply
absent rather than present-and-refused. So the failure names that, and names the
laptop alternative, instead of reporting an authorization error that did not
happen.
Discovery earns its keep beyond convenience: resolving `{issuer}/.well-known/
openid-configuration` also reads `grant_types_supported`, so asking a server for
a grant it does not implement fails by name, before the request, rather than as
a bare 400 from a URL nobody recognises.
`aws_sts` keeps a hand-rolled reader for three known tags rather than an XML
dependency. `http` is the vendor escape hatch — a URL, headers, a body, and JSON
pointers — with a deliberately tiny `{field}` substitution rather than a
template language: this text sits in a BUILD file next to a cache key, and every
extra capability is another thing that can differ between two machines.
Every message built here goes through the redactor, including an OAuth error
body, because a failed exchange can echo what it was given.
**The audit trail.** Every mint emits `SecretGranted`: which descriptor, for
which target, by which route, and how long the value lives. Never the value and
never the subject — a build log is not the place for either, and nothing an
incident actually needs is missing without them. Emitted per *consumer* rather
than per mint: the broker dedupes minting, but "who held it" is the question an
incident asks, and one line per descriptor would answer a different one.
The provider name comes from the recorded route rather than `acquire[0]`, so a
two-route descriptor reports whichever actually ran — which is the whole reason
the route is recorded — and is a stable string rather than a `Debug` rendering
that would change if a variant were renamed.
`testkit` grows `run_collecting_events`, because the stream is a consumer-facing
surface and what lands on it deserves assertions rather than trust.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RLvHrjGHVTciRH4DU3jcwR
CI hands a job an ambient OIDC identity; a laptop had none, so `oidc` was a
CI-only feature and a developer's alternative was the long-lived token in
`~/.zshrc` this whole design exists to remove. `heph auth login` closes that:
one sign-in against the org IdP produces an identity of the same *kind* CI
already has, so a single descriptor resolves in both places.
heph auth login # RFC 8252 loopback + PKCE-S256
heph auth login --device-code # RFC 8628, for a machine with no browser
heph auth status [--all] # what identity this machine has
heph auth logout [--all] # forget it here — not revoked at the IdP
The workspace commits two lines (`auth: {issuer, clientId}`) and nothing else;
it holds no secret, because a CLI is a public client and PKCE replaces the
client secret.
**Nothing in a build is ever interactive.** A build that opens a browser at
target 400 of 900 is an ambush for a human and a silent hang for an agent, so
the provider only ever presents an identity that already exists: ambient CI
identity first (it is scoped to the job, so it wins whenever both exist), then
the stored session refreshed non-interactively, then an immediate failure
naming `heph auth login`.
Storage: one refresh token, in a mode-0600 file under `$HOME/.heph/auth`,
written atomically. A file on **all three supported targets** rather than the
OS keychain — the user's call, per the per-platform rule. It is weaker at rest
than Keychain or libsecret, and it removes the case where *where a credential
lives* depends on whether the machine has a D-Bus session. One code path, one
failure mode, no new dependency; the trade is recorded rather than assumed.
Two things the design had to get right, both of which fail on the *next* build
rather than this one:
- **Rotation.** Most IdPs invalidate the refresh token they were presented, so
two builds refreshing at once would each store a token the other had already
spent. Refresh is single-flighted by an flock plus an in-process mutex, the
session is re-read *under* the lock, and `store` is a rename so an interrupt
between spending the old token and writing the new one cannot lock anyone out.
- **Audience.** Asking an IdP for an audience is not getting one; most ignore
it on a refresh grant. The `aud` claim that actually came back is checked
against what the descriptor asked for, so a mismatch fails here — where both
values can be shown — instead of at STS, which names itself and never the
audience.
Also wires up the background refresh sweep, which was implemented but never
started, so a long build cannot hand target 900 a credential minted for
target 1.
Review board: `compatibility` COMPATIBLE — no plugin ABI, cache key or on-disk
cache format is reached; both its MINORs are recorded in the code (the config
defaults are frozen behaviour; the session file carries no version field by
design). `product-vision` and `code-quality` findings are fixed, including the
circular dead end where every error said "run `heph auth login`" and that
command replied "already signed in", and the collapse of every network failure
into "your session was revoked".
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RLvHrjGHVTciRH4DU3jcwR
`Identity` carried `account`, `region`, `bucket` and `endpoint` as named
fields. Checked against what they actually do:
- `bucket` and `account` were read in exactly one line each — `interpolate()`
in the `oidc` provider — as `{bucket}` / `{account}` template substitutions.
Four lines later the same function iterates `params` and substitutes those
identically. They appeared in zero examples and zero tests. They contributed
nothing a named field is for.
- `region` and `endpoint` were read in one line each by the `aws_profile`
renderer, emitting `~/.aws/config` keys. Real work, but one vendor's config
vocabulary.
All four bought a schema surface welded to S3, frozen into every consumer's
cache key, that cost a release to extend — so an organization with an internal
IdP had no way to say what its identity was at all. That is the same objection
that moved `app_id`/`install`/`impersonate` into `params`; it was applied to
the new fields and never to the ones already there.
The rule now, stated in the type: a field is named only if a standard defines
it (`role`, `audience`, `scope`) or the collision check reasons about it
(`machine`, `registry`, `profile`, `env`). Everything else is `params`, hashed
exactly as before.
- region = "eu-west-1",
+ params = {"region": "eu-west-1"},
Behaviour is unchanged: `params` was already hashed and already interpolated,
so this moves no cache key except by moving the bytes. The cost is recorded
rather than assumed — a map has no schema, so a misspelled `regoin` renders no
region instead of failing.
`SECRET_JSON_VERSION` deliberately does not move. The layout changed before
anything shipped, and a bump would promise a migration for artifacts that never
existed; `the_emitted_bytes_are_frozen` is updated to the new layout, which is
the test doing its job.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RLvHrjGHVTciRH4DU3jcwR
…them
`heph auth login` read its issuer and client id from a workspace-level `auth:`
block in `.hephconfig`. That was wrong, and the reason is structural rather
than aesthetic: **a client id is registered per integration.** An organization
with one Okta tenant still registers a separate application for AWS, for GCP,
for each SaaS — so the issuer is shared and the client id is not, and a single
optional block can describe exactly one of them. Any workspace touching two
clouds needs two, which is most of them.
So the declaration moves to the thing that names the destination:
OKTA = "https://org.okta.com/oauth2/default"
secret(name = "ecr", role = "…", provider = "oidc",
sign_in = {"issuer": OKTA, "client_id": "aws-app"},
exchange = {"kind": "aws_sts"})
secret(name = "gar", role = "…", provider = "oidc",
sign_in = {"issuer": OKTA, "client_id": "gcp-app"},
exchange = {"kind": "token_exchange", "issuer": "…"})
`Source::Oidc` was an empty variant in a tagged union whose whole purpose is
that each variant carries its own configuration — its doc comment said "it has
no configuration of its own", which was only true because the configuration was
in another file. Filling it in also puts sign-in in the **acquisition half**:
unhashed, per `acquire` entry, so how a laptop signs in can differ from how CI
does without re-keying a consumer. Nested under `sign_in` rather than flat,
because `audience` and `scope` already exist on the identity and mean something
else.
`heph auth login` now signs in to every integration the workspace declares,
deduplicated by `(issuer, client_id)`. That reads like N browser logins and is
not: after the first flow the IdP session carries the rest through silently.
`--json` emits a list even for one, since a shape that varies with the count is
one every consumer must branch on.
Positioning corrected along with it: this is the **direct-federation** path.
Where an org does per-cloud SSO, `aws sso login` and `gcloud auth login` already
perform that OIDC flow and hold the session, and `provider = "exec"` consumes
it. `sign_in` is present only when federating to your own IdP — absent on most
secrets, which the per-secret placement handles and a config block could not.
Second half: `Provider::list_secrets`, so login does not resolve the graph.
Reaching the sign-ins through list+get means discovering every target, so
`plugin-go` would run `go list` to answer a question about credentials Go never
declares. **The default is `None`, not an empty list**, and that is the whole
safety of it: an empty list is a claim the host believes, so a provider with
secrets that had not implemented this would make them invisible to login and
fail later as a missing session nobody could explain. `None` says nothing and
the host enumerates properly; `Some(vec![])` is how a provider opts into being
fast and means it. An older plugin answers the unknown method id with an error,
which decodes to `None` — same fallback, no ABI break: the provider surface is a
frozen generic dispatch, so a new method id leaves the vtable untouched.
`crates/e2e/tests/secret_examples.rs` pins the property that makes the
optimization safe — the provider's answer and the graph's must agree, against a
workspace with the real shape: one issuer, one application per integration, two
secrets sharing one of them, one with no sign-in at all.
Recorded trade: `list_secrets` names a *driver* in an interface every provider
implements. The alternative considered was a general `list_by_driver`, one extra
`get` per match, serving runners and tests later. Directness won; the cost is
that the next such need adds a method rather than an argument.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RLvHrjGHVTciRH4DU3jcwR
…ience An adoption review of the finished feature found four claims the design made that the implementation did not honour. Nothing failed, and the suite was green throughout — which is the point: these are the class a passing test suite is worst at catching. **The exchange pipeline lived inside the `oidc` provider**, while `Acquire::validate` checked every step's endpoint for every source. So an `exec` source with an `exchange` parsed, validated, and then delivered the source's raw output — silently. Two of the four documented cases are exactly that shape: the Cloudflare R2 descriptor handed the target the 1Password *parent* token where the BUILD file promised a bucket-scoped 12h credential, and the non-federated GCP one handed over the service-account key rather than an access token. A build authenticated as something other than what it declared, with no error, no warning and no log line. The pipeline is now the broker's `Exchanger`, run after whatever any source returned; the type carries the rule, since every step takes a `Credential` and returns one. **`audience` was in the hashed identity half**, and the `aud` an assertion can carry is a property of whichever issuer minted it, not of the principal. GitHub Actions stamps any audience a job asks for, so the CI route needs `sts.amazonaws.com`; an OIDC ID token's `aud` is the client id, because OIDC Core §2 requires it, so the `heph auth login` route cannot ask for that and the cloud-side trust names the client id instead. One principal, two audiences — and while the field was hashed, declaring both split every consumer's cache key by environment. That is `pass_env`'s disease one level up, in the feature built to cure it. It moves to `Source::Oidc`, unhashed and per `acquire` entry. The BUILD spelling is unchanged. **Only GitHub Actions had a federated route.** GitLab, CircleCI, Buildkite, Kubernetes and Azure Pipelines had none, and the diagnostic told them about GitHub's `permissions: id-token: write`. Fixed with no new noun: `static_env` gains a `file`/`files` spelling — k8s projects a service account token to a path and Azure Pipelines points `$AZURE_FEDERATED_TOKEN_FILE` at one, neither reachable through a variable — and the exchange fix above does the rest. Adding a CI system is now a BUILD-file edit rather than a heph release. **Five first-run error messages named the `.hephconfig` `auth:` block** deleted in the commit before this one, sending a user (or an agent) to edit a file heph ignores. `no_auth_block` was dead and is gone. Also: `heph auth login`'s headless hint leads with SSH port forwarding and offers `--device-code` second, because the device grant is the flow an organization is most likely to have disabled — Microsoft's Conditional Access guidance is to block it and Okta ships it off. The fixed redirect ports are what makes forwarding work. `docs/SECRETS.md`'s IdP-admin section is corrected against vendor documentation on three further points: neither Okta nor Entra accepts a wildcard loopback port (Entra needs a manifest edit for `127.0.0.1` at all), and Okta's `/oauth2/default` is the licensed custom authorization server rather than the free org one. `crates/secrets` was missing from `qualityCrates`, so `lint` never fmt-checked it — caught by `tests/ci_gate.rs`, added here. Azure/Entra federation remains unreachable and is now documented as such: `client_credentials` resolves its subject token and never sends it, and `http` forces a JSON content-type where the token endpoint needs a form. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RLvHrjGHVTciRH4DU3jcwR
…ow` work `example/secrets/` — a package beside the other `example/` workspaces covering every case in `docs/SECRETS.md`: all three sources, the four helper wire protocols, every shape, every exchange grant, `allow`, transitive credentials, and the federated declarations for AWS, GCP, GitHub and R2. Built around `acquire` rather than around single-route declarations, because the single-route form teaches the wrong thing. `//secrets:api` is the whole idea in one target: two routes guarded on `DEMO_CI`, standing in for `GITHUB_ACTIONS` so a reader can flip between them on their own machine with no cloud, no network and no IdP. Both mint for real; `heph inspect hashin` is identical either way, which is the claim the feature rests on and is now something a reader can watch happen rather than take on trust. The README is explicit about the boundary: `static_env` and `exec` routes run, the federated ones parse and need a cloud. A published example that does not work is read as a promise, tried once, and then the whole document is distrusted — so `crates/e2e/tests/secret_examples.rs` now reads the shipped `BUILD` from the repository, parses and validates every `driver = "secret"` declaration in it (scraped, so a target added later is covered without anyone remembering), and runs the ones the README says to run. Nothing else in the tree walks `example/`. Writing it surfaced three defects, all in published examples: **`label()` in an `allow` denied everything.** `Matcher::matches_addr` answers `MatchShrug` for a label — a label set is not derivable from an address — and the policy check required `MatchYes`. So `allow = "//svc/... && label(deploy)"`, the exact form `docs/SECRETS.md` publishes, could never permit any target. It failed closed, which is the right direction to fail and still a broken feature: the author of the documented spelling got a flat refusal naming a query their target visibly satisfies. `check_allow` now evaluates against the consumer's labels, taken from the `TargetDef` the caller already holds — re-resolving the consumer's spec from inside its own resolution reports a cycle from the target to itself, speculative `rs` included. Anything still undecidable (`tree_output`) denies: a policy field is the wrong place to guess. **The query language has no `+`.** `docs/SECRETS.md` and the design doc both spelled a compound `allow` as `"//svc/... + label(deploy)"`, which is a parse error. The operators are `&&`, `||`, `!`. **There is no `secret()` builtin.** Two `docs/SECRETS.md` snippets used it; declarations are `target(driver = "secret", …)`. Also corrects the example's GCP route, which needs the pool provider as both the assertion audience and the `token_exchange` step's — one string, two different questions, and the step's would otherwise be absent now that it no longer falls back to a hashed identity field. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RLvHrjGHVTciRH4DU3jcwR
You were right that the federated declarations were inconsistent: `ecr` and `github` carried both routes while `gcp` and `r2` carried one, so the two that best show *why* the split matters were the two teaching it least. **`gcp` gains its laptop route**, and it is the sharpest illustration in the file: two hops in CI — RFC 8693 against the pool, then impersonation — and *zero* on a laptop, where `gcloud auth print-access-token` returns the end of the pipeline rather than the start. Both land on the same `gcloud_adc` and `docker_config` shapes, because `shape` is hashed identity and may not vary per route; what differs is what is inside the ADC file, and contents are exactly what is never hashed. **`r2` gains a CI route** reaching the same exchange: the parent token comes from a CI secret in one and a 1Password desktop session in the other, and neither lets it reach a target. Hoisting the API call into one `R2_MINT` constant makes the shared half literal rather than duplicated. **`ecr_anywhere` is gone.** Six routes in one target was a catalogue, not an example, and it made the multi-CI story look like the point when the point is one identity reached two ways. The GitLab / Kubernetes / Buildkite forms survive as a short comment pointing at `docs/SECRETS.md`, which keeps the fact that adding a CI system is a BUILD-file edit rather than a heph release. Every federated target now reads the same way — GitHub Actions, then the catch-all — so the pattern is visible by repetition instead of stated once. `the_example_still_covers_an_exchange_on_a_non_oidc_source` asserted on the first `acquire` entry; it now asserts on every entry, which is a stronger claim and the one the fix it guards actually makes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RLvHrjGHVTciRH4DU3jcwR
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.
A target names the credential it needs by address. The recipe is built, hashed and shared like any other dependency; the value is minted at run time, delivered as a file, redacted on the way out, and never persisted.
This is phase 1 of the design — the declaration and the broker. It is shippable on its own: it freezes the wire contract, which is the part that has to be right from day one. Consumer wiring (
secrets = {…}), sandbox delivery of the shapes, the transitiveSandboxfield,cache.subject_scoped, theoidcprovider andheph authfollow in their own PRs.Design doc:
docs/SECRETS.md.The one thing to understand
A
secret()declaration has two halves that behave completely differently:role,audience,scope,impersonate,app_id,install,account,region,bucket,endpoint,registry,machine,profile,shape,envprovider,var,vars,helper,protocol,runner,exchange,timeout,ttl,acquireIf
providerwere hashed, CI sayingoidcand a laptop sayingexecwould produce different hashouts and never share a cache entry for any consumer —pass_env's disease moved one level up.The split is structural, not a flag anyone has to remember. A consumer's
hashinfolds in its inputs' hashouts, so the driver writes only the identity half intosecret.jsonand the broker reads the acquisition half from the target's spec. A field that never becomes an artifact has no hashout to contribute. Reading the spec is also what keeps the collision and policy checks alive on a fully warm build, where every consumer is a hit and nothing executes.crates/e2e/tests/secret.rspins both directions on a real consumer'shashin.Three defects the review board caught
Each was reproduced before being fixed, and each has a regression test.
Two descriptors shared one cache key. Every
Identityfield is optional, so//creds:prodand//creds:staging— differing only in which env var they read — emitted byte-identical artifacts. A consumer of one was a cache hit on the other's, served output built with the wrong credential, with the winner decided by scheduling order. Measured at90e184a271c23ab8for both. The descriptor's address is now in the hashed bytes. Therunner.jsonprecedent deliberately does not transfer: a runner's config fully describes its environment, whereas asecret.jsonomits the half that decides which principal you get. Cost, stated rather than discovered: moving a descriptor re-keys its consumers.The redacting tee leaked at chunk boundaries. Holding back a partial prefix could truncate a longer match and let a shorter pattern win, so a chunk split emitted nine bytes of a live credential and masked it under the wrong name — and which name depended on where the pipe split. Matches straddling the hold region are now carried whole.
6–7 byte values were neither masked nor reported. The length filter ran per encoding, so a short value's base64 survived while its raw form — the form actually printed — went out verbatim, with no warning, in the crate whose entire job is preventing that.
Also from review
chrono(already a workspace dep, zero new crates in the lock). The hand-rolled version rolled Feb 30th forward three days, read a missing offset as UTC, and never checked separators — expiries wrong in the direction that strands a build mid-target, and succeeding, so the fallback warning never fired.credential_processno longer denies unknown fields: it is AWS's schema, emitted independently by three tools, and an added field must be ignored rather than break every mint.Identitykeepsdeny_unknown_fieldsbecause that one is ours and a skew is worth reporting.envpointers are normalized, so"$.","$"and"$.token"are one cache key rather than three.CancelledErrorfor the engine's downcast — every string that could carry a value is redacted at construction instead of the chain being flattened.Helper deadline
exechelpers get a 60s deadline, overridable withtimeoutper entry. Closing stdin only enforces half of "never interactive during a build": a macOS keychain dialog, a Touch ID prompt fromop, and a blocked network call read no stdin at all, and under the broker's per-descriptor lock every consumer queues behind the hang. The deadline arrives as a cancellation rather than a dropped future, so the helper is killed and reaped rather than orphaned.Tests
164 new tests.
lintexit 0;tstgreen except a pre-existingoci_dockerfailure on this machine (the OrbStack daemon socket does not exist — unrelated, and it never touches this path).Highlights worth reviewing:
hashinbyte-identical; changing the identity half moves itCompatibility
No proto or ABI change. The driver is compiled into the host and crosses no plugin seam.
secret.jsonis a new format at version 1, checked by exact match, and local-cache-only (a descriptor carries role ARNs and internal endpoints, and is trivially cheap to rebuild).Worth recording for the follow-up, because it corrects an assumption I had stated: a future
secretsfield onTargetDef/Sandboxis cold-path prost percrates/plugin-stabby/ABI_VERSIONING.md, where additive fields explicitly do not force anABI_SEMVERbump. What would: a native stabby struct, or changing an existing vtable symbol's signature. Decide the transport lane before writing it rather than bumping reflexively.Per-platform
No
cfgsplits, no intrinsics, nothing arch-conditional. A hanging helper was the one macOS-leaning hazard, and the deadline is uniform across all three supported targets.Known gap, called out rather than buried
The
ocirunner rewrites a helper's argv todocker execwith no-i, andSpecRewritecarries no stdin — so under it adocker_credentialorengflowhelper (the two protocols with a request payload) would read EOF and fail obscurely or return the wrong registry's credential, whilewrap-style runners are fine. Nothing resolves a runner yet in this PR, so it is latent; it must be either rejected or made a declared property of the runner seam before the wiring lands. Documented indocs/SECRETS.md.🤖 Generated with Claude Code
https://claude.ai/code/session_01RLvHrjGHVTciRH4DU3jcwR