Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
83 changes: 83 additions & 0 deletions cli/internal/doctor/checks_bw_mapping.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
package doctor

import (
"fmt"
"sort"
"strings"

"github.com/mlorentedev/dotfiles/cli/internal/secrets"
)

// checkBWMapping asserts that every Bitwarden item the registry names actually
// exists in the vault — the drift between the mapping SSOT and the store it maps
// into.
//
// It exists because that drift is not a cosmetic mismatch: `dotf secrets run`
// resolves the WHOLE registry when the caller passes no --only and fails fast on
// the first unresolvable entry, by design (a child must never launch with a
// partially-populated secret set). So one stale item name takes down every
// unscoped run — which is the `pi` shell wrapper and, worse, `dotf spec review`,
// whose launcher builds an unscoped run. On 2026-08-15 a single entry naming
// `dockerhub` while the vault held `DockerHub` made the adversarial-review gate
// unrunnable for every spec in every repo, and the only symptom anyone saw was a
// review that produced an empty transcript.
//
// The check is name-only: it lists item names through the daemon and compares
// sets. It never reads a field, never resolves a secret, and never sees a value —
// so it stays cheap enough for the full sweep and safe to run anywhere.
//
// Severity mirrors checkBitwardenReach's rule: an unreachable or locked vault is
// not a finding here (that section owns it), but a reachable vault missing an
// item a live entry depends on is a FAIL, because something is already broken.
func checkBWMapping(sys *System, cfg *Config, rep *Report) {
rep.Section("Bitwarden mapping (registry -> vault)")

reg, err := loadRegistry(cfg)
if err != nil {
rep.Skip("secrets/registry.yaml not readable — checkSecrets owns that failure")
return
}

declared := map[string][]string{} // item name -> secret ids naming it
for i := range reg.Secrets {
s := &reg.Secrets[i]
if s.Backend != secrets.BackendBW || s.BW == nil || s.BW.Item == "" {
continue
}
declared[s.BW.Item] = append(declared[s.BW.Item], s.ID)
}
if len(declared) == 0 {
rep.Skip("no bw-backed secrets in the registry")
return
}

present, err := sys.BWItemNames()
if err != nil {
// Locked, absent daemon, transport error: not this section's finding.
rep.Skip(fmt.Sprintf("vault item list unavailable (%v) — mapping unverifiable", err))
return
}
have := make(map[string]bool, len(present))
for _, n := range present {
have[n] = true
}

missing := make([]string, 0, len(declared))
for item := range declared {
if !have[item] {
missing = append(missing, item)
}
}
sort.Strings(missing)

for _, item := range missing {
ids := declared[item]
sort.Strings(ids)
rep.Fail(fmt.Sprintf(
"%s: no such item in the vault, named by %s — every `dotf secrets run` without --only fails on it, including `dotf spec review`",
item, strings.Join(ids, ", ")))
}
if len(missing) == 0 {
rep.Pass(fmt.Sprintf("all %d bw item(s) named by the registry exist in the vault", len(declared)))
}
}
113 changes: 113 additions & 0 deletions cli/internal/doctor/checks_bw_mapping_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
package doctor

import (
"bytes"
"errors"
"strings"
"testing"
)

// The exact live state that took the archive gate down on 2026-08-15: the
// registry names `dockerhub`, the vault holds `DockerHub`. Nothing resolved it
// because item lookup is an exact-name match, and nothing reported it because no
// check compared the two sets — the only symptom was `dotf spec review`
// producing an empty transcript.
func TestCheckBWMapping_MissingItemFails(t *testing.T) {
registry := "version: 1\nsecrets:\n" +
" - {id: DOCKERHUB_TOKEN, plane: app, backend: bw, bw: {item: dockerhub, field: PAT}, expose: {env: DOCKERHUB_TOKEN}}\n" +
" - {id: NAN_API_KEY, plane: app, backend: bw, bw: {item: nan-api-key, field: api-key}, expose: {env: NAN_API_KEY}}\n"

sys := newSys(nil, nil, nil)
sys.BWItemNames = func() ([]string, error) {
return []string{"DockerHub", "nan-api-key", "Stripe"}, nil
}

var buf bytes.Buffer
rep := capture(&buf)
checkBWMapping(sys, patCfg(t, registry), rep)

if rep.Failures() != 1 {
t.Fatalf("one declared item is absent from the vault; want 1 failure, got %d\n%s", rep.Failures(), buf.String())
}
out := buf.String()
// The message has to be actionable: which item, and which secret named it.
for _, want := range []string{"dockerhub", "DOCKERHUB_TOKEN"} {
if !strings.Contains(out, want) {
t.Errorf("output must name %q\n%s", want, out)
}
}
// And it must state the consequence, because the symptom never points here.
if !strings.Contains(out, "without --only") {
t.Errorf("output must say what breaks, not just what mismatches\n%s", out)
}
// The item that DOES exist must not be reported.
if strings.Contains(out, "nan-api-key: no such item") {
t.Errorf("a present item must not be reported missing\n%s", out)
}
}

