From 4706c508e9d9f8d47491551f9a82931d89f1dd98 Mon Sep 17 00:00:00 2001 From: mathias Date: Fri, 5 Jun 2026 12:47:39 +0000 Subject: [PATCH] =?UTF-8?q?docs:=20ratify=20ADR-017=20KEEP=20=E2=80=94=20i?= =?UTF-8?q?nvite=20flow=20is=20load-bearing=20(some=20users=20can't=20use?= =?UTF-8?q?=20Google=20OIDC)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Records the deliberate keep-or-reverse decision on the Dex-write invite flow. Decision: KEEP. Deciding fact: not all intended Future-B users will use Google accounts, so Google OIDC alone can't onboard them — the invite flow is load-bearing, not redundant. Trust-surface cost accepted deliberately, explicitly NOT as a precedent for widening further, and explicitly NOT by adding delete RBAC to fix the orphan gap. Open items reframed as tracked follow-ups (verify RBAC against the real manifest; accept orphan for Future B, revisit before Future C). Added the reversal to rejected-alternatives. --- DECISIONS.md | 47 +++++++++++++++++++++++++++++++---------------- 1 file changed, 31 insertions(+), 16 deletions(-) diff --git a/DECISIONS.md b/DECISIONS.md index 1682b4f..b1193a5 100644 --- a/DECISIONS.md +++ b/DECISIONS.md @@ -519,12 +519,25 @@ 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). +**Status:** Accepted (2026-06-03), **recorded retroactively during reconciliation, then +deliberately ratified KEEP (2026-06-03).** 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). + +**Keep-or-reverse, decided (2026-06-03).** After the retroactive recording, the maintainer +weighed keep vs. reverse (drop the RBAC + invite flow, rely on Google OIDC only). **Decision: +KEEP.** Deciding fact: not all intended Future-B users have / will use Google accounts, so Google +OIDC alone *cannot* onboard them — the invite flow is therefore **load-bearing, not a redundant +convenience**, and reversing it would leave some intended users with no onboarding path. The +trust-surface cost (scoped `create` on `passwords` in `auth`) is accepted deliberately in +exchange. The opposing argument (this contradicts ADR-015's "no write credential to shared infra" +logic, applied there to secrets) was considered and outweighed *only* because the capability is +genuinely necessary for real users here — it is **not** a precedent for widening the surface +further. **Explicitly NOT chosen:** adding `delete` to the RBAC to close the orphan gap (that +widens the surface in the wrong direction; accept the orphan at Future B instead — see open +items). **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 @@ -556,7 +569,8 @@ the shared identity provider" posture (ADR-002/013). - 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. + which is genuinely useful for Future B — and necessary for users who won't use Google (the + deciding fact in the keep decision above). - 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. @@ -567,7 +581,8 @@ the shared identity provider" posture (ADR-002/013). - **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.** + registration gate with no profile). This widens the ADR-013 right-to-erasure gap. **Open — + accepted for Future B, revisit before Future C (do NOT add `delete` RBAC just to fix this).** - **`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. @@ -579,16 +594,15 @@ parallel agent sessions while the main planning thread was elsewhere, and shippe 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. +trust surface) are tracked rather than buried in a release note. The maintainer ratified KEEP +after the fact (see status block) on the deciding fact that some intended users cannot use Google +OIDC. -**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). +**Open items this ADR creates (tracked in infra):** - Confirm the RBAC really is `create`+`get` only (not broader) against the deployed manifest in - `infra`. + `infra` — a security claim currently resting on a sibling report, not a verified manifest read. +- Accept the orphaned-Dex-account gap for Future B; revisit (orphan cleanup + the whole Dex-write + surface) before any Future C move. Do **not** add `delete` RBAC solely to fix the orphan. - Review the larger trust surface before any Future C move. --- @@ -614,6 +628,7 @@ maps to the ADR that settles it. | Vault-write SA for per-user OAuth tokens (ESO as runtime write path) | ESO syncs vault→cluster at deploy time, not a runtime write API; a write-SA widens blast radius to shared infra to store app row-data | ADR-015, infra#88 | | Supabase for per-user credential storage | Adds a second datastore for a few encrypted strings PG18 already holds; reopens ADR-002 | ADR-015, infra#88 | | Feedback-based Stage 0 gate (friends saying it's useful) | Politeness bias makes asked-for feedback the least reliable signal; return-usage is the real test | ADR-016 | +| Reverse the Dex-write invite flow (Google OIDC only) | Some intended Future-B users won't use Google; OIDC-only leaves them with no onboarding path — invite flow is load-bearing | ADR-017 | If a future case genuinely reopens one of these, that's a new ADR superseding the relevant one — not a silent reversal.