From 99743af1828bdb03c4309b0bfc82783eb5fc89bc Mon Sep 17 00:00:00 2001 From: mathias Date: Wed, 3 Jun 2026 21:38:49 +0000 Subject: [PATCH] docs: add ADR-017 (Dex-write invite flow) retroactively; annotate ADR-002/013/014 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reconciliation pass after parallel agent sessions shipped v0.6.0/v0.7.0. ADR-017 documents the v0.7.0 invite flow, which gave Tapir scoped create+get on passwords.dex.coreos.com in the auth namespace — Tapir now WRITES to the shared identity provider. This shipped with no ADR; recorded retroactively with the principle-reversal named (partially supersedes ADR-002/013), the security analysis (bounded RBAC, but a real larger trust surface), and the open gaps (orphaned Dex accounts on delete; plaintext invite tokens). Cross-referenced in ADR-002 and ADR-013 status lines so a future reader isn't misled. ADR-014 annotated: 429 handling shipped in v0.6.0 but decision item 2 (shared per-egress-IP rate gate) appears realised as per-VIDEO backoff, not a process-wide IP gate — flagged not-confirmed-done. Also flags the missing migration 008 / rate_limited_at discrepancy (v0.6.0 report cited 008; tree jumps 007->009). No code changed in this commit — audit trail only. --- DECISIONS.md | 117 ++++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 110 insertions(+), 7 deletions(-) diff --git a/DECISIONS.md b/DECISIONS.md index 21516e0..6fa94aa 100644 --- a/DECISIONS.md +++ b/DECISIONS.md @@ -29,7 +29,9 @@ Go's concurrency model fits the watcher/worker shape). ## ADR-002 — No Supabase; reuse existing Dex / ESO / Postgres conventions -**Status:** Accepted (2026-06-02) +**Status:** Accepted (2026-06-02). The "Tapir does not write to the shared identity provider" +implication is **partially superseded by ADR-017** (invite flow writes Dex Password CRs); the +no-Supabase / reuse-existing-primitives decision stands. **Context.** The draft proposed self-hosted Supabase for auth + RLS + secrets (Vault). The homelab already runs **Dex** (OIDC), **ESO + 1Password** (secrets), and a **postgres18** @@ -292,7 +294,11 @@ explicit call, with isolation as the guardrail that keeps it safe. ## ADR-013 — Account deletion is Tapir-side only; the Dex identity is left intact -**Status:** Accepted (2026-06-03) +**Status:** Accepted (2026-06-03). The "Tapir never holds write access to the shared identity +provider" rationale is **partially superseded by ADR-017** (the invite flow now *creates* Dex +Password CRs). The deletion-behaviour decision itself — delete Tapir-side state, leave the Dex +identity intact — still stands; ADR-017 only changes the create side, not delete. See ADR-017 +for the now-asymmetric posture (Tapir can create Dex accounts but still does not delete them). **Context.** Stage 1 (ADR-012) added account deletion. A registered user is two things: a `users` row (plus all their data, cascade-linked) in Tapir's Postgres, and a subject identity @@ -308,7 +314,9 @@ deprovision the Dex identity. The maintainer chose (a). - All of that user's secrets in the SecretStore (the per-user YouTube refresh-token refs). The **Dex identity is deliberately left intact.** Tapir does not deprovision, disable, or -modify the shared Dex directory. +modify the shared Dex directory **on deletion**. (Note per ADR-017: Tapir *does* now create Dex +Password CRs on invite — so the create and delete sides are deliberately asymmetric, and a +deleted user's Dex Password CR persists. See ADR-017 consequences.) **Consequences.** - **Clean re-registration:** a deleted user who logs in again arrives as a Dex-authenticated @@ -319,9 +327,13 @@ modify the shared Dex directory. the identity carries no Tapir content. **But if Tapir ever moves toward Future C (real external/public users), this is a GDPR-shaped gap** — a true "delete my account" there must also deprovision or anonymise the Dex identity, which is a new ADR and likely a Dex-admin - integration Tapir does not currently have. -- **Blast radius stays small:** Tapir never holds write access to the shared identity provider, - consistent with the estate's blast-radius-minimisation posture (ADR-002, architecture review). + integration Tapir does not currently have. **ADR-017 widens this gap:** Tapir now *creates* + the Dex Password CR but does not delete it, so a deleted Tapir user leaves an orphaned Dex + local-password account. Tracked as a known gap in ADR-017. +- **Blast radius:** ADR-013 originally claimed "Tapir never holds write access to the shared + identity provider." **ADR-017 changes this** — Tapir now holds scoped `create`+`get` on + `passwords.dex.coreos.com` in the `auth` namespace. The blast radius is no longer zero; it is + bounded by that RBAC. See ADR-017 for the security analysis. **Reversibility.** Adding Dex deprovisioning later is a superseding ADR; nothing about the current choice blocks it. Recorded now because "deletion is partial by design" is a deliberate @@ -331,7 +343,9 @@ semantic that future-Tapir (and any compliance review) must know was chosen, not ## ADR-014 — Timedtext 429 handling: per-host backoff + honest in-flight UX, before any Whisper reconsideration -**Status:** Accepted (2026-06-03) +**Status:** Accepted (2026-06-03). **Partially implemented as of v0.6.0** — see the +implementation note at the end; the shared per-egress-IP rate gate (decision item 2) may not be +fully realised. Verify against `internal/runner` + the youtube adapter before treating as done. **Context.** ADR-010 acquires captions from the unauthenticated `timedtext` baseUrl. Live runs show that endpoint **rate-limits per source IP (HTTP 429) under volume** — many videos fetched @@ -386,6 +400,19 @@ clean data then tells you whether Whisper is warranted. dedicated egress IP / outbound proxy is worth it; CronJob-driven `tapir run` interaction with the rate gate (the batch path moves into k3s per the deferred CronJob item). +**Implementation note (2026-06-03, v0.6.0 — added during reconciliation).** A v0.6.0 release +shipped 429 handling: `domain.SourceRateLimited` (429 no longer collapsed to `SourceNone`), +youtube adapter maps 429 → `SourceRateLimited`, migration 007 `transcript_status` + (per the +v0.6.0 report) a `videos.rate_limited_at` column, `TAPIR_FETCH_BACKOFF` config, runner records +rate-limited + skips within the backoff window, and a "⏳ Retrying later" badge. This realises +decision items 1, 3, and 4 well. **Item 2 (a single shared per-egress-IP rate gate) appears to +be realised as per-*video* backoff state, NOT a process-wide IP gate** — multiple videos can +still each hit the endpoint and collectively trip the per-IP 429. Treat item 2 as **not yet +confirmed done**; verify in `internal/runner`/youtube adapter and, if absent, it remains open. +ALSO: the v0.6.0 report referenced "migration 008 videos.rate_limited_at" but **no migration 008 +exists on `main`** (tree jumps 007→009). Resolve whether `rate_limited_at` actually shipped (in +007? lost? renumbered?) — a documentation/build discrepancy flagged during reconciliation. + --- ## ADR-015 — Per-user credentials: envelope-encrypted in PG18, not vault-stored @@ -489,6 +516,82 @@ about editing a gate while it's unmet) so the reasoning survives, not just the n --- +## ADR-017 — Invite flow: Tapir creates Dex local-password accounts (write access to the shared identity provider) + +**Status:** Accepted (2026-06-03), **recorded retroactively during reconciliation.** This +capability **shipped in v0.7.0 without an ADR** — code, RBAC, and a deployed ServiceAccount +landed before any decision record existed. This ADR documents what shipped and honestly records +that the decision-before-code discipline was not followed here (see "Process note"). **Partially +supersedes ADR-002 and ADR-013** (the "Tapir holds no write access to the shared identity +provider" posture). + +**Context.** Stage 1 onboarding needs a way for a friend to get a login. Two paths shipped: +Google OIDC via Dex (no Tapir code — Dex handles it), and an **invite flow** where Tapir itself +provisions a Dex **local-password** account. The invite flow is what this ADR is about. + +**What shipped (reconstructed from `internal/adapters/dex/dex.go`, `internal/web/invite.go`, +migration `009_invitations`).** +1. `tapir invite ` (host CLI) writes an `invitations` row: a 32-byte crypto-random, + single-use, 7-day-expiry `token` (the token is the capability), `email`, `used_at`. + The `invitations` table is deliberately **not** RLS/`user_id`-scoped — the invitee has no + user yet and the token itself is the secret. (Sound; documented in the migration.) +2. Recipient visits `/invite/{token}` (public, no session), sets a password (validated *before* + the token is consumed, so a typo doesn't burn it), the token is claimed exactly once. +3. Tapir bcrypt-hashes (cost 12) and **POSTs a `passwords.dex.coreos.com` Custom Resource into + the `auth` namespace** via the in-cluster Kubernetes API, authenticating as the `tapir` + ServiceAccount. TLS validated against the mounted cluster CA. Off-cluster (dev) it returns + `ErrNotInCluster` and degrades without consuming the invite. +4. RBAC applied this deploy: the `tapir` ServiceAccount has **`create`+`get` on + `passwords.dex.coreos.com` in the `auth` namespace** (and only that). +5. Dex (kubernetes storage) then serves local-password login for that email; Tapir's + registration gate creates the profile on first login. + +**Decision (as ratified now).** Accept the invite flow as built: Tapir *may* hold scoped write +access to Dex's `passwords` resource in the `auth` namespace, for the purpose of invite-based +local-account creation. This is a deliberate, bounded reversal of the prior "no write access to +the shared identity provider" posture (ADR-002/013). + +**Why this is acceptable (the case for keeping it).** +- The RBAC is **minimally scoped**: `create`+`get` on one resource type in one namespace, not + broad Dex/cluster write. Blast radius is bounded and inspectable. +- It enables friend onboarding **without Google OAuth app verification** (ADR-008 deferred that), + which is genuinely useful for Future B. +- The code is careful: validate-before-consume, single-use tokens, graceful off-cluster degrade, + sentinel errors mapped to clear messages, the base64-bcrypt storage gotcha handled. + +**Consequences / known gaps (the case to watch).** +- **Reverses a load-bearing principle.** ADR-002 and ADR-013 leaned on "Tapir never writes to the + shared identity provider" for blast-radius minimisation. That is no longer true. Anyone reading + those ADRs without this one would be misled — hence the cross-references added to both. +- **Create/delete asymmetry → orphaned Dex accounts.** Tapir now *creates* Dex Password CRs but + (per ADR-013) does not *delete* them on account deletion. A deleted Tapir user leaves an + orphaned Dex local-password account that can still authenticate (though it would hit the + registration gate with no profile). This widens the ADR-013 right-to-erasure gap. **Open.** +- **`create` on a shared-namespace identity resource** is a meaningfully larger trust surface than + the rest of Tapir. If the `tapir` pod is compromised, the attacker can mint Dex local accounts. + Bounded by the namespace/resource scope, but real — worth a deliberate look before Future C. +- Token custody for invites is in PG (`invitations.token`) in plaintext; single-use + 7-day TTL + bound the exposure, but a DB read yields live invite tokens until claimed/expired. + +**Process note (why this ADR is retroactive).** This capability was built and deployed by +parallel agent sessions while the main planning thread was elsewhere, and shipped with no ADR — +the first time in this project a guardrail-reversing change skipped the decision-before-code +discipline. Recorded here not to rubber-stamp it but to restore the audit trail: the decision is +now visible, its principle-reversal is named, and its open gaps (orphaned accounts, the larger +trust surface) are tracked rather than buried in a release note. The maintainer is ratifying it +after the fact; if on reflection the trust surface is unwanted, reversing it (drop the RBAC + +invite flow, rely on Google OIDC only) is a clean follow-up ADR. + +**Open items this ADR creates:** +- Decide whether to close the orphaned-Dex-account gap (delete the Password CR on account + deletion — but that *widens* write access to `delete`, the opposite direction; or accept the + orphan for Future B and fix at Future C). +- Confirm the RBAC really is `create`+`get` only (not broader) against the deployed manifest in + `infra`. +- Review the larger trust surface before any Future C move. + +--- + ## Rejected alternatives Approaches considered during the 2026-06-02 planning + grill session and **deliberately not