// Every declared item present is the healthy case: one PASS, no failures, and no
// per-item noise in a section that is clean.
func TestCheckBWMapping_AllPresentPasses(t *testing.T) {
registry := "version: 1\nsecrets:\n" +
" - {id: NAN_API_KEY, plane: app, backend: bw, bw: {item: nan-api-key, field: api-key}, expose: {env: NAN_API_KEY}}\n"

sys := newSys(nil, nil, nil)
sys.BWItemNames = func() ([]string, error) { return []string{"nan-api-key", "unrelated"}, nil }

var buf bytes.Buffer
rep := capture(&buf)
checkBWMapping(sys, patCfg(t, registry), rep)

if rep.Failures() != 0 {
t.Fatalf("all items present; want 0 failures, got %d\n%s", rep.Failures(), buf.String())
}
if !strings.Contains(buf.String(), "exist in the vault") {
t.Errorf("expected the clean-state PASS\n%s", buf.String())
}
}

// A locked or unreachable vault is not this section's finding — checkBitwardenReach
// owns that severity. Reporting it twice inflates the failure count and trains the
// reader to ignore the section that is actually specific.
func TestCheckBWMapping_UnavailableVaultSkips(t *testing.T) {
registry := "version: 1\nsecrets:\n" +
" - {id: NAN_API_KEY, plane: app, backend: bw, bw: {item: nan-api-key, field: api-key}, expose: {env: NAN_API_KEY}}\n"

sys := newSys(nil, nil, nil)
sys.BWItemNames = func() ([]string, error) { return nil, errors.New("vault is locked") }

var buf bytes.Buffer
rep := capture(&buf)
checkBWMapping(sys, patCfg(t, registry), rep)

if rep.Failures() != 0 {
t.Fatalf("an unavailable vault is not a mapping failure; got %d\n%s", rep.Failures(), buf.String())
}
if !strings.Contains(buf.String(), "mapping unverifiable") {
t.Errorf("expected the unverifiable SKIP\n%s", buf.String())
}
}

