fix(secrets): map DOCKERHUB_TOKEN to the scoped PAT, and detect registry/vault drift - #990
Conversation
|
Warning Review limit reached
Next review available in: 14 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. 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: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (10)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
5c73d51 to
74cc65d
Compare
…try/vault drift The registry names a Bitwarden item `dockerhub`; the vault holds `DockerHub`. Item lookup is an exact-name match, so the entry never resolved — and because `dotf secrets run` without `--only` resolves the whole registry and fails fast on the first bad entry, one stale name took down every unscoped run. That includes `dotf spec review`, whose launcher builds one, so the adversarial-review gate was unrunnable for every spec in every repo and two PRs sat blocked behind it. The only symptom was a review that produced an empty transcript. Reading the item that would not resolve turned up a second defect: the entry mapped `field: password` — the account password — while a scoped PAT sat beside it. Both authenticate with HTTP 200, which is why it stayed invisible. Now mapped to the PAT, with the evidence recorded so it is not reverted as a regression. The guard is a doctor section comparing the items the registry declares against the names the vault holds. Name-only: no field is read, no secret resolved, no value seen. It reports the missing item, every secret id naming it, and what breaks — because the symptom never points back here. Observed failing against the real vault, not only a fixture. The vault-side rename that clears the current instance is an operator action and is deliberately not in this PR: `dotf secrets backup` cannot run (bw export uses the locked CLI session, not the unlocked daemon), and a live password vault does not get mutated without a working escrow. Refs #985
74cc65d to
4d9ef0b
Compare
1 participant
Refs #985. Spec:
specs/BUG-080-registry-vault-drift/.Not a closing keyword: the operator action that clears the current instance (renaming the vault item
DockerHub→dockerhub) is not in this PR — see below.Why this was expensive
The registry names a Bitwarden item
dockerhub; the vault holdsDockerHub. Lookup is an exact-name match, so the entry never resolved. That would be minor if it stayed local, and it does not:dotf secrets runwithout--onlyresolves the whole registry and fails fast on the first bad entry — deliberately, so a child never launches with a partially-populated secret set (#612 A1).So one stale name took down every unscoped run:
pishell wrapperdotf spec review, whose launcher builds an unscoped run — confirmed from its own--dry-run, which prints'dotf' 'secrets' 'run' '--' 'pi' '--print' ...with no scopeWhich means the adversarial-review gate was unrunnable for every spec in every repo, and with it
dotf spec archive, and with itspec-gateon any PR closing a spec's issue. Two PRs were stuck behind it. The signal was worse than useless: the launcher printed[OK] Review running detachedover a process that had already died, and one session diagnosed a locked vault and was heading for a fix that would have changed nothing.What is here
1.
DOCKERHUB_TOKEN→field: PAT. Found while reading the item that would not resolve: the entry pointed at the account password while a scoped personal access token sat beside it in the same item. Verified live — both return HTTP 200 against the Hub API, which is exactly why it never surfaced. A blast-radius fix, not the repair of a broken credential, and the registry now carries that evidence so nobody reverts it as a regression.2. A doctor section asserting the registry→vault mapping. For every
backend: bwentry, the item it names must exist. Name-only — it lists item names through the daemon and compares sets, never reading a field, resolving a secret, or seeing a value. It reports the missing item, every secret id that named it, and what breaks, because the symptom never points back here.Evidence
Observed failing against the real vault, not only a fixture:
go build,go vet,go test ./...all green.What is deliberately NOT here
dotf secrets backupfails withVault is locked—bw exportuses the CLI's own session while the daemon holds a separate unlocked one. A live password vault does not get mutated without a working escrow, for a rename that takes 30 seconds in the UI. That tooling gap is a finding of its own.runtolerant of a secret the caller never asked for. The deeper fix, and a deliberate reversal of a settled design — a decision, not a bug fix. Until it is taken the next drift breaks the same things; this makes the drift visible before it does.Dotfiles/folder rename (refactor(secrets): drop the Dotfiles/ prefix from the folder taxonomy #982, another session).