You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Copy file name to clipboardExpand all lines: .claude/commands/hort-architect.md
+5-2Lines changed: 5 additions & 2 deletions
Display the source diff
Display the rich diff
Original file line number
Diff line number
Diff line change
@@ -367,7 +367,7 @@ These invariants must hold in every implementation that touches artifact state:
367
367
368
368
1.**Downloads are blocked while `quarantine_status = 'quarantined'`**, regardless of `quarantine_until`. The status field is the gate; the timestamp is the expiry signal for the background sweep.
369
369
370
-
2. **A clean scan result does NOT release an artifact early, and a *missing* scan does NOT release it at all on a timer (ADR 0007).** `ScanCompleted(clean)` leaves `quarantine_status = 'quarantined'` and `quarantine_until` unchanged. The background sweep transitions to `released` only when **`quarantine_until <= now()` AND** the application layer can supply a release authority — a successful `ScanCompleted` on the artifact stream (`ReleaseAuthorization::ScanSucceeded`) **or** the resolved `ScanPolicy` declares `scan_backends: []` (`ScanWaived`) **or** an admin override (`AdminOverride`) **or** a curator waiver (`CuratorWaiver`) **or** post-exclusion policy re-evaluation (`PolicyReEvaluation`). The release predicate accepts exactly these five authorities; every other `(reason, authority)` pair is denied. A scan job that exhausts retries transitions the artifact to the terminal `scan_indeterminate` status (event `ScanIndeterminate`), which is non-downloadable and non-promotable and is *not* releasable by a timer alone — only by admin override or post-exclusion policy re-evaluation (curator-waive is intentionally narrower — `Quarantined` only). `quarantine_until <= now()` is the sweep's candidacy filter, never a release authority.
370
+
2. **A clean scan result does NOT release an artifact early, and a *missing* scan does NOT release it at all on a timer (ADR 0007).** `ScanCompleted(clean)` leaves `quarantine_status = 'quarantined'` and `quarantine_until` unchanged. The background sweep transitions to `released` only when **`quarantine_until <= now()` AND** the application layer can supply a release authority — a successful `ScanCompleted` on the artifact stream (`ReleaseAuthorization::ScanSucceeded`) **or** the resolved `ScanPolicy` declares `scan_backends: []` (`ScanWaived`) **or** an admin override (`AdminOverride`) **or** a curator waiver (`CuratorWaiver`) **or** post-exclusion policy re-evaluation (`PolicyReEvaluation`). The release predicate accepts exactly these five authorities; every other `(reason, authority)` pair is denied. A scan job that exhausts retries **fails closed by status** (issue #6, ADR 0007 amendment): if the artifact is still `quarantined` (mid-observation-window), it **stays `quarantined`** — no `ScanIndeterminate` event, no status UPDATE — and the failed `jobs` row is the signal that `RescanCandidatesRepository::select_stranded` + `CronRescanTickHandler` use to re-scan it once the scanner recovers (it heals via the *existing* `ScanSucceeded` authority, never a timer); if the artifact is any other status (`None` — permissive default with no window to fall back into — or already-terminal), it transitions to the terminal `scan_indeterminate` status (event `ScanIndeterminate`), non-downloadable and non-promotable, releasable only by admin override or post-exclusion policy re-evaluation (curator-waive is intentionally narrower — `Quarantined` only). Both branches are fail-closed: a `quarantined` or `scan_indeterminate` artifact that never had a successful `ScanCompleted` is *never* releasable by a timer alone. `quarantine_until <= now()` is the sweep's candidacy filter, never a release authority.
371
371
372
372
3.**`ScanCompleted(findings)` immediately sets `quarantine_status = 'rejected'`.** Time does not reverse this — `quarantine_until` expiry has no effect on `rejected` status. **`rejected` is terminal under the release surfaces: its only exit is exclusion re-evaluation** (third mechanism below). `admin_release` and curator-waive act on the *non-terminal* held states and do **not** reverse `rejected` — both go through the same source-state guard, so attempting either on a `rejected` (or `released`/`None`) artifact returns **`409 Conflict`** (`DomainError::InvalidState`, ADR 0025), not a release. The three mechanisms that change a held/blocked artifact's state, each per-artifact with attribution + justification:
373
373
@@ -510,7 +510,7 @@ Use this during review. Every item is a hard block unless explicitly justified.
510
510
-[ ]**SQL in a domain entity** — domain entities must not import `sqlx`. Move queries to repository adapters.
511
511
-[ ]**Caller-supplied storage key** — storage writes must go through `put(stream) → ContentHash`. No logical path keys to storage.
512
512
-[ ]**State mutation via direct DB update bypassing event log** — all artifact state changes must emit a domain event AND persist state. No silent UPDATE without a corresponding event.
513
-
-[ ]**Scanner clean → immediate release** — `ScanCompleted(clean)` must not clear `quarantine_until` or set `quarantine_status = 'released'`. See quarantine invariants. The fail-closed release predicate *strengthens* this: not only must a clean scan not release early, a never-successfully-scanned artifact must not release on `quarantine_until` expiry at all (the release predicate requires `ScanSucceeded ∨ ScanWaived ∨ admin override`; terminal scan failure → `scan_indeterminate`). See ADR 0007.
513
+
-[ ]**Scanner clean → immediate release** — `ScanCompleted(clean)` must not clear `quarantine_until` or set `quarantine_status = 'released'`. See quarantine invariants. The fail-closed release predicate *strengthens* this: not only must a clean scan not release early, a never-successfully-scanned artifact must not release on `quarantine_until` expiry at all (the release predicate requires `ScanSucceeded ∨ ScanWaived ∨ admin override`; on retry-exhaustion the artifact fails closed by status — stays `quarantined` if mid-window and is auto-rescanned, else → terminal `scan_indeterminate`; issue #6). See ADR 0007 → *Scan-execution failure vs ambiguous result*.
514
514
-[ ]**409 for quarantined artifact in proxy path** — the proxy path must return 503 + Retry-After. 409 is for explicit conflicts, not temporary holds.
515
515
-[ ]**Flat WIT interface for OCI or Git LFS** — stateful upload protocols cannot be modelled with a request/response Core interface. They require `StatefulUpload` group or a compiled-in adapter.
516
516
-[ ]**Per-handler auth code** — format handlers must not contain `authenticate()` or `extract_basic_credentials()`. Auth lives in middleware.
@@ -540,6 +540,7 @@ Use this during review. Every item is a hard block unless explicitly justified.
540
540
-[ ]**`ServiceAccount` with empty `federatedIdentities[].claims`** — apply-time validation rejects this; if a code path accepts an envelope with an empty `claims` map, that's a bug. Empty claims = "any JWT from this issuer can assume me" — a privilege-escalation footgun on a misconfigured issuer. (see ADR 0018)
541
541
- [ ] **Policy field accepted at apply, inert at runtime** — a new policy field (anything on `PrefetchPolicy`, `ScanPolicy`, `RetentionPolicy`, `RepositoryUpstreamMapping`, etc.) must be either *enforced* by the consuming use case or *not surfaced* in the apply path. Accepting the field at gitops apply while the consumer silently ignores it is a hard block — operators set risk-significant values (e.g. `max_age_days: 90`) and make threat-model decisions on the assumption the field is load-bearing; an inert field is a silent footgun. The canonical precedent is: the operator surface was *removed* until the feature was functional. The `max_age_days` field is the canonical exemplar (apply-time linter rejects any non-`None` value until the per-version timestamp surface ships). A new field landing in this shape must either (a) ship its consumer in the same PR, or (b) ship an apply-time rejection that points the operator at the future enforcement initiative. Aspirational acceptance is the failure mode this rule prevents. (see ADR 0015)
542
542
- [ ] **Cross-opt-in collapse of a Gate-2-style invariant** — any new operator-opt-in that lets untrusted input influence the release-gate computation (`trust_upstream_publish_time`-shaped: lets an upstream-asserted value flow into the quarantine deadline; `scan_backends:[]`-shaped: waives a release-authority requirement; `IndexMode`-shaped: changes what version states are advertised in indices) must enumerate its interaction with every other such opt-in in the design doc *before* implementation. The canonical exemplar: the `trust_upstream_publish_time = true` × `scan_backends: []` combination — each individually documented as a bounded opt-in — collapses the Gate-2 observation window to ≤ sweep-tick latency when set on overlapping scopes (apply-time linter rejects the combination). The structural close is fail-closed apply-time rejection of the dangerous combination, never a runtime "fallback to a degraded authority" path (that re-introduces the collapse with an escape hatch). See the Cross-opt-in interaction matrix below. (see ADR 0016)
543
+
- [ ] **Dependency-graph change without regenerating third-party attribution** — ANY change to the compiled dependency graph (a version bump, `cargo update -p`/`-w`, adding/removing a dep or a feature that pulls crates, any `Cargo.lock` delta that adds/removes/re-versions a **non-workspace** crate) MUST regenerate `THIRD-PARTY-LICENSES.{md,json}` (`scripts/regenerate-attribution.sh`, in the sandbox) and commit them in the **same** change/MR. A release **workspace-version-only** bump (`X.Y.Z-dev → X.Y.Z-<pre>`) is exempt (no third-party crate changes — confirm via `check-attribution`). This is authoring-time discipline, load-bearing because `security:attribution-sync` was rescoped to **release-relevant pipelines only** (#30) and no longer runs per-MR — the CI gate is the fail-closed release backstop, not the first-line check. Canonical failure: the `spin 0.9.8 → 0.9.9` bump (#28, `994e88ee`) merged to `develop` with stale attribution and sat latent until the `v0.9.9-beta.4` tag pipeline failed on it (#33). Verify: any `Cargo.lock` non-workspace-crate delta in the diff is accompanied by a `THIRD-PARTY-LICENSES.{md,json}` regeneration. (see ADR 0049; ADR 0047)
543
544
-[ ]**Operator-config naming hazard***(review-only — not structurally enforced)* — enum variants whose names suggest the *opposite* of their behaviour (a more-permissive variant named more-strictly than a more-strict variant, or vice versa) are caught at design-doc review. The canonical exemplar: `IndexMode::FilterQuarantined` retained MORE versions (added the `Unknown`/upstream-advertised set) than `IndexMode::ReleasedOnly`; an operator reaching for "more conservative" via `FilterQuarantined` got the *more permissive* view. The data-leak axis was already covered (`NonServableStatusFilter` runs first in either mode), but the operator-UX axis is the real failure — risk decisions made on a misread name (fixed by in-place rename `FilterQuarantined → IncludePending` per pre-v1.0 discipline). Pre-v1.0 the fix is rename; post-v1.0 it becomes a deprecation cycle, so the check belongs at design-doc-review time, not implementation-review time. (see ADR 0015)
544
545
545
546
#### Cross-opt-in interaction matrix (ADR 0016)
@@ -638,6 +639,7 @@ Read these before making architectural decisions:
638
639
|`docs/adr/0026-streaming-metadata-projection.md`| Streaming metadata projection (no whole-body buffering on pull-through) |
|`docs/adr/0028-destructive-task-idempotency.md`| Durable per-UTC-day idempotency key for destructive task kinds |
642
+
|`docs/adr/0049-regenerate-attribution-on-any-dependency-change.md`| Any dependency-graph change regenerates `THIRD-PARTY-LICENSES.{md,json}` in the same change/MR; authoring-time discipline is first-line because `attribution-sync` is release-scoped (#30); release-workspace-version-only bumps exempt |
641
643
|`docs/adr/0036-oci-auth-capability-token.md`| OCI `/v2/auth` is a per-identity capability token: authority = `User`-subject grants ∩ cap, no ambient admin; the B1 fail-closed Pat/SA cap backstop (OIDC/CliSession `None`-cap untouched); admin off the OCI surface |
642
644
|`docs/adr/0037-gitops-service-account-grant.md`| gitops `PermissionGrant` may target a ServiceAccount by name: `GrantSubjectSpec::ServiceAccount { name }` resolves at apply to `GrantSubject::User(backing_user_id)`; domain `GrantSubject` taxonomy unchanged (ADR 0012 not reopened) |
643
645
|`docs/adr/0038-admin-identity-model.md`| Admin-identity model: human admin is IdP-assumed (OIDC → CliSession via a group→`admin` ClaimMapping); service accounts strictly non-admin (`issue-svc-token` rejects `--permission=admin`); the DSN-gated `bootstrap-session` is the only no-IdP/first-admin admin path; `task:destructive`-as-claim kept; Dex `staticPasswords` emit no `groups`|
@@ -682,6 +684,7 @@ When reviewing an implementation (Step 5):
682
684
-[ ]**DB-backed test parallel-safety:** any new `hort-adapters-postgres` test that acquires a real connection (calls `maybe_pool()` / touches the shared DB) carries the crate-wide `#[serial(hort_pg_db)]` key (or an equivalent per-test isolation mechanism). The suite runs in parallel against one shared DB with no isolation and production is single-flight by design; a DB-gated test without the key silently reintroduces the identity-shifting flake fixed in `ed79360a`. Coverage % passing is **not** sufficient. There is no compile-time/lint enforcement — this is a mandatory manual check. (See CLAUDE.md → Test Coverage Tiers → DB-backed test isolation.)
683
685
-[ ] Pre-existing tests still pass (`cargo test --workspace --lib`)
684
686
-[ ]**`cargo audit --deny warnings` was actually run and is clean** — unconditionally, regardless of whether the change touched `Cargo.toml`/`Cargo.lock` (live RustSec DB → the blocking CI security gate can be red with no dep change). A "no dependency change, audit not required" inference is a hard block — the gate's pass must come from the command's output, not a heuristic. See CLAUDE.md → Pre-push Quality Checklist.
687
+
-[ ]**Third-party attribution regenerated if the dependency graph changed** — any `Cargo.lock` delta adding/removing/re-versioning a **non-workspace** crate (a bump, `cargo update`, add/remove dep or feature) is accompanied by a `THIRD-PARTY-LICENSES.{md,json}` regeneration (`scripts/regenerate-attribution.sh`) in the **same** change. `security:attribution-sync` is release-scoped (#30) and will NOT catch this per-MR — it is the release backstop, so the authoring/review check is the first line. A release workspace-version-only bump is exempt. Hard block. (see ADR 0049)
685
688
-[ ] E2E smoke tests pass (`./scripts/native-tests/run.sh --hort=compose`) if the component is on a tested path
686
689
-[ ] No structural duplication (3+ similar blocks without a shared helper)
687
690
-[ ] Migration number does not collide (`ls migrations/ | tail -5`)
0 commit comments