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
12 changes: 6 additions & 6 deletions docs/audits/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ This directory holds DevStrap's chronological design & implementation audits. **

Currently-actionable findings, pass-scoped. Earlier passes (1–3) are largely implemented or superseded (see `spec/18_WORK_LOG.md` for the shipped history); the open backlog is concentrated in passes 4, 5, and 6.

> **2026-07-01 — Pass 6 landed.** The sixth pass ([`AUDIT_RECOMMENDATIONS_2026-07-01_PASS6.md`](AUDIT_RECOMMENDATIONS_2026-07-01_PASS6.md)) audited trunk `8c739b8` (PR #25) via a verification-driven nine-dimension workflow: 43 findings (P1=5, P2=25, P3=13), each adversarially verified against the code and checked for novelty against this backlog. Headlines: `P6-SEC-01`/`P6-SYNC-01` (the envelope layer still trusts the hub — unverified grant ingestion + whole-batch abort on one bad event), `P6-HUB-01`/`P6-DATA-01` (the now-live hub GC deletes live draft blobs), and `P6-GIT-01` (a universal 2-minute git timeout silently breaks eager materialization of large repos). This pass also **reconciled the ledger** below per convention #3: `P4-SEC-02` moved to *Recently shipped*, `P4-SEC-05` corrected to *partial* (its own finding is `P6-DOC-02`). **2026-07-02 update:** the hub-trust workstream (`P6-SYNC-01`, `P6-SEC-01`, `P6-SEC-02`; PRs #30–#34), `P6-DATA-01` (PR #35), `P6-HUB-01` (PR #36), and `P6-GIT-01` are shipped and moved to *Recently shipped* — **all five Pass 6 P1s are closed** (the AD-2 hardening-freeze P1 wave is complete); the open backlog is now the P2/P3 set below.
> **2026-07-01 — Pass 6 landed.** The sixth pass ([`AUDIT_RECOMMENDATIONS_2026-07-01_PASS6.md`](AUDIT_RECOMMENDATIONS_2026-07-01_PASS6.md)) audited trunk `8c739b8` (PR #25) via a verification-driven nine-dimension workflow: 43 findings (P1=5, P2=25, P3=13), each adversarially verified against the code and checked for novelty against this backlog. Headlines: `P6-SEC-01`/`P6-SYNC-01` (the envelope layer still trusts the hub — unverified grant ingestion + whole-batch abort on one bad event), `P6-HUB-01`/`P6-DATA-01` (the now-live hub GC deletes live draft blobs), and `P6-GIT-01` (a universal 2-minute git timeout silently breaks eager materialization of large repos). This pass also **reconciled the ledger** below per convention #3: `P4-SEC-02` moved to *Recently shipped*, `P4-SEC-05` corrected to *partial* (its own finding is `P6-DOC-02`). **2026-07-02 update:** the hub-trust workstream (`P6-SYNC-01`, `P6-SEC-01`, `P6-SEC-02`; PRs #30–#34), `P6-DATA-01` (PR #35), `P6-HUB-01` (PR #36), and `P6-GIT-01` are shipped and moved to *Recently shipped* — **all five Pass 6 P1s are closed** (the AD-2 hardening-freeze P1 wave is complete). **2026-07-03 update:** `P6-DATA-02` is shipped; the open backlog is now the remaining P2/P3 set below.

<!-- MD028 separator between adjacent dated blockquotes -->

Expand All @@ -45,21 +45,22 @@ Currently-actionable findings, pass-scoped. Earlier passes (1–3) are largely i
| P6-DATA-01 | P1 | PR #35 (2026-07-02) | Origin records its own `draft_snapshots` row in one transaction with the `draft.snapshot.created` event (`InsertLocalEventTx` + `RecordDraftSnapshotTx`), on both the create and revoke-rewrap paths; e2e-pinned by `draft_snapshot_gc_retains_origin.txtar`. |
| P6-HUB-01 | P1 | PR #36 (2026-07-02) | `hub gc` is sync-first (pre-GC pull+apply via `pullAndApplyEvents`, including blob caching), refuses to sweep on any truncated/skipped pull (`PullStats`), quarantined/cursor-held apply (`ApplyEventsWithStats`), or open quarantine conflict, and keeps unreferenced blobs younger than `--grace-window` (default 24h; `Hub.ListBlobs` now carries `LastModified`). Skew quarantines auto-resolve when their event later applies, so a clock hiccup cannot block gc forever. E2e-pinned by `hub_gc_stale_marks.txtar`. Follow-ups: signed retention marker (`P6-HUB-04`); sweep lock + dedup-`PutBlob` `LastModified` refresh, so gc racing a >window-late recovery sync cannot delete a just-re-referenced blob (`P4-HUB-12`); skipped-at-log-tail keeps gc refused until any newer event advances the cursor (`P6-SEC-03`'s class). |
| P6-GIT-01 | P1 | `fix/p6-git-01` (2026-07-02) | Git timeout split by command class: `Runner.LongTimeout` (default 30m, config `materialization.clone_timeout`, `gitRunner(opts)` at every CLI call site) applies per attempt to clone/fetch/LFS; a self-imposed deadline kill is the distinct terminal `ErrTimeout` (network exit code), ending the wipe-and-retry — a >2-minute blobless clone now completes. Pinned by the one-attempt/no-wipe fake-git tests + config round-trips. **Completes all five Pass 6 P1s (AD-2 P1 wave).** |
| P6-SYNC-03 | P2 | `fix/p6-sync-03` (2026-07-03) | Sticky fail-closed enrollment: `hasEnrolledDevices` counts `trust_state IN ('approved','revoked','lost')`, so revoking/losing the last approved device keeps verification fail-closed instead of reopening the pre-enrollment window; post-revoke traffic (revoked or unknown devices) quarantines per `P6-SYNC-01`. Pending placeholders still don't count; the never-enrolled bootstrap window (`P4-SEC-04`) is unchanged. Pinned by `TestHasEnrolledDevicesStickyAfterRevoke` + `TestApplyEventsRevokedLastDeviceStaysFailClosed`. Residual: synced `device.revoked` trust propagation. |
| P6-SYNC-03 | P2 | PR #38 (2026-07-03) | Sticky fail-closed enrollment: `hasEnrolledDevices` counts `trust_state IN ('approved','revoked','lost')`, so revoking/losing the last approved device keeps verification fail-closed instead of reopening the pre-enrollment window; post-revoke traffic (revoked or unknown devices) quarantines per `P6-SYNC-01`. Pending placeholders still don't count; the never-enrolled bootstrap window (`P4-SEC-04`) is unchanged. Pinned by `TestHasEnrolledDevicesStickyAfterRevoke` + `TestApplyEventsRevokedLastDeviceStaysFailClosed`. Residual: synced `device.revoked` trust propagation. |
| P6-DATA-02 | P2 | `fix/p6-data-02` (2026-07-03) | `ClearRotationForProject` now filters via `namespace_entries.env_profile_id` instead of the non-existent `env_profiles.namespace_id`; store coverage proves per-project isolation and CLI coverage proves one-arg `env rotate <path>` succeeds. |
| P6-DOC-02 | P2 | audit PR #28 | Ledger P4-SEC-05 contradiction + convention-#3 violation reconciled in the PR that landed the audit. Fully applied. |
| P6-DOC-03 | P3 | audit PR #28 | spec/00 re-drift (planned-sync comment, command/test inventories) fixed in the PR that landed the audit. Fully applied. |

### Pass 6 (2026-07-01) — 33 open of 43; **all five P1s shipped**; P2 quick-win wave underway

> The `P6-DOC-*` findings are documentation/gate accuracy fixes; their doc portions were applied in the same PR that landed this audit (spec/00, spec/13, spec/07/09/15 `tracks_code`, and this ledger). `P6-SYNC-01`, `P6-SEC-01`, `P6-SEC-02` (PRs #30–#34), `P6-DATA-01` (PR #35), `P6-HUB-01` (PR #36), `P6-GIT-01` (PR #37), and `P6-SYNC-03` moved to *Recently shipped* above per convention #3.
> The header count equals the rows in the table below (CodeRabbit, PR #39): 43 findings − 8 shipped − 2 fully-applied doc fixes (`P6-DOC-02`/`P6-DOC-03`, now in *Recently shipped*) = 33. `P6-DOC-01`/`P6-DOC-04` stay listed because their test-hardening residuals are open even though their doc portions were applied in the audit PR. `P6-SYNC-01`, `P6-SEC-01`, `P6-SEC-02` (PRs #30–#34), `P6-DATA-01` (PR #35), `P6-HUB-01` (PR #36), `P6-GIT-01` (PR #37), `P6-SYNC-03` (PR #38), and `P6-DATA-02` moved to *Recently shipped* above per convention #3.

| ID | Sev | Effort | Finding |
|---|---|---|---|
| P6-CLI-01 | P2 | S | Detect a root change on re-`init`; rewrite config.yaml or refuse |
| P6-CLI-02 | P2 | S | Gate `scan --adopt` on the scanned root matching the workspace root |
| P6-DATA-02 | P2 | S | Fix `ClearRotationForProject` to join through `namespace_entries` (`env rotate` always fails today) |
| P6-DATA-03 | P2 | M | Emit event + state mutation in one transaction at every emission site |
| P6-DATA-04 | P2 | M | Ship `db backup --full` (blobs + keys) and a restore path/runbook |
| P6-DOC-01 | P2 | S | Fix spec/13's stale status block; document `env rotate`; path-anchor the command gate _(doc portion applied this PR; command-gate test hardening open)_ |
| P6-DOC-02 | P2 | S | Reconcile the audit ledger's P4-SEC-05 contradiction + convention-#3 violation _(applied this PR)_ |
| P6-GIT-02 | P2 | S | Diff `agent` runs against the recorded base SHA, not just the working tree |
| P6-GIT-03 | P2 | S | Run dependency rebuild before `.env` hydrate; capture a 0600 log |
| P6-GIT-04 | P2 | M | Honor the stored `lfs_policy` on materialize/hydrate (mirror the worktree path) |
Expand All @@ -81,7 +82,6 @@ Currently-actionable findings, pass-scoped. Earlier passes (1–3) are largely i
| P6-CLI-05 | P3 | S | Document the shipped `r2://` hub path; stop steering users to the file hub |
| P6-DATA-05 | P3 | S | Add `idx_events_device_hlc` to serve the hot push/doctor query |
| P6-DATA-06 | P3 | S | Add a single-`local`-device partial unique index + race-tolerant `EnsureDevice` |
| P6-DOC-03 | P3 | S | Fix spec/00's re-drift (planned-sync comment, command/test inventories) _(applied this PR)_ |
| P6-DOC-04 | P3 | S | Add `internal/workspacekeys/**` to the spec/07/09/15 `tracks_code` frontmatter _(applied this PR; new-package mapping gate test open)_ |
| P6-GIT-06 | P3 | S | Gate `agent pr` on run status; reconcile crash-stuck `running` rows |
| P6-HUB-03 | P3 | S | Fan-out `R2Hub.Push` in HLC-ordered waves |
Expand Down
59 changes: 59 additions & 0 deletions internal/cli/env_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package cli
import (
"context"
"path/filepath"
"strings"
"testing"

"github.com/Reederey87/DevStrap/internal/state"
Expand Down Expand Up @@ -63,3 +64,61 @@ func TestEnvRotateAllClearsRotationFlag(t *testing.T) {
t.Fatalf("bindings needing rotation after env rotate --all = %d, want 0", remaining)
}
}

func TestEnvRotateProjectClearsRotationFlag(t *testing.T) {
home := filepath.Join(t.TempDir(), ".devstrap")
root := filepath.Join(t.TempDir(), "Code")
if _, stderr, err := executeForTest("--home", home, "--root", root, "init", "--workspace-name", "personal"); err != nil {
t.Fatalf("init stderr = %q err = %v", stderr, err)
}
ctx := context.Background()
store, err := state.Open(ctx, filepath.Join(home, "state.db"))
if err != nil {
t.Fatal(err)
}
if err := store.Migrate(); err != nil {
t.Fatal(err)
}
if _, err := store.UpsertProject(ctx, state.UpsertProjectParams{Path: "work/x", Type: "git_repo", RemoteKey: "github.com/acme/x", RemoteURL: "https://github.com/acme/x"}); err != nil {
t.Fatal(err)
}
proj, err := store.ProjectByPath(ctx, "work/x")
if err != nil {
t.Fatal(err)
}
if _, err := store.SaveCapturedEnvProfile(ctx, proj.ID, "default", []string{"API_KEY", "DB_URL"}, "age_blob:"+hex64a); err != nil {
t.Fatal(err)
}
flagged, err := store.MarkEncryptedBindingsNeedingRotation(ctx)
if err != nil {
t.Fatal(err)
}
if flagged != 2 {
t.Fatalf("flagged = %d, want 2", flagged)
}
closeStore(store)

stdout, stderr, err := executeForTest("--home", home, "--root", root, "env", "rotate", "work/x")
if err != nil {
t.Fatalf("env rotate stderr = %q err = %v", stderr, err)
}
if want := "Cleared the needs-rotation flag on 2 binding(s) for work/x"; !strings.Contains(stdout, want) {
t.Fatalf("stdout = %q, want %q", stdout, want)
}

store2, err := state.Open(ctx, filepath.Join(home, "state.db"))
if err != nil {
t.Fatal(err)
}
defer closeStore(store2)
if err := store2.Migrate(); err != nil {
t.Fatal(err)
}
remaining, err := store2.CountSecretBindingsNeedingRotation(ctx)
if err != nil {
t.Fatal(err)
}
if remaining != 0 {
t.Fatalf("bindings needing rotation after env rotate = %d, want 0", remaining)
}
}
5 changes: 4 additions & 1 deletion internal/state/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -1636,7 +1636,10 @@ func (s *Store) ClearRotationForProject(ctx context.Context, namespaceID string)
UPDATE secret_bindings
SET needs_rotation = 0, updated_at = ?
WHERE needs_rotation = 1
AND env_profile_id IN (SELECT id FROM env_profiles WHERE namespace_id = ?);
AND env_profile_id IN (
SELECT env_profile_id FROM namespace_entries
WHERE id = ? AND env_profile_id IS NOT NULL
);
`, timestampNow(), namespaceID)
if err != nil {
return 0, fmt.Errorf("clear rotation for project: %w", err)
Expand Down
79 changes: 79 additions & 0 deletions internal/state/store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1206,3 +1206,82 @@ func TestMarkEncryptedBindingsNeedingRotation(t *testing.T) {
t.Fatalf("second flag = %d, err = %v, want 0", again, err)
}
}

func TestClearRotationForProject(t *testing.T) {
ctx := context.Background()
st, err := Open(context.Background(), filepath.Join(t.TempDir(), "state.db"))
if err != nil {
t.Fatal(err)
}
defer st.Close()
if err := st.Migrate(); err != nil {
t.Fatal(err)
}
if err := st.EnsureWorkspace(ctx, "personal", "/tmp/Code"); err != nil {
t.Fatal(err)
}
if _, err := st.EnsureDevice(ctx, "test-device"); err != nil {
t.Fatal(err)
}
projectA, err := st.UpsertProject(ctx, UpsertProjectParams{
Path: "work/acme/api",
Type: "git_repo",
RemoteURL: "git@github.com:acme/api.git",
RemoteKey: "github.com/acme/api",
DefaultBranch: "main",
})
if err != nil {
t.Fatal(err)
}
projectB, err := st.UpsertProject(ctx, UpsertProjectParams{
Path: "work/acme/web",
Type: "git_repo",
RemoteURL: "git@github.com:acme/web.git",
RemoteKey: "github.com/acme/web",
DefaultBranch: "main",
})
if err != nil {
t.Fatal(err)
}
varsA := []string{"API_TOKEN", "DB_URL"}
varsB := []string{"API_TOKEN"}
if _, err := st.SaveCapturedEnvProfile(ctx, projectA.ID, "default", varsA, "age_blob:deadbeef"); err != nil {
t.Fatal(err)
}
if _, err := st.SaveCapturedEnvProfile(ctx, projectB.ID, "default", varsB, "age_blob:cafebabe"); err != nil {
t.Fatal(err)
}
flagged, err := st.MarkEncryptedBindingsNeedingRotation(ctx)
if err != nil {
t.Fatal(err)
}
if flagged != len(varsA)+len(varsB) {
t.Fatalf("flagged = %d, want %d", flagged, len(varsA)+len(varsB))
}

cleared, err := st.ClearRotationForProject(ctx, projectA.ID)
if err != nil {
t.Fatal(err)
}
if cleared != len(varsA) {
t.Fatalf("cleared = %d, want %d", cleared, len(varsA))
}
_, bindingsA, err := st.EnvProfileForProject(ctx, projectA.ID)
if err != nil {
t.Fatal(err)
}
for _, binding := range bindingsA {
if binding.NeedsRotation {
t.Fatalf("project A binding %s still flagged needs_rotation", binding.VarName)
}
}
_, bindingsB, err := st.EnvProfileForProject(ctx, projectB.ID)
if err != nil {
t.Fatal(err)
}
for _, binding := range bindingsB {
if !binding.NeedsRotation {
t.Fatalf("project B binding %s unexpectedly cleared needs_rotation", binding.VarName)
}
}
}
15 changes: 7 additions & 8 deletions spec/09_SECRETS_AND_ENVIRONMENT.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
---
last_reviewed: 2026-07-01
last_reviewed: 2026-07-03
tracks_code: [internal/childenv/**, internal/cli/devices.go, internal/cli/env.go, internal/cli/run.go, internal/devicekeys/**, internal/envbundle/**, internal/envfile/**, internal/platform/**, internal/workspacekeys/**]
---
# Secrets and Environment Design
Expand Down Expand Up @@ -216,7 +216,7 @@ Approval requires out-of-band fingerprint verification. The approving device sho

Device add, revoke, lost, or rotate events trigger re-encryption of affected bundles to the current approved-recipient set. Re-encryption removes future access to stored bundle ciphertext but does not make previously exposed secret values safe; revocation workflows must also mark affected values as requiring provider-side or service-side value rotation. At least one approved device must retain recoverable plaintext for every bundle before revocation completes.

`devstrap env rotate <path> <env-file>` re-captures and re-encrypts a profile to the current approved recipient set and clears its `needs_rotation` flags; `devstrap env rotate --all` clears flags workspace-wide. **Known defect (P6-DATA-02, open):** the one-argument flag-clear-only form (`devstrap env rotate <path>`) is currently broken — it filters on a phantom `env_profiles.namespace_id` column and fails with a SQL logic error; only `env rotate --all` is exercised today (see the P6-DATA-02 section below).
`devstrap env rotate <path> <env-file>` re-captures and re-encrypts a profile to the current approved recipient set and clears its `needs_rotation` flags; `devstrap env rotate <path>` clears the flags for that project without re-capturing, and `devstrap env rotate --all` clears flags workspace-wide. The per-project flag-clear path now joins through `namespace_entries.env_profile_id` (`P6-DATA-02`, shipped 2026-07-03) so it does not depend on a phantom `env_profiles.namespace_id` column.

## Secret redaction

Expand Down Expand Up @@ -373,14 +373,13 @@ if err := rebuildDependencies(ctx, dir); err != nil { /* logged to 0600 log */ }
if err := hydrateProjectEnv(ctx, project, dir); err != nil { return err }
```

### P6-DATA-02 — `ClearRotationForProject` filters on a non-existent `env_profiles.namespace_id`
### P6-DATA-02 — `ClearRotationForProject` filters on a non-existent `env_profiles.namespace_id` — **shipped (2026-07-03)**

**Problem.** The one-arg `devstrap env rotate <path>` (flag-clear-only) runs a subquery `SELECT id FROM env_profiles WHERE namespace_id = ?`, but `env_profiles` has no `namespace_id` column (`internal/state/store.go:1632-1637`); the link is `namespace_entries.env_profile_id`. Every invocation fails with `no such column: namespace_id` → "clear rotation for project: SQL logic error"; only `env rotate --all` is tested.
**Was.** The one-arg `devstrap env rotate <path>` (flag-clear-only) ran a subquery `SELECT id FROM env_profiles WHERE namespace_id = ?`, but `env_profiles` has no `namespace_id` column; the link is `namespace_entries.env_profile_id`. Every invocation failed with `no such column: namespace_id` → "clear rotation for project: SQL logic error"; only `env rotate --all` was tested.

**Actionable steps.**
1. Join through `namespace_entries` instead of the phantom column.
2. Add a per-project store test (capture → `MarkEncryptedBindingsNeedingRotation` → `ClearRotationForProject` → assert cleared).
3. Add a CI lint that `db.Prepare`s every static query in `store.go` against a migrated in-memory DB.
**Shipped fix.** `ClearRotationForProject` now filters `secret_bindings` by `namespace_entries.env_profile_id` for the requested namespace entry. `TestClearRotationForProject` covers two projects with encrypted profiles and proves clearing one project leaves the other flagged; `TestEnvRotateProjectClearsRotationFlag` covers the one-arg CLI form and its success message.

**Remaining follow-up.** Add a CI lint that `db.Prepare`s every static query in `store.go` against a migrated in-memory DB.

**Example.**
```sql
Expand Down
Loading
Loading