// An age-only registry has nothing to compare: SKIP, never a PASS implying the
// mapping was checked. "Nothing to check" and "checked, all good" are different
// statements and only one of them is evidence.
func TestCheckBWMapping_NoBwSecretsSkips(t *testing.T) {
registry := "version: 1\nsecrets:\n" +
" - {id: SSH_KEY, plane: floor, backend: age-offline, age: ssh.key, expose: {env: SSH_KEY}}\n"

sys := newSys(nil, nil, nil)
var called bool
sys.BWItemNames = func() ([]string, error) { called = true; return nil, nil }

var buf bytes.Buffer
rep := capture(&buf)
checkBWMapping(sys, patCfg(t, registry), rep)

if called {
t.Error("must not touch the vault when no bw-backed secret is declared")
}
if rep.Failures() != 0 || !strings.Contains(buf.String(), "no bw-backed secrets") {
t.Errorf("expected the no-bw-secrets SKIP\n%s", buf.String())
}
}
1 change: 1 addition & 0 deletions cli/internal/doctor/doctor.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ func Run(opts Options) (int, error) {
checkSecretsTooling(sys, rep)
checkBitwardenReach(sys, rep)
checkBWServeDaemon(sys, rep)
checkBWMapping(sys, cfg, rep)
checkDisasterRecovery(sys, cfg, rep)
checkPATExpiry(sys, cfg, rep)
checkGuardHooks(sys, cfg, rep, opts.Fix)
Expand Down
9 changes: 9 additions & 0 deletions cli/internal/doctor/system.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,12 @@ type System struct {
// it for one Authorization header. Returns secrets.ErrSecretAbsent (wrapped)
// when the secret is genuinely not provisioned on this machine.
ResolveSecret func(e secrets.Entry) (string, error)
// BWItemNames lists every item name in the Bitwarden vault — names only, no
// fields and no values. It exists so the registry->vault mapping can be
// asserted without resolving a secret: a registry entry naming an item the
// vault does not have takes down every unscoped `dotf secrets run`, and the
// only symptom is whatever that run was driving (BUG-080).
BWItemNames func() ([]string, error)
}

// resolveSecret is the production ResolveSecret: the age store (checkout-first,
Expand Down Expand Up @@ -185,6 +191,9 @@ func realSystem() *System {
return (&secrets.BWServeDaemon{Client: secrets.BWServeClient{}}).Status()
},
ResolveSecret: resolveSecret,
BWItemNames: func() ([]string, error) {
return secrets.BWServeReader{Client: secrets.BWServeClient{}}.ItemNames()
},
CommandOutputBounded: func(d time.Duration, name string, args ...string) (string, string, error) {
ctx, cancel := context.WithTimeout(context.Background(), d)
defer cancel()
Expand Down
3 changes: 3 additions & 0 deletions cli/internal/doctor/testhelpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,9 @@ func newSys(env map[string]string, onPath []string, cmdOut map[string]string) *S
// default (no token in the environment ⇒ SKIP), so full-sweep tests that
// do not care about PATs behave as they always did. Tests exercising
// resolution inject their own.
// Default: the vault holds exactly what the registry declares, so the
// mapping check is quiet unless a test deliberately introduces drift.
BWItemNames: func() ([]string, error) { return nil, errors.New("no vault in tests") },
ResolveSecret: func(e secrets.Entry) (string, error) {
return "", fmt.Errorf("%w: %s", secrets.ErrSecretAbsent, e.Var)
},
Expand Down
25 changes: 25 additions & 0 deletions cli/internal/secrets/bwserve.go
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,31 @@ type bwServeListData struct {
Data []bwServeListItem `json:"data"`
}

// ItemNames lists every item name in the vault, for callers that need to compare
// the registry's declared items against what the store actually holds rather than
// resolve any of them. It is the one read here that touches no field and returns
// no value — only names — so a health check can assert the mapping without ever
// holding a secret.
//
// It deliberately does NOT use the ?search= filter: the point is the whole set,
// and search is a fuzzy substring match (see getItemJSON), which would make an
// absent item indistinguishable from one whose name merely failed to match.
func (r BWServeReader) ItemNames() ([]string, error) {
data, err := r.Client.call(http.MethodGet, "/list/object/items", nil)
if err != nil {
return nil, fmt.Errorf("bw serve list items: %w", err)
}
var list bwServeListData
if err := json.Unmarshal(data, &list); err != nil {
return nil, fmt.Errorf("bw serve list items: unparseable data: %w", err)
}
names := make([]string, 0, len(list.Data))
for _, it := range list.Data {
names = append(names, it.Name)
}
return names, nil
}

func (r BWServeReader) getItemJSON(item string) ([]byte, error) {
data, err := r.Client.call(http.MethodGet, "/list/object/items?search="+url.QueryEscape(item), nil)
if err != nil {
Expand Down
8 changes: 6 additions & 2 deletions secrets/registry.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,15 @@ secrets:
rotate: 90d
validate: github-token # sync ci probes `gh api user` before upload (#635 follow-up)

# DockerHub — login pair, one bw item: token→password, username→username.
# DockerHub — one bw item: token→PAT (a scoped token), username→username.
- id: DOCKERHUB_TOKEN
plane: app
backend: bw
bw: { item: dockerhub, field: password, folder: apps }
# PAT, not password: the item carries both, and the account password
# authenticates against the Hub API exactly as the scoped token does — which is
# why mapping the wrong one stayed invisible. Verified 2026-08-15: both return
# HTTP 200, so this is a blast-radius fix, not a repair of a broken credential.
bw: { item: dockerhub, field: PAT, folder: apps }
expose: { env: DOCKERHUB_TOKEN }
consumers: ["ci:image-push"]
rotate: 180d
Expand Down
Loading
Loading