From fef537b34378a4551ee0caad34bd76a81a0c6e6d Mon Sep 17 00:00:00 2001 From: myselfsiddharth Date: Fri, 14 Aug 2026 12:18:51 -0700 Subject: [PATCH 1/2] feat(privacy): gate non-local session establishment on recorded consent (SC-05) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PRD §7 / SC-05 required explicit "you are authorizing automation of your own account" consent before Fork A rides a real user's session, and nothing enforced it. ADR-0018 decides the consent moment is a stored consent record checked before every session-establishing run (not an onboarding flow or a one-time first-run banner, both rejected in writing) and ships the guard: `establishSession` now requires a `SessionAuthorization`, obtainable only via `SessionAuthorization.authorize(baseUrl, consent?)` (src/session/consent.ts), which refuses a non-local target with no `ConsentAcknowledgment`. Both classes use TenantKey's private-constructor pattern so the check cannot be bypassed by an object literal. The gate itself is enforced by construction (no second door into establishSession); the refusal for a non-local, non-consented call is enforced by test (tests/unit/session-consent.test.ts), since TypeScript cannot evaluate a hostname at compile time. Local targets (the Grafana test-bed, fixture credentials the project owns) are unaffected — tests/unit/recorder-preamble.test.ts passes unchanged, 8/8. docs/privacy/session-consent-copy.md drafts the copy PRD §7 asks for. It is explicitly NOT legally reviewed, and says so in its own body — issue #102 requires review by whoever owns legal risk (founder), which has not happened. No consent record ever carries credentials or session material; ConsentAcknowledgment is copy_version + timestamp only. docs/privacy/session-custody.md's SC-05 row and gap analysis are updated to match exactly what shipped: the gate and refusal are real, the persistence layer and UI are not built (no real non-local caller exists yet to build them for), and the copy is unreviewed. Closes #102 Co-Authored-By: Claude Opus 5 --- docs/README.md | 4 +- .../ADR-0018-session-consent-gate.md | 216 ++++++++++++++++++ docs/privacy/counsel-packet-sizing.md | 9 +- docs/privacy/session-consent-copy.md | 108 +++++++++ docs/privacy/session-custody.md | 70 +++++- experiments/gate-v1/live-run.ts | 8 +- src/recorder/cli.ts | 8 +- src/recorder/preamble.ts | 17 +- src/session/consent.ts | 213 +++++++++++++++++ src/session/index.ts | 29 ++- tests/unit/recorder-preamble.test.ts | 15 +- tests/unit/session-consent.test.ts | 183 +++++++++++++++ 12 files changed, 849 insertions(+), 31 deletions(-) create mode 100644 docs/decisions/ADR-0018-session-consent-gate.md create mode 100644 docs/privacy/session-consent-copy.md create mode 100644 src/session/consent.ts create mode 100644 tests/unit/session-consent.test.ts diff --git a/docs/README.md b/docs/README.md index 66c115b..73ad2af 100644 --- a/docs/README.md +++ b/docs/README.md @@ -66,6 +66,7 @@ Integrity surface: **[INTEGRITY-AUDIT.md](./INTEGRITY-AUDIT.md)**. | [ADR-0015](decisions/ADR-0015-task-identity-and-intent-resolution.md) | Task identity (phrasing/parameters/host don't fork it, product version does); intent → task_key via normalized exact match behind a swappable `IntentMatcher`, MISS on no confident match; `site_key` drops the address (`grafana-oss@{version}`, never `@{host}:{port}`) | accepted | 2026-08-12 | Issue #124. New package `src/intent/`, wired into `src/recorder/cli.ts`. `gate:matrix --from-cache` wiring **not done yet** | | [ADR-0016](decisions/ADR-0016-session-key-custody.md) | Session-key custody: KMS-wrapped master; rotation via a **global** `key_epoch` + batch re-encryption; per-tenant erasure via a **per-tenant secret** mixed into HKDF (a non-secret marker would erase nothing), not file deletion; dev/CI keep the env-var path unmodified | accepted | 2026-08-12 | Issue #146, follow-up to #98/#143. Decision only — no vendor picked, nothing implemented. Changes derivation, deliberately before a first caller exists | | [ADR-0017](decisions/ADR-0017-pool-vocabulary-rule.md) | Pinned-version vocabulary rule added to the pool allowlist (`src/cache/vocabulary.ts`), additive to `isChromeName`; measured **zero** row-level yield change on the one live bundle — compiler pre-check and recorder tagging are the binding constraints there, not vocabulary | accepted | 2026-08-12 | Issue #126. Multi-version matrix yield is `no_data` — no Docker testbed in this environment | +| [ADR-0018](decisions/ADR-0018-session-consent-gate.md) | SC-05 consent moment = a stored consent record checked before every session-establishing run (not onboarding, not a one-time banner); `establishSession` now requires a `SessionAuthorization`, obtainable only via `SessionAuthorization.authorize(baseUrl, consent?)`, which refuses a non-local target with no `ConsentAcknowledgment` | accepted | 2026-08-14 | Issue #102. Gate is enforced by construction, the refusal is enforced by test; persistence, UI, and legal review of the copy are still open | --- @@ -131,9 +132,10 @@ Integrity surface: **[INTEGRITY-AUDIT.md](./INTEGRITY-AUDIT.md)**. | Doc | What it is | Status | Last true | Notes | | --- | --- | --- | --- | --- | | [boundary-spec.md](privacy/boundary-spec.md) | Write-time allowlist | review | 2026-07-24 | PRD §6; canary merge-blocking | -| [session-custody.md](privacy/session-custody.md) | Session-custody checklist + gap analysis | draft | 2026-08-11 | PRD §7; distinct from §6's pooling allowlist — see doc. SC-01 closed by #98; SC-02/04 enforced; SC-03/05/06 open (#100, #102, #103) | +| [session-custody.md](privacy/session-custody.md) | Session-custody checklist + gap analysis | draft | 2026-08-14 | PRD §7; distinct from §6's pooling allowlist — see doc. SC-01 closed by #98; SC-02/04 enforced; SC-05 gate+refusal built ([ADR-0018](decisions/ADR-0018-session-consent-gate.md), #102, persistence/UI/legal review still open); SC-03/06 open (#100, #103) | | [session-state-encryption.md](privacy/session-state-encryption.md) | SC-01 mechanism: threat model per environment, the envelope, and what key custody v1 **defers** | accepted | 2026-08-12 | Issue #98. A capability with **no callers** — nothing persists session material yet, and this does not change that. Custody/rotation/erasure now **decided** in [ADR-0016](decisions/ADR-0016-session-key-custody.md), which also changes the derivation this doc describes | | [counsel-packet-sizing.md](privacy/counsel-packet-sizing.md) | Counsel-packet sizing (pivot brief §5): position outline, architecture note, storage decision, cost/preconditions | draft | 2026-08-14 | Issue #36. Not legal advice. Track 2 FAIL / no anchor locked (ADR-0004) — a template + trigger, not a position on an invented site. SC-06 remains "not addressed" pending #103's trigger | +| [session-consent-copy.md](privacy/session-consent-copy.md) | SC-05 draft consent screen copy (`copy_version: sc05-v1`) | draft | 2026-08-14 | Issue #102, [ADR-0018](decisions/ADR-0018-session-consent-gate.md). **Not legally reviewed** — do not treat as cleared | --- diff --git a/docs/decisions/ADR-0018-session-consent-gate.md b/docs/decisions/ADR-0018-session-consent-gate.md new file mode 100644 index 0000000..6f0730f --- /dev/null +++ b/docs/decisions/ADR-0018-session-consent-gate.md @@ -0,0 +1,216 @@ +--- +title: "ADR-0018 — Where the SC-05 consent moment lives, and its guard" +doc_type: adr +status: accepted +owner: B0 +created: 2026-08-14 +updated: 2026-08-14 +confidence: MED +supersedes: null +sources_verified: true +--- + +# ADR-0018 — Where the SC-05 consent moment lives, and its guard + +## Status + +accepted + +## Context + +**Triggered by:** issue #102, itself filed by `docs/privacy/session-custody.md`'s gap analysis +for SC-05. PRD §7 requires "explicit customer consent language ('you are authorizing automation of +your own account')" before Fork A rides a user's own authenticated session. At the time #102 was +filed, that sentence existed nowhere but the PRD itself — no UI copy, no CLI banner, no onboarding +step, no ToS click-through, and no code path that checks for one. + +**Not currently blocking Track 1, and this ADR does not change that.** `establishSession` +(`src/recorder/preamble.ts`) authenticates to a local, self-hosted Grafana container using +`FIXTURE_ADMIN_USER`/`FIXTURE_ADMIN_PASS` (`src/testbed/constants.ts`) — a fixture credential the +project itself provisions and tears down. There is no real account holder to consent on behalf of, +which is the same reasoning `docs/privacy/session-custody.md`'s "Track-1 relevance" section already +gives for SC-05 being inapplicable today. **It becomes required the moment anything establishes a +session against a non-local target**, and that is the moment this ADR's guard is built for, not a +hypothetical future one. + +Issue #102 asks for three things. This ADR resolves the first two; the third is the code delivered +alongside it, described under Decision 2. + +1. Where the consent moment lives. +2. The actual copy, reviewed by whoever owns legal risk (founder). +3. A guard function that refuses a non-local session establishment without a recorded consent + acknowledgment, with a unit test on the refusal path. + +## Decision 1 — Where the consent moment lives: a stored consent record, checked before every session-establishing run + +### Options considered + +#### A — Onboarding flow (rejected) + +Honest case for: the conventional place a product asks for this kind of agreement, and it would +sit naturally beside a future account-creation or workspace-setup step. + +Honest case against: **this product has no onboarding flow to put it in.** Paragent today is a CLI +and a set of libraries (`src/recorder/cli.ts`, `src/testbed/cli.ts`, the `paragent` binary shipped +in the commit immediately before this one) — there is no signup, no hosted account, no step a user +passes through once before ever running a task. Inventing an onboarding flow to hold a consent +screen would be building product surface this ADR has no mandate for, and it would decouple the +consent moment from the thing it is supposed to gate: a user can complete "onboarding" once and +then point the same install at a dozen different real accounts over months, with no onboarding +step in between any of them. + +#### B — First-run CLI banner (rejected as the sole mechanism) + +Honest case for: it fits the CLI shape this repo actually has, and "first run" is a real, checkable +event — this repo already has first-run logic for an unrelated reason (`dismissFirstRunModal` in +`src/recorder/preamble.ts` handles Grafana's own first-run dialog, so the concept is not foreign +to this codebase). + +Honest case against: **"first run" is a property of the install, not of the account being +automated, and Fork A's whole premise is that the same install may be pointed at many different +real accounts over its lifetime.** A banner shown once and never checked again cannot express "the +user agreed to automate *this* account" versus "the user agreed to something, once, a long time +ago, possibly about a different site." It is also structurally unable to satisfy SC-05's own +checkable restatement — "shown before **the session is used**, and this is enforced somewhere +checkable" (`docs/privacy/session-custody.md`) — for any run after the first, because by +definition it does not run again. The banner is real UX, but by itself it answers "has this CLI +ever shown this text," not "did the user agree to automate *this* account, recently enough for +that agreement to still mean something" — and only the latter is what a guard at the point of +session establishment can check. + +#### C — A stored consent record, checked before every session-establishing run (chosen) + +Honest case for: it is the only one of the three candidates that is a **runtime precondition +rather than a UI moment**, which is what makes it possible to write a guard function against at +all — a guard cannot check "did an onboarding flow run" or "was a banner shown eventually," +but it can check "does a valid record exist for this target, right now." It is also +mechanism-agnostic about *how* the record gets created: an onboarding flow, a first-run banner, or +a plain CLI prompt can all be the thing that writes it, so choosing C does not foreclose A or B +later as the UX that *produces* the record — it only insists that whatever produces it, the record +is what gets checked, not the act of production itself. + +Honest case against: it is the option that requires actually building a gate in the code, not +just copy and a screen — which is the same reason it is the one that makes SC-05 "enforced +somewhere checkable" rather than aspirational. This ADR accepts that cost; see Decision 2. + +### Decision + +**C.** The consent moment is a stored acknowledgment, checked immediately before any +session-establishing run against a non-local target. What "stored" and "checked" mean concretely +is Decision 2. + +## Decision 2 — The guard: `SessionAuthorization`, gating `establishSession` itself + +`src/session/consent.ts` adds: + +- **`isLocalTarget(baseUrl)`** — true for the IPv4 loopback block (127.0.0.0/8, not just + `127.0.0.1`), `localhost`, and IPv6 loopback (`::1`, bracketed or not), false for everything + else including an unparseable URL. Verified against what the test-bed actually binds: + `DEFAULT_HOST_PORT` in `src/testbed/constants.ts` is served on `127.0.0.1` + (`experiments/gate-v1/live-run.ts`'s `hostOf` defaults there too), so the predicate is a + deliberate superset of the one address in use today rather than a pinned literal that would trap + the next port or interface choice. +- **`ConsentAcknowledgment`** — `{ copy_version, acknowledged_at }`, nothing else. Private + constructor and a private `#brand` field (mirroring `TenantKey`, `src/session/keys.ts`), so an + object literal cannot forge one; only `ConsentAcknowledgment.record(copyVersion)` can produce it. + Carries no credential or session material, by construction — there is no field to put one in. +- **`SessionAuthorization`** — `{ baseUrl, local, consent }`. Same private-constructor-plus-`#brand` + shape. The only way to obtain one is `SessionAuthorization.authorize(baseUrl, consent?)`, which + throws `ConsentRequiredError` for a non-local `baseUrl` with no `consent`. +- **`establishSession`'s signature changed**: `EstablishSessionOptions` now takes + `target: SessionAuthorization` instead of `baseUrl: string`. This is the load-bearing change — + every existing caller (`src/recorder/cli.ts`, `experiments/gate-v1/live-run.ts`, + `tests/unit/recorder-preamble.test.ts`) now constructs its target via + `SessionAuthorization.authorize(baseUrl)` first. All of them target the local test-bed today, so + none needed a `consent` argument and none changed behavior — verified by running + `tests/unit/recorder-preamble.test.ts` (the real-browser login suite) unchanged against the new + signature: 8/8 pass. + +### Where this lands on the enforcement ladder + +`docs/privacy/session-custody.md` defines three rungs: **enforced by construction** (the bad state +cannot be represented), **enforced by test** (representable, caught by a test), **conventional +only**. This guard is genuinely on the first rung for one specific claim and the second rung for +another, and the two must not be collapsed into one word: + +- **"Can a session be established without going through the check at all" — enforced by + construction.** `establishSession` no longer accepts a `baseUrl` string. There is no second door: + every call site must first call `SessionAuthorization.authorize`, or it does not type-check. + `tests/unit/session-consent.test.ts` pins this with `@ts-expect-error` cases — a raw string in + place of `target`, and a `baseUrl` field where `EstablishSessionOptions` has none — the same + pattern `tests/unit/session-store.test.ts` uses to pin `TenantKey`. +- **"Does a non-local, non-consented call actually get refused" — enforced by test, not by + construction.** `consent` is an optional argument to `authorize`, and TypeScript cannot evaluate + a runtime string (is this hostname local?) at compile time — so a non-local `baseUrl` with no + `consent` **type-checks**, and the refusal is a runtime throw (`ConsentRequiredError`), not a + compile error. `tests/unit/session-consent.test.ts` covers this refusal directly, plus the + matching non-refusal for a local target and for a non-local target *with* consent supplied. + +Stated together: **you cannot reach the login flow without passing through the gate, and the gate +refuses the one case SC-05 cares about — but the refusal itself is a checked runtime property, not +an unrepresentable state.** A caller who is determined to bypass consent cannot do it by forgetting +a step; they would have to call `SessionAuthorization.authorize` with a non-local URL and no +consent and have that call *not* throw, which is exactly the case the test suite exercises. + +### What this guard does not do + +- **It does not persist or read a consent record from disk.** Nothing in this repo establishes a + session against a non-local target yet (re-verified: `grep -rn "SessionAuthorization\.authorize"` + across `src/` and `experiments/` returns only the three local call sites above), so there is no + real caller to build storage for, and inventing one would be exactly the kind of speculation + `src/recorder/preamble.ts`'s own header warns against ("no speculative branches for versions the + matrix does not contain" — the same principle applied to a persistence layer nothing calls). + `ConsentAcknowledgment.record()` is the seam a future CLI prompt or config-file reader calls into; + this ADR builds the seam, not the caller. +- **It does not show anyone the copy.** `docs/privacy/session-consent-copy.md` is the draft text; + displaying it and capturing the typed confirmation is UI work with no caller to attach to today, + for the same reason as above. +- **It is not legal review.** The copy is explicitly marked unreviewed. See that document's own + "Open questions" section. + +## Consequences + +**`EstablishSessionOptions.baseUrl` is gone; every caller now supplies `target`.** This is a +breaking change to `establishSession`'s public shape, contained to three files plus the new test +file. All are local, so the change is mechanical there — normalize-and-wrap — but it is a real +signature change and anything outside this repo importing `establishSession` directly (there is no +such consumer today; it is not published as part of the `paragent` binary's public surface) would +need the same update. + +**SC-05 moves from "not addressed" to "enforced by construction (unbypassable gate) + enforced by +test (the refusal itself)", with the persistence and UI halves still open.** `docs/privacy/session-custody.md`'s +SC-05 row and gap-analysis section are updated in the same PR to say exactly this, not more. + +**No behavior changes for Track 1.** `tests/unit/recorder-preamble.test.ts` passes unchanged +(8/8), and `experiments/gate-v1/live-run.ts` / `src/recorder/cli.ts` both authorize their local +targets with no `consent` argument, exactly as today. + +## Reversal cost + +**Low today, and this ADR is timed the same way ADR-0016 timed itself** — before a real caller +exists rather than after. Reverting `establishSession`'s signature back to a raw `baseUrl` costs +nothing yet: zero real (non-local) callers exist, so nothing would need re-authorizing. That +changes the moment a persistence layer and a real caller land on top of this gate — at that point, +reversing means every stored consent record and every call site built against +`SessionAuthorization` would need migrating, which is the same "decide the shape before real data +exists" argument ADR-0016 made for session-key custody. + +## Open questions / what I could not verify + +- **The persistence and UI mechanism for the consent record are not designed here.** Decision 1 + commits to "a stored consent record, checked before every session-establishing run" as the + *shape*; where it is stored (a local config file, an OS keychain entry, something else), and what + UI writes it (a CLI prompt is the natural fit for this repo today, per + `docs/privacy/session-consent-copy.md`), is left to the first real non-local caller, the same way + ADR-0016 left key custody to a caller that does not exist yet. +- **Whether one consent record should cover every future target, or one per distinct site.** + `docs/privacy/session-consent-copy.md`'s open questions cover this from the copy side; + `SessionAuthorization.authorize` takes a `consent` argument per call, which is compatible with + either policy without further code change, but this ADR does not pick one. +- **Whether a consent acknowledgment should expire.** Nothing here gives `ConsentAcknowledgment` a + validity window — `acknowledged_at` is recorded but never compared against a policy. Not decided, + because there is no caller yet to enforce a window against. +- **The legal adequacy of the draft copy** — entirely out of this ADR's competence; see + `docs/privacy/session-consent-copy.md`. +- **This ADR has not been reviewed by anyone outside this repo**, the same caveat ADR-0016 and + `session-state-encryption.md` carry for their own threat models. diff --git a/docs/privacy/counsel-packet-sizing.md b/docs/privacy/counsel-packet-sizing.md index b1e355b..ae035de 100644 --- a/docs/privacy/counsel-packet-sizing.md +++ b/docs/privacy/counsel-packet-sizing.md @@ -390,8 +390,13 @@ merely undocumented. - The specific portal's ToS text is in hand, verbatim, not summarized from memory. - SC-05 (explicit customer consent language, [issue #102](https://github.com/DevToolie/Paragent/issues/102)) has a product decision, since counsel - will reasonably ask what the account holder is told before their session is automated, and that - is currently "not addressed" per session-custody.md. + will reasonably ask what the account holder is told before their session is automated. **Updated + 2026-08-14:** the moment now has a decision ([ADR-0018](../decisions/ADR-0018-session-consent-gate.md)) + and an engineering gate (`src/session/consent.ts`) refusing a non-local session with no recorded + consent — but the copy the account holder would actually see is drafted, not legally reviewed + ([session-consent-copy.md](./session-consent-copy.md)), and nothing shows it to anyone yet + (no persistence layer, no UI caller). Bringing this precondition to counsel today means bringing + that copy as a draft for review, not as settled language. - Someone has decided whether §3.3's missing rate limiter is acceptable to bring to counsel as-is or needs to be built first — bringing an inaccurate architecture note to counsel is worse than bringing an honest gap. diff --git a/docs/privacy/session-consent-copy.md b/docs/privacy/session-consent-copy.md new file mode 100644 index 0000000..297bbd2 --- /dev/null +++ b/docs/privacy/session-consent-copy.md @@ -0,0 +1,108 @@ +--- +title: Session-automation consent copy (SC-05 draft) +doc_type: spec +status: draft +owner: B0 +created: 2026-08-14 +updated: 2026-08-14 +confidence: LOW +supersedes: null +sources_verified: true +--- + +# Session-automation consent copy (SC-05, PRD §7) + +**This copy has NOT been legally reviewed.** [ADR-0018](../decisions/ADR-0018-session-consent-gate.md) +requires review "by whoever owns legal risk here (founder)" (issue #102, echoing pivot brief §5's +counsel-packet pattern) before this text is shown to a real user against a real account. Nothing +in this repo has done that review. Treat every string below as a first draft, not as cleared +copy — do not ship it, and do not cite this document as evidence the requirement is satisfied. +`sources_verified: true` in the frontmatter means the *factual* claims the copy makes about this +codebase (what is/isn't logged, what SC-06 covers) were checked against the code and other privacy +docs at authoring time; it says nothing about legal adequacy, which is a different kind of +verification this document cannot perform. + +## What this must say, and why + +PRD §7 requires "explicit customer consent language ('you are authorizing automation of your own +account')" before Fork A rides a user's own authenticated session. The requirement names a specific +sentence as the bar (`or equivalent`), not a topic to cover loosely, so the draft below states it +close to verbatim rather than paraphrasing it away. + +## When this is shown + +Per ADR-0018, the enforcement point is **a stored consent record, checked before every +session-establishing run** — not a one-time onboarding screen and not only a first-run banner. +This copy is the text a user sees at the moment that record is created. The natural home for that +moment, given this repo is a CLI today, is an interactive prompt the first time +`establishSession` (`src/recorder/preamble.ts`) is about to run against a **non-local** target; +`docs/privacy/session-custody.md`'s Track-1 relevance section is why local (test-bed) runs never +show this at all. **Building that prompt and its storage is explicitly out of scope for this +document and for issue #102** — see `src/session/consent.ts`'s module doc for what is and is not +built. This file only owns the words. + +## Consent screen text — `copy_version: sc05-v1` + +> ### You're about to let Paragent act inside your account +> +> Paragent is going to use **your** sign-in to continue. That means **you are authorizing +> automation of your own account** — every action it takes from here (clicking, typing, +> navigating, saving) happens as you, with your permissions, inside your own session. +> +> Paragent does not create a separate account and does not act on anyone else's behalf. +> +> Your password is never stored by Paragent. Your session (cookies / sign-in state) is never +> written into a recorded task, a log, or anything shared with other users of this tool. +> +> Before continuing, confirm: +> +> - [ ] I am automating **my own account**, or an account I am explicitly authorized to operate. +> - [ ] I understand Paragent will take real, effective actions inside this account on my behalf — +> not a simulation. +> - [ ] I am responsible for checking whether the site I'm pointing Paragent at allows this kind of +> automation under its own terms. +> +> Type **I CONSENT** to continue, or anything else to cancel. + +## Recorded acknowledgment + +Confirming records `{ copy_version: "sc05-v1", acknowledged_at: }` and nothing +else — see `ConsentAcknowledgment` in `src/session/consent.ts`. No credential, cookie, or session +value is ever a field on that record; CONTRIBUTING rule 1 ("no secrets ever") applies to a consent +record exactly as it does to a trajectory or a log line. + +## Declining + +Typing anything other than the exact confirmation string is a decline, not a retry loop that +nags — `establishSession` is never called for that run, and the caller sees +`ConsentRequiredError`'s message (`src/session/consent.ts`), which restates why and points back +at this document's `copy_version`. + +## What is deliberately NOT in this copy + +- **No liability waiver or indemnification language.** That is exactly the kind of clause pivot + brief §5 reserves for whoever owns legal risk (the founder), and drafting one here would be + writing that person's decision for them under the cover of "consent copy." +- **No per-site legalese about a specific portal's terms of service.** That is SC-06's job + (`docs/privacy/session-custody.md`, filed as + [#103](https://github.com/DevToolie/Paragent/issues/103)), triggered by a real anchor site being + locked, not by this document. This copy only asks the user to take responsibility for checking — + it does not check on their behalf. + +## Open questions / what I could not verify + +- **Legal adequacy — the load-bearing open question.** Whether this text actually satisfies PRD + §7's "or equivalent" bar in a way that would hold up under scrutiny is not something this + document, or the agent that wrote it, can verify. It requires the founder's review named in + issue #102, which has not happened. +- **Whether a typed confirmation string (`I CONSENT`) is the right affirmative-action pattern for + a CLI, versus a single keypress or a `--i-consent` flag.** Chosen here for being unambiguous and + hard to trigger accidentally; not tested against a real user. +- **Whether the copy needs to name the specific site/portal being automated**, once SC-06 exists + for a real anchor. This draft is deliberately site-agnostic because no anchor is locked yet + (`docs/privacy/session-custody.md`, SC-06); revisit when one is. +- **Whether one global acknowledgment should cover every future site**, or whether a new consent + event is needed per distinct non-local target. Leaning toward per-target (a user agreeing to + automate their email account says nothing about their bank), but not decided — `src/session/consent.ts`'s + `SessionAuthorization.authorize` takes a `consent` argument per call, which is compatible with + either policy but does not itself pick one. diff --git a/docs/privacy/session-custody.md b/docs/privacy/session-custody.md index 74dca7a..62d9c8a 100644 --- a/docs/privacy/session-custody.md +++ b/docs/privacy/session-custody.md @@ -4,7 +4,7 @@ doc_type: spec status: draft owner: B5 created: 2026-07-30 -updated: 2026-08-11 +updated: 2026-08-14 confidence: HIGH supersedes: null sources_verified: true @@ -31,7 +31,7 @@ below); everything else is sized, not built, here. | SC-02 | Never written to trajectories | No trajectory ever produced by the recorder contains a cookie/storage-shaped field, on disk, regardless of call path | | SC-03 | Never written to logs | No console output, CI log, or persisted log artifact ever contains cookie/storage-shaped content | | SC-04 | Session material excluded from the compiler's input by construction | The compiler's input type has no field capable of carrying it, so there is nothing to exclude at runtime — it was never representable | -| SC-05 | Explicit customer consent language | A user automating their own account is shown "you are authorizing automation of your own account" (or equivalent) before the session is used, and this is enforced somewhere checkable | +| SC-05 | Explicit customer consent language | A user automating their own account is shown "you are authorizing automation of your own account" (or equivalent) before the session is used, and this is enforced somewhere checkable — **enforced by construction (gate) + enforced by test (refusal), copy drafted but not legally reviewed; see [ADR-0018](../decisions/ADR-0018-session-consent-gate.md)** | | SC-06 | Documented ToS position per anchor site | For each real (non-local) portal the agent authenticates against, a written position on authorized-user automation exists before any run against it | `SC-02` and `SC-03` split PRD's single "never written to logs or trajectories" clause into two @@ -263,13 +263,57 @@ would arrive in whichever convention its author reached for. The live-browser case sets a real cookie on the context before capturing, so "no cookies present" is not the reason it passes. -### SC-05 — explicit consent language — **not addressed** - -Repo-wide search for consent/authorization language found nothing except the PRD requirement -sentence itself. No onboarding flow, CLI banner, or stored-consent record exists. This is product -and legal copy, not an engineering gap this doc can close by itself — filed as -[#102](https://github.com/DevToolie/Paragent/issues/102) to force the "where does this moment -live" decision before Fork A ever runs against a real account. +### SC-05 — explicit consent language — **enforced by construction (the gate) + enforced by test (the refusal); persistence, UI, and legal review still open** + +**Updated 2026-08-14 (#102).** The gap analysis below was written when nothing existed at all — +no decision about where the consent moment lives, no copy, no code. [ADR-0018](../decisions/ADR-0018-session-consent-gate.md) +decides the moment (a stored consent record, checked before every session-establishing run, and +explicitly not an onboarding flow or a one-time first-run banner — both rejected in writing there) +and ships the guard it implies. The status splits into three parts, none of which should be +rounded up to cover the others: + +- **The gate: enforced by construction.** `establishSession` (`src/recorder/preamble.ts`) no + longer accepts a `baseUrl` string — `EstablishSessionOptions.target` requires a + `SessionAuthorization`, and `SessionAuthorization` has a private constructor plus a private + `#brand` field (mirroring `TenantKey`, `src/session/keys.ts`), so an object literal cannot forge + one. The only way to obtain one is `SessionAuthorization.authorize(baseUrl, consent?)` + (`src/session/consent.ts`). There is no second door into the login flow. Pinned by + `@ts-expect-error` cases in `tests/unit/session-consent.test.ts`, the same pattern + `tests/unit/session-store.test.ts` uses for `TenantKey`. +- **The refusal: enforced by test, not by construction.** Whether a non-local target with no + `consent` argument actually gets refused is a runtime check inside `authorize` — TypeScript + cannot evaluate "is this hostname local" at compile time, so the bad call *type-checks* and the + refusal (`ConsentRequiredError`) is a throw, verified by + `tests/unit/session-consent.test.ts`'s refusal-path cases, not an unrepresentable state. ADR-0018 + states this distinction explicitly rather than claiming the whole guard is "enforced by + construction." +- **Local-target predicate, verified against what the test-bed actually does.** `isLocalTarget` + treats the IPv4 loopback block (127.0.0.0/8), `localhost`, and IPv6 loopback as needing no + consent — a deliberate superset of the one address the test-bed binds + (`DEFAULT_HOST_PORT` in `src/testbed/constants.ts`, `127.0.0.1`), not a pinned literal. + `tests/unit/session-consent.test.ts` also asserts a private-LAN address (`192.168.x.x`) is + **not** treated as local — loopback-only, not "looks internal." + +**Not built, and said plainly rather than implied by omission:** + +- **No persistence layer.** `ConsentAcknowledgment.record()` produces an in-memory acknowledgment; + nothing reads or writes one from disk. Nothing in this repo establishes a session against a + non-local target yet (re-verified: `SessionAuthorization.authorize` has exactly three callers, + `src/recorder/cli.ts`, `experiments/gate-v1/live-run.ts`, and the test suite, all targeting the + local test-bed), so there is no real caller to build storage for. This is the same "decide the + shape before real data exists, build persistence when a caller exists" ordering ADR-0016 used for + session-key custody. +- **No UI.** Nobody sees a consent screen yet. `docs/privacy/session-consent-copy.md` is a draft of + the copy; no CLI prompt or banner displays it. +- **The copy is drafted, not legally reviewed.** [`session-consent-copy.md`](./session-consent-copy.md) + states PRD §7's "you are authorizing automation of your own account" close to verbatim, but issue + #102 requires review "by whoever owns legal risk here (founder)," and that has not happened. The + document says so in its own frontmatter (`confidence: LOW`) and body — this line is not the only + place that caveat lives, and it should not need to be found here to be believed. + +**No change to Track 1.** All three real call sites target the local test-bed and pass no +`consent` argument; `tests/unit/recorder-preamble.test.ts` (the real-browser login suite) passes +unchanged, 8/8, against the new `establishSession` signature. ### SC-06 — documented ToS position per anchor site — **not addressed, and currently inapplicable** @@ -338,7 +382,7 @@ finding under SC-03 shows at least one of them would not catch it if it were. | SC-02 residual | Unit test: `toTrajectory()`'s return value, serialized directly (not via `write()`), still matches none of `assertNoLiteralSecrets`'s patterns. Replace the hand-maintained `extraTrajectories` list with a glob | [#99](https://github.com/DevToolie/Paragent/issues/99) | | SC-03 | New `secret-scan.mjs` pattern(s) for the storageState shape; fixture proves a catch; every existing doc discussing cookies/storage in prose proves a non-catch | [#100](https://github.com/DevToolie/Paragent/issues/100) | | SC-04 | Tripwire: forbidden-key list never appears in serialized compiler `Trajectory`/`Fingerprint` or runner `PageStateSnapshot` output | [#101](https://github.com/DevToolie/Paragent/issues/101) | -| SC-05 | Not a test until the product decision lands; then a guard-function refusal path with a unit test | [#102](https://github.com/DevToolie/Paragent/issues/102) | +| SC-05 | ~~Not a test until the product decision lands; then a guard-function refusal path with a unit test~~ **Built** — `SessionAuthorization.authorize` (`src/session/consent.ts`), gating `establishSession`; refusal path pinned by `tests/unit/session-consent.test.ts`. Persistence, UI, and legal review of the copy remain open | [#102](https://github.com/DevToolie/Paragent/issues/102) | | SC-06 | Not a test; a counsel packet per pivot brief §5, gated on a vertical being locked | [#103](https://github.com/DevToolie/Paragent/issues/103) | ## One-line fix taken in this PR @@ -366,3 +410,9 @@ no citation, since it reads as verified when it silently is not. Playwright's storageState format is the one this codebase would actually produce, but other session-material shapes (a raw `document.cookie` string dump, a JWT in a header log) were not separately fuzzed against the current patterns. +- **SC-05's persistence layer, UI, and legal review are not built (#102, ADR-0018).** The gate and + its refusal path are real and tested; reading/writing a consent record from disk, showing anyone + the copy, and getting the copy reviewed by whoever owns legal risk (founder) are all still open, + and none of them has a caller to attach to yet because nothing in this repo establishes a session + against a non-local target. See [ADR-0018](../decisions/ADR-0018-session-consent-gate.md) and + [session-consent-copy.md](./session-consent-copy.md) for what each piece still needs. diff --git a/experiments/gate-v1/live-run.ts b/experiments/gate-v1/live-run.ts index 8310d0f..d9e7c22 100644 --- a/experiments/gate-v1/live-run.ts +++ b/experiments/gate-v1/live-run.ts @@ -25,6 +25,7 @@ import { chromium, type Browser, type Page } from "playwright"; import { MetricsEmitter } from "../../src/metrics/emitter.js"; import type { ProgramSource } from "../../src/metrics/types.js"; import { establishSession, LoginFailedError } from "../../src/recorder/preamble.js"; +import { SessionAuthorization } from "../../src/session/consent.js"; import { ReplayRunner } from "../../src/runner/replay.js"; import type { RepairModelClient } from "../../src/runner/repair.js"; import type { @@ -378,8 +379,13 @@ export async function runVersionLive( if (opts.preamble) { try { + // SessionAuthorization.authorize (SC-05, #102): no `consent` + // argument — the gate matrix always targets the local test-bed + // with fixture credentials the project owns (ADR-0003), so + // there is no account holder to consent on behalf of. A + // non-local baseUrl here would refuse with ConsentRequiredError. const session = await establishSession(page, { - baseUrl, + target: SessionAuthorization.authorize(baseUrl), username: FIXTURE_ADMIN_USER, password: FIXTURE_ADMIN_PASS, }); diff --git a/src/recorder/cli.ts b/src/recorder/cli.ts index a8ab864..c0c818f 100644 --- a/src/recorder/cli.ts +++ b/src/recorder/cli.ts @@ -22,6 +22,7 @@ import { establishSession, LoginFailedError } from "./preamble.js"; import { resolveTaskKeyForRecording } from "./select-task.js"; import { RECORDER_VERSION, TrajectoryRecorder } from "./session.js"; import { buildLiveSiteKey } from "./site-identity.js"; +import { SessionAuthorization } from "../session/consent.js"; const __dirname = path.dirname(fileURLToPath(import.meta.url)); const ROOT = path.resolve(__dirname, "../.."); @@ -385,8 +386,13 @@ async function main() { // measurement — see src/recorder/preamble.ts. Nothing here touches the // recorder, so no preamble action reaches trajectory.steps or a // step-validity denominator. + // + // SessionAuthorization.authorize (SC-05, #102): no `consent` argument + // here because --base-url in this recorder is always the local + // test-bed (ADR-0006's gate task) — a non-local target would refuse + // with ConsentRequiredError instead of silently logging in. const session = await establishSession(page, { - baseUrl, + target: SessionAuthorization.authorize(baseUrl), username, password: userPass, }); diff --git a/src/recorder/preamble.ts b/src/recorder/preamble.ts index c763173..c0709f0 100644 --- a/src/recorder/preamble.ts +++ b/src/recorder/preamble.ts @@ -64,6 +64,7 @@ */ import type { Page } from "playwright"; +import { SessionAuthorization } from "../session/consent.js"; /** Login stage failed. Named so a caller can tell it from a step failure. */ export class LoginFailedError extends Error { @@ -77,8 +78,15 @@ export class LoginFailedError extends Error { } export interface EstablishSessionOptions { - /** Origin of the Grafana instance, e.g. `http://127.0.0.1:3000`. */ - baseUrl: string; + /** + * Where to log in, plus proof the session-consent gate was cleared (SC-05, + * #102). Obtained from `SessionAuthorization.authorize(baseUrl, consent?)` + * (`src/session/consent.ts`) — not a raw string, so there is no way to + * reach the login flow below without going through that check. A local + * target (the test-bed) authorizes with no `consent` argument; a non-local + * target throws `ConsentRequiredError` without one. + */ + target: SessionAuthorization; username: string; /** Never persisted — drives Playwright only, exactly as typed values do. */ password: string; @@ -126,7 +134,10 @@ export async function establishSession( page: Page, opts: EstablishSessionOptions, ): Promise { - const baseUrl = opts.baseUrl.replace(/\/$/, ""); + // opts.target already cleared the SC-05 consent gate (or is local, which + // needs none) — see EstablishSessionOptions.target's doc. baseUrl is + // already normalized (no trailing slash) by SessionAuthorization.authorize. + const baseUrl = opts.target.baseUrl; const timeout = opts.timeoutMs ?? LOGIN_TIMEOUT_MS; try { diff --git a/src/session/consent.ts b/src/session/consent.ts new file mode 100644 index 0000000..4e1a04c --- /dev/null +++ b/src/session/consent.ts @@ -0,0 +1,213 @@ +/** + * Session-establishment consent gate (SC-05, PRD §7, issue #102). + * + * PRD §7 commits to Fork A: the agent rides the user's own authenticated + * session. The moment that session belongs to a real account, the product is + * automating something a human owns, and PRD §7 requires "explicit customer + * consent language ('you are authorizing automation of your own account')" + * before that happens. [ADR-0018](../../docs/decisions/ADR-0018-session-consent-gate.md) + * decides where that consent moment lives — a stored consent record, checked + * before every session-establishing run — and this module is the checkable + * form of that decision. + * + * ## The shape of the guarantee + * + * `SessionAuthorization` has a **private constructor**, exactly like + * `TenantKey` (`src/session/keys.ts`): it cannot be produced by an object + * literal, only by {@link SessionAuthorization.authorize}, which is where the + * refusal lives. `establishSession` (`src/recorder/preamble.ts`) takes one of + * these as a required parameter instead of a raw `baseUrl` string, so a + * caller cannot reach the login flow without first passing through the + * check — the same move `writeEncryptedStorageState` makes for SC-01. + * + * **Where this lands on the enforcement ladder + * (`docs/privacy/session-custody.md`'s three-value scale):** enforced by + * construction for "did this session establishment go through the check at + * all" — there is no second door into `establishSession`. The *decision* the + * check makes for a non-local target — consent present vs. absent — is still + * a runtime predicate over a value (`ConsentAcknowledgment | undefined`) that + * TypeScript cannot evaluate at compile time, so refusing a real non-local, + * non-consented run is enforced-by-construction-that-you-cannot-skip-the-gate + * plus a runtime check *inside* the gate, backed by + * `tests/unit/session-consent.test.ts`. That composite is stated here + * honestly rather than rounded up to a single word. + * + * ## What this module does not do + * + * It does not read or write a consent record from disk, and it does not show + * anyone the copy in `docs/privacy/session-consent-copy.md`. Those are the + * persistence and UI halves of ADR-0018's decision — "a stored consent + * record" needs a store, and nothing in this repo establishes a session + * against a non-local target yet (`docs/privacy/session-custody.md`, + * Track-1 relevance section), so there is no caller to build that store for. + * `ConsentAcknowledgment.record()` is the seam a future CLI banner or + * onboarding step calls into once one exists; this module is the gate that + * makes skipping it a refusal instead of a silent gap. + */ + +/** Copy version acknowledged by {@link ConsentAcknowledgment.record}. See `docs/privacy/session-consent-copy.md`. */ +export const CONSENT_COPY_VERSION = "sc05-v1" as const; + +/** + * True when `hostname` needs no consent to establish a session against: the + * whole IPv4 loopback block (RFC 5735, 127.0.0.0/8 — not just 127.0.0.1), + * `localhost`, and IPv6 loopback (`::1`, with or without the `[...]` a URL's + * `.hostname` keeps around it). This is deliberately a superset of the one + * address the test-bed actually binds + * (`DEFAULT_HOST_PORT` in `src/testbed/constants.ts`, always `127.0.0.1`), + * because "local" is a property of the network, not a pinned literal — a + * narrower match would be a trap for the next port or interface choice, and + * a real customer account is never reachable on loopback in the first place. + */ +export function isLocalHostname(hostname: string): boolean { + const h = hostname.toLowerCase().replace(/^\[|\]$/g, ""); + if (h === "localhost") return true; + if (h === "::1" || h === "0:0:0:0:0:0:0:1") return true; + if (/^127(?:\.\d{1,3}){3}$/.test(h)) return true; + return false; +} + +/** + * True when `baseUrl` needs no consent — see {@link isLocalHostname}. + * An unparseable URL is **not** treated as local: `establishSession` will + * fail it with a clear navigation error of its own, and this guard does not + * paper over a bad URL by guessing at its safety. + */ +export function isLocalTarget(baseUrl: string): boolean { + let hostname: string; + try { + hostname = new URL(baseUrl).hostname; + } catch { + return false; + } + return isLocalHostname(hostname); +} + +/** + * Refusing a non-local, non-consented session establishment. Named so it is + * distinguishable from {@link LoginFailedError} (`src/recorder/preamble.ts`) + * in a catch block or a log line — this is a policy refusal, not a login + * failure, and the two must never be reported as the same kind of event. + */ +export class ConsentRequiredError extends Error { + readonly code = "CONSENT_REQUIRED" as const; + constructor(readonly baseUrl: string) { + super( + `refusing to establish a session against a non-local target (${baseUrl}) ` + + "without a recorded consent acknowledgment (SC-05, PRD §7). Local " + + "targets (localhost, 127.0.0.0/8, ::1) do not need one — see " + + "docs/decisions/ADR-0018-session-consent-gate.md. Obtain a " + + "ConsentAcknowledgment via ConsentAcknowledgment.record(...) and pass " + + "it to SessionAuthorization.authorize(baseUrl, consent) before calling " + + "establishSession.", + ); + this.name = "ConsentRequiredError"; + } +} + +/** + * A recorded acknowledgment that the SC-05 consent copy + * (`docs/privacy/session-consent-copy.md`, version {@link CONSENT_COPY_VERSION}) + * was shown and agreed to. + * + * Deliberately tiny: a copy version and a timestamp, nothing else. No + * credential, cookie, or session material is ever a field on this class — + * CONTRIBUTING rule 1 applies to consent records exactly as it does to + * everything else this repo persists. Private constructor, mirroring + * `TenantKey`: an object literal cannot forge one, so a caller cannot + * satisfy `SessionAuthorization.authorize` by typing `{ copy_version: "...", + * acknowledged_at: "..." }` — it must come from `record()`, which is the one + * place a future persistence layer (reading a stored record back) or a + * future CLI banner (writing one after the user agrees) would call into. + */ +export class ConsentAcknowledgment { + readonly copy_version: string; + readonly acknowledged_at: string; + /** + * Nominal brand only — carries no material. A `#private` field, like + * `TenantKey`'s `#material`, is what makes TypeScript refuse a + * structurally identical object literal in place of a real instance; + * without one, a `{ copy_version, acknowledged_at }` literal would + * type-check even though `record()` is the only place this class can + * actually be constructed from. + */ + readonly #brand = "ConsentAcknowledgment" as const; + + private constructor(copyVersion: string, acknowledgedAt: string) { + this.copy_version = copyVersion; + this.acknowledged_at = acknowledgedAt; + } + + /** + * Record that `copyVersion` was acknowledged now. + * + * `now` is injectable for tests; defaults to the real clock. Throws on an + * empty `copyVersion` rather than silently recording an unattributable + * acknowledgment — an ack that cannot say *what* was agreed to is not one. + */ + static record( + copyVersion: string = CONSENT_COPY_VERSION, + now: () => Date = () => new Date(), + ): ConsentAcknowledgment { + if (!copyVersion.trim()) { + throw new Error("ConsentAcknowledgment.record: copy_version must not be empty"); + } + return new ConsentAcknowledgment(copyVersion, now().toISOString()); + } + + /** `#brand` earns its keep here too: a readable form for a log line or an assertion failure. */ + toString(): string { + return `${this.#brand}(copy_version=${this.copy_version}, acknowledged_at=${this.acknowledged_at})`; + } +} + +/** + * Proof that a session establishment against `baseUrl` is authorized. + * + * The only way to obtain one is {@link SessionAuthorization.authorize}, and + * that is the only place `ConsentRequiredError` is thrown from. Because + * `establishSession` requires a `SessionAuthorization` — not a raw + * `baseUrl` — instead of a raw string, there is no path into the login flow + * that has not gone through this check first. + */ +export class SessionAuthorization { + /** Normalized (no trailing slash) — the exact string `establishSession` navigates to. */ + readonly baseUrl: string; + readonly local: boolean; + readonly consent: ConsentAcknowledgment | null; + /** Nominal brand only — see {@link ConsentAcknowledgment.#brand}'s doc for why this exists. */ + readonly #brand = "SessionAuthorization" as const; + + private constructor(baseUrl: string, local: boolean, consent: ConsentAcknowledgment | null) { + this.baseUrl = baseUrl; + this.local = local; + this.consent = consent; + } + + /** + * Authorize a session establishment against `baseUrl`. + * + * A local target (see {@link isLocalTarget}) is always authorized — the + * test-bed authenticates fixture credentials the project itself owns + * (`FIXTURE_ADMIN_USER`/`FIXTURE_ADMIN_PASS`, `src/testbed/constants.ts`) + * against a container it also owns, so there is no account holder to + * consent on behalf of (`docs/privacy/session-custody.md`, "Track-1 + * relevance"). A non-local target requires `consent`, and its absence is + * {@link ConsentRequiredError}, not a silent pass-through. + */ + static authorize(baseUrl: string, consent?: ConsentAcknowledgment): SessionAuthorization { + const normalized = baseUrl.replace(/\/$/, ""); + const local = isLocalTarget(normalized); + if (!local && !consent) { + throw new ConsentRequiredError(normalized); + } + return new SessionAuthorization(normalized, local, consent ?? null); + } + + /** `#brand` earns its keep here too: a readable form for a log line or an assertion failure. */ + toString(): string { + return `${this.#brand}(baseUrl=${this.baseUrl}, local=${this.local}, consent=${ + this.consent ? this.consent.copy_version : "none" + })`; + } +} diff --git a/src/session/index.ts b/src/session/index.ts index 7bcf91d..14ea5d5 100644 --- a/src/session/index.ts +++ b/src/session/index.ts @@ -1,15 +1,32 @@ /** - * Session-custody package — encrypted-at-rest persistence of browser session - * state (PRD §7 SC-01, issue #98). + * Session-custody package (PRD §7). * - * The only exported write path requires a `TenantKey`, and `TenantKey` cannot - * be constructed outside `keys.ts`. There is no plaintext sibling to reach for. - * See `docs/privacy/session-state-encryption.md` for the threat model and what - * v1 defers. + * Two independent guarantees, both gated by a private-constructor capability + * a caller cannot forge: + * + * - **Encryption at rest (SC-01, issue #98).** The only exported write path + * requires a `TenantKey`, and `TenantKey` cannot be constructed outside + * `keys.ts`. There is no plaintext sibling to reach for. See + * `docs/privacy/session-state-encryption.md` for the threat model and what + * v1 defers. + * - **Consent before establishing a non-local session (SC-05, issue #102).** + * `establishSession` (`src/recorder/preamble.ts`) requires a + * `SessionAuthorization`, obtainable only from `SessionAuthorization.authorize`, + * which refuses a non-local target with no `ConsentAcknowledgment`. See + * `docs/decisions/ADR-0018-session-consent-gate.md`. */ export const PACKAGE = "session" as const; +export { + CONSENT_COPY_VERSION, + ConsentAcknowledgment, + ConsentRequiredError, + SessionAuthorization, + isLocalHostname, + isLocalTarget, +} from "./consent.js"; + export { KEY_ID_BYTES, MASTER_KEY_BYTES, diff --git a/tests/unit/recorder-preamble.test.ts b/tests/unit/recorder-preamble.test.ts index e4aafa7..662dc33 100644 --- a/tests/unit/recorder-preamble.test.ts +++ b/tests/unit/recorder-preamble.test.ts @@ -22,6 +22,7 @@ import { type Browser } from "playwright"; import { launchTestBrowser } from "../helpers/browser.js"; import { afterAll, beforeAll, describe, expect, it } from "vitest"; import { establishSession, LoginFailedError } from "../../src/recorder/preamble.js"; +import { SessionAuthorization } from "../../src/session/consent.js"; const ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), "../.."); const FIXTURES = path.join(ROOT, "src/recorder/fixtures"); @@ -158,7 +159,7 @@ describe("login preamble (#60)", () => { async (variant, _versions) => { await withServer({ variant }, async (baseUrl, page) => { const session = await establishSession(page, { - baseUrl, + target: SessionAuthorization.authorize(baseUrl), username: USER, password: PASS, }); @@ -174,7 +175,7 @@ describe("login preamble (#60)", () => { { variant: "labelled", landingQuery: "?orgId=1&from=now-6h&to=now&timezone=browser" }, async (baseUrl, page) => { const session = await establishSession(page, { - baseUrl, + target: SessionAuthorization.authorize(baseUrl), username: USER, password: PASS, }); @@ -189,7 +190,7 @@ describe("login preamble (#60)", () => { { variant: "labelled", firstRunModal: true }, async (baseUrl, page) => { const session = await establishSession(page, { - baseUrl, + target: SessionAuthorization.authorize(baseUrl), username: USER, password: PASS, }); @@ -202,7 +203,7 @@ describe("login preamble (#60)", () => { it("fails at the login stage on a wrong password, naming the stage", async () => { await withServer({ variant: "labelled", rejectLogin: true }, async (baseUrl, page) => { const err = await establishSession(page, { - baseUrl, + target: SessionAuthorization.authorize(baseUrl), username: USER, password: "wrong-password-never-persisted", timeoutMs: 8_000, @@ -220,7 +221,7 @@ describe("login preamble (#60)", () => { { variant: "labelled", sessionLogin: "somebody-else" }, async (baseUrl, page) => { const err = await establishSession(page, { - baseUrl, + target: SessionAuthorization.authorize(baseUrl), username: USER, password: PASS, timeoutMs: 8_000, @@ -236,7 +237,7 @@ describe("login preamble (#60)", () => { try { // Port 1 is reserved and refuses connections. const err = await establishSession(page, { - baseUrl: "http://127.0.0.1:1", + target: SessionAuthorization.authorize("http://127.0.0.1:1"), username: USER, password: PASS, timeoutMs: 5_000, @@ -251,7 +252,7 @@ describe("login preamble (#60)", () => { it("never lets the password reach the returned session info", async () => { await withServer({ variant: "labelled", acceptAnyCredential: true }, async (baseUrl, page) => { const session = await establishSession(page, { - baseUrl, + target: SessionAuthorization.authorize(baseUrl), username: USER, password: "canary-secret-never-persist", }); diff --git a/tests/unit/session-consent.test.ts b/tests/unit/session-consent.test.ts new file mode 100644 index 0000000..c55c327 --- /dev/null +++ b/tests/unit/session-consent.test.ts @@ -0,0 +1,183 @@ +/** + * SC-05 unit tests (#102): the session-consent gate. + * + * The refusal path is the load-bearing case — `docs/decisions/ADR-0018-session-consent-gate.md` + * only closes SC-05 if a non-local, non-consented session establishment is + * actually rejected, not merely documented as should-be-rejected. The + * `@ts-expect-error` cases mirror `tests/unit/session-store.test.ts`'s + * treatment of `TenantKey`: they are checked by `npm run typecheck`, and fail + * the build if the call they mark ever starts compiling. + */ + +import { describe, expect, it } from "vitest"; + +import { + CONSENT_COPY_VERSION, + ConsentAcknowledgment, + ConsentRequiredError, + SessionAuthorization, + isLocalHostname, + isLocalTarget, +} from "../../src/session/consent.js"; +import { establishSession, type EstablishSessionOptions } from "../../src/recorder/preamble.js"; + +describe("isLocalHostname / isLocalTarget (SC-05)", () => { + it("treats the whole IPv4 loopback block, not just 127.0.0.1, as local", () => { + expect(isLocalHostname("127.0.0.1")).toBe(true); + expect(isLocalHostname("127.0.0.2")).toBe(true); + expect(isLocalHostname("127.255.255.255")).toBe(true); + }); + + it("treats localhost and IPv6 loopback as local, case-insensitively and bracketed", () => { + expect(isLocalHostname("localhost")).toBe(true); + expect(isLocalHostname("LOCALHOST")).toBe(true); + expect(isLocalHostname("::1")).toBe(true); + expect(isLocalHostname("[::1]")).toBe(true); + }); + + it("does not treat a real hostname or a public/private-but-non-loopback IP as local", () => { + expect(isLocalHostname("grafana.example.com")).toBe(false); + expect(isLocalHostname("8.8.8.8")).toBe(false); + // A private LAN address is not the loopback interface — a real device on + // the network can be a real account, so this must not be waved through. + expect(isLocalHostname("192.168.1.5")).toBe(false); + expect(isLocalHostname("10.0.0.1")).toBe(false); + }); + + it("derives from the URL's hostname, ignoring port/path/query", () => { + expect(isLocalTarget("http://127.0.0.1:3000/login?orgId=1")).toBe(true); + expect(isLocalTarget("https://grafana.example.com:3000/login")).toBe(false); + }); + + it("treats an unparseable URL as non-local rather than guessing", () => { + expect(isLocalTarget("not a url")).toBe(false); + }); +}); + +describe("ConsentAcknowledgment.record (SC-05)", () => { + it("records the copy version and a timestamp", () => { + const ack = ConsentAcknowledgment.record(CONSENT_COPY_VERSION, () => new Date("2026-08-14T00:00:00.000Z")); + expect(ack.copy_version).toBe(CONSENT_COPY_VERSION); + expect(ack.acknowledged_at).toBe("2026-08-14T00:00:00.000Z"); + }); + + it("defaults to the current CONSENT_COPY_VERSION", () => { + const ack = ConsentAcknowledgment.record(); + expect(ack.copy_version).toBe(CONSENT_COPY_VERSION); + }); + + it("refuses an empty copy_version rather than recording an unattributable ack", () => { + expect(() => ConsentAcknowledgment.record("")).toThrow(/copy_version/); + expect(() => ConsentAcknowledgment.record(" ")).toThrow(/copy_version/); + }); + + it("carries no credential- or session-shaped field — SC-05 must not become an SC-01/02 hole", () => { + const ack = ConsentAcknowledgment.record(); + const keys = Object.keys(ack); + expect(keys).toEqual(["copy_version", "acknowledged_at"]); + expect(JSON.stringify(ack)).not.toMatch(/cookie|password|token|session/i); + }); + + it("cannot be constructed by an object literal — record() is the only door", () => { + // @ts-expect-error — private constructor; a structurally identical + // literal is not assignable, exactly as TenantKey's is not + // (tests/unit/session-store.test.ts). + const forged: ConsentAcknowledgment = { + copy_version: CONSENT_COPY_VERSION, + acknowledged_at: new Date().toISOString(), + }; + expect(forged.copy_version).toBe(CONSENT_COPY_VERSION); + }); +}); + +describe("SessionAuthorization.authorize — the refusal path (SC-05)", () => { + it("authorizes a local target with no consent argument at all", () => { + const auth = SessionAuthorization.authorize("http://127.0.0.1:3000"); + expect(auth.local).toBe(true); + expect(auth.consent).toBeNull(); + expect(auth.baseUrl).toBe("http://127.0.0.1:3000"); + }); + + it("normalizes a trailing slash the same way establishSession used to itself", () => { + const auth = SessionAuthorization.authorize("http://127.0.0.1:3000/"); + expect(auth.baseUrl).toBe("http://127.0.0.1:3000"); + }); + + it("REFUSES a non-local target with no consent acknowledgment", () => { + expect(() => SessionAuthorization.authorize("https://real-customer.example.com")).toThrow( + ConsentRequiredError, + ); + }); + + it("the refusal names the target and points at the resolution", () => { + let caught: unknown; + try { + SessionAuthorization.authorize("https://real-customer.example.com"); + } catch (err) { + caught = err; + } + expect(caught).toBeInstanceOf(ConsentRequiredError); + const err = caught as ConsentRequiredError; + expect(err.code).toBe("CONSENT_REQUIRED"); + expect(err.baseUrl).toBe("https://real-customer.example.com"); + expect(err.message).toMatch(/real-customer\.example\.com/); + expect(err.message).toMatch(/SC-05/); + }); + + it("does not refuse a local target even with an absurd path — locality is host-only", () => { + expect(() => + SessionAuthorization.authorize("http://localhost:3000/anything/at/all?x=1"), + ).not.toThrow(); + }); + + it("authorizes a non-local target once a consent acknowledgment is supplied", () => { + const consent = ConsentAcknowledgment.record(); + const auth = SessionAuthorization.authorize("https://real-customer.example.com", consent); + expect(auth.local).toBe(false); + expect(auth.consent).toBe(consent); + }); + + it("cannot be constructed by an object literal — authorize() is the only door", () => { + // @ts-expect-error — private constructor, same shape as TenantKey. A + // caller cannot skip the refusal by typing the result of authorize(). + const forged: SessionAuthorization = { + baseUrl: "https://real-customer.example.com", + local: false, + consent: null, + }; + expect(forged.baseUrl).toBe("https://real-customer.example.com"); + }); +}); + +describe("the gate is load-bearing on establishSession's actual signature", () => { + it("EstablishSessionOptions has no baseUrl field a caller could pass instead of target", () => { + // `baseUrl` is not a key of EstablishSessionOptions; only + // `target: SessionAuthorization` is. This is what makes bypassing + // SessionAuthorization.authorize a compile error rather than a runtime + // check a caller could accidentally skip. `target` is supplied correctly + // here so the only diagnostic is the excess `baseUrl` property. + const target = SessionAuthorization.authorize("http://127.0.0.1:3000"); + const opts: EstablishSessionOptions = { + target, + // @ts-expect-error — excess/unknown property; there is no baseUrl field to assign. + baseUrl: "https://real-customer.example.com", + username: "u", + password: "p", + }; + expect(opts.target).toBe(target); + }); + + it("establishSession itself is only reachable with an authorized target (type-level)", () => { + // No network call is made — this asserts the *type* rejects a raw + // string, which `tsc --noEmit` enforces via the @ts-expect-error below. + // A real refusal-at-runtime is covered by the SessionAuthorization.authorize + // suite above, which is what a non-local caller actually hits. + void ((page: import("playwright").Page) => + establishSession(page, { + // @ts-expect-error — target must be a SessionAuthorization, not a string. + target: "https://real-customer.example.com", + username: "u", + password: "p", + })); + }); +}); From 70164fb0c824a8670d8572e93dabd9e1b40e9d58 Mon Sep 17 00:00:00 2001 From: myselfsiddharth Date: Fri, 14 Aug 2026 12:27:11 -0700 Subject: [PATCH 2/2] fix(privacy): address CodeQL alert and file the deferred half of SC-05 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - tests/unit/session-consent.test.ts: replace two unanchored .toMatch() regex assertions with .toContain() substring checks. CodeQL flagged these as "missing regular expression anchor" on a URL-shaped string (high severity); the assertions never drove a security decision, but toContain is strictly the correct matcher for a plain substring check and removes the alert rather than suppressing it. - src/session/consent.ts: tighten the IPv4 loopback regex to a proper 0-255 octet range so "127.999.999.999" (no such host resolves, and it is not a loopback address) no longer matches. Pinned by a new test. - File #163, "the deferred half of SC-05" (founder legal review of the draft copy, consent-record persistence, the UI moment that displays it), matching the house style of #146 (SC-01's deferred custody half). Linked from ADR-0018, docs/privacy/session-custody.md's SC-05 section, and src/session/consent.ts's module doc, so a reader of any of them lands on what's left. States explicitly that ConsentAcknowledgment.record() takes no required arguments today, so the gate currently proves "a caller asserted consent," not "a human was shown the copy and agreed" — a gap that only closes once persistence and UI land. npm run ci and npm run test:canary both green after these changes. Co-Authored-By: Claude Opus 5 --- docs/decisions/ADR-0018-session-consent-gate.md | 10 ++++++++++ docs/privacy/session-custody.md | 4 +++- src/session/consent.ts | 15 ++++++++++++++- tests/unit/session-consent.test.ts | 9 +++++++-- 4 files changed, 34 insertions(+), 4 deletions(-) diff --git a/docs/decisions/ADR-0018-session-consent-gate.md b/docs/decisions/ADR-0018-session-consent-gate.md index 6f0730f..1a73671 100644 --- a/docs/decisions/ADR-0018-session-consent-gate.md +++ b/docs/decisions/ADR-0018-session-consent-gate.md @@ -168,6 +168,13 @@ consent and have that call *not* throw, which is exactly the case the test suite - **It is not legal review.** The copy is explicitly marked unreviewed. See that document's own "Open questions" section. +**All three of the above are tracked, not just implied by omission**: filed as +[#163](https://github.com/DevToolie/Paragent/issues/163), "the deferred half of SC-05" — the same +shape #146 gave SC-01's custody half after #98 landed the mechanism. That issue also names the +sharpest limitation of what ships here: `ConsentAcknowledgment.record()` takes no required +arguments, so what this guard proves today is "a caller asserted consent," not "a human was shown +the copy and agreed" — a distinction that only closes once persistence and UI land. + ## Consequences **`EstablishSessionOptions.baseUrl` is gone; every caller now supplies `target`.** This is a @@ -197,6 +204,9 @@ exists" argument ADR-0016 made for session-key custody. ## Open questions / what I could not verify +- **Tracked as [#163](https://github.com/DevToolie/Paragent/issues/163).** The next four bullets — + persistence, UI, expiry, and legal review — are exactly what that issue exists to resolve; it is + the place to look for whether any of them has since been answered, not just this list. - **The persistence and UI mechanism for the consent record are not designed here.** Decision 1 commits to "a stored consent record, checked before every session-establishing run" as the *shape*; where it is stored (a local config file, an OS keychain entry, something else), and what diff --git a/docs/privacy/session-custody.md b/docs/privacy/session-custody.md index 62d9c8a..673503e 100644 --- a/docs/privacy/session-custody.md +++ b/docs/privacy/session-custody.md @@ -294,7 +294,9 @@ rounded up to cover the others: `tests/unit/session-consent.test.ts` also asserts a private-LAN address (`192.168.x.x`) is **not** treated as local — loopback-only, not "looks internal." -**Not built, and said plainly rather than implied by omission:** +**Not built, and said plainly rather than implied by omission — tracked as +[#163](https://github.com/DevToolie/Paragent/issues/163), "the deferred half of SC-05," the same +shape #146 gave SC-01's custody half:** - **No persistence layer.** `ConsentAcknowledgment.record()` produces an in-memory acknowledgment; nothing reads or writes one from disk. Nothing in this repo establishes a session against a diff --git a/src/session/consent.ts b/src/session/consent.ts index 4e1a04c..c5caea8 100644 --- a/src/session/consent.ts +++ b/src/session/consent.ts @@ -43,6 +43,15 @@ * `ConsentAcknowledgment.record()` is the seam a future CLI banner or * onboarding step calls into once one exists; this module is the gate that * makes skipping it a refusal instead of a silent gap. + * + * **The sharpest limitation of what ships here:** `record()` takes no + * required arguments, so what this gate proves today is *"a caller asserted + * consent,"* not *"a human was shown the copy and agreed."* Nothing stops a + * future caller from calling `record()` without ever having displayed + * anything. That gap is harmless while nothing calls it against a real + * target, but it is exactly what persistence + UI landing together has to + * close — tracked as + * [#163](https://github.com/DevToolie/Paragent/issues/163). */ /** Copy version acknowledged by {@link ConsentAcknowledgment.record}. See `docs/privacy/session-consent-copy.md`. */ @@ -59,11 +68,15 @@ export const CONSENT_COPY_VERSION = "sc05-v1" as const; * narrower match would be a trap for the next port or interface choice, and * a real customer account is never reachable on loopback in the first place. */ +/** A single 0-255 octet — used instead of `\d{1,3}` so `127.999.999.999` (no such host resolves, but is not a loopback address either) does not match. */ +const OCTET = "(?:25[0-5]|2[0-4]\\d|1?\\d?\\d)"; +const LOOPBACK_V4 = new RegExp(`^127(?:\\.${OCTET}){3}$`); + export function isLocalHostname(hostname: string): boolean { const h = hostname.toLowerCase().replace(/^\[|\]$/g, ""); if (h === "localhost") return true; if (h === "::1" || h === "0:0:0:0:0:0:0:1") return true; - if (/^127(?:\.\d{1,3}){3}$/.test(h)) return true; + if (LOOPBACK_V4.test(h)) return true; return false; } diff --git a/tests/unit/session-consent.test.ts b/tests/unit/session-consent.test.ts index c55c327..a73dc27 100644 --- a/tests/unit/session-consent.test.ts +++ b/tests/unit/session-consent.test.ts @@ -44,6 +44,11 @@ describe("isLocalHostname / isLocalTarget (SC-05)", () => { expect(isLocalHostname("10.0.0.1")).toBe(false); }); + it("does not treat an out-of-range octet as local — no such host resolves, but it is not loopback either", () => { + expect(isLocalHostname("127.999.999.999")).toBe(false); + expect(isLocalHostname("127.256.0.1")).toBe(false); + }); + it("derives from the URL's hostname, ignoring port/path/query", () => { expect(isLocalTarget("http://127.0.0.1:3000/login?orgId=1")).toBe(true); expect(isLocalTarget("https://grafana.example.com:3000/login")).toBe(false); @@ -120,8 +125,8 @@ describe("SessionAuthorization.authorize — the refusal path (SC-05)", () => { const err = caught as ConsentRequiredError; expect(err.code).toBe("CONSENT_REQUIRED"); expect(err.baseUrl).toBe("https://real-customer.example.com"); - expect(err.message).toMatch(/real-customer\.example\.com/); - expect(err.message).toMatch(/SC-05/); + expect(err.message).toContain("real-customer.example.com"); + expect(err.message).toContain("SC-05"); }); it("does not refuse a local target even with an absurd path — locality is host-only", () => {