From 7ab2614a4d3e613d22cdee00c2731bde8ba5e644 Mon Sep 17 00:00:00 2001 From: Dan Draper Date: Sun, 18 Oct 2026 17:55:41 +1100 Subject: [PATCH] docs: record what a stored value authenticates, and tell reviewers what is not a finding Agent reviews have flagged, four or five times, that a stored container's membership and order are not authenticated. That is deliberate (vitaminc ADR 0002): stored values are rows and columns, read in any order and in part. stack-encrypt ADR 0010 applies it to what Stack Encrypt stores: each leaf is bound to its context, key, role, type and version; a container's membership and order, a passthrough field (except a context field), and which row a leaf belongs to are not. AGENTS.md gains "Review guidelines" with these as known non-findings (Codex reads it; CLAUDE.md imports it for Claude Code's /code-review), and .github/copilot-instructions.md repeats them for Copilot. --- .github/copilot-instructions.md | 20 +++++++ AGENTS.md | 33 +++++++++++ .../0010-what-a-stored-value-authenticates.md | 59 +++++++++++++++++++ 3 files changed, 112 insertions(+) create mode 100644 .github/copilot-instructions.md create mode 100644 packages/stack-encrypt/docs/adr/0010-what-a-stored-value-authenticates.md diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md new file mode 100644 index 000000000..f36468fef --- /dev/null +++ b/.github/copilot-instructions.md @@ -0,0 +1,20 @@ +# Copilot instructions for cipherstash/stack + +The project instructions are in `AGENTS.md`. When reviewing a pull request, +follow its "Review guidelines", in particular these known non-findings, +which are deliberate design decisions and not defects: + +- A sealed container's membership, order and length are not authenticated: + which entries a stored map holds, and a stored sequence's order, length + and duplicates, are not bound into any AAD, because stored values are + database rows and columns that come back in any order and in part. A + target that refuses a container's shape is a shape mismatch + (`Error::OpenedType`), not tampering (`Error::Aead`). +- A passthrough field is stored in the clear and is not authenticated, + except a plan's context field, which every other field is sealed under. +- Two leaves sealed under the same context are interchangeable between rows; + binding a value to its row is the caller's choice of context. + +A leaf that is not bound to its context, map key, role (single value or +sequence element), type tag or wire version is still a defect. See +`packages/stack-encrypt/docs/adr/0010-what-a-stored-value-authenticates.md`. diff --git a/AGENTS.md b/AGENTS.md index 662389482..875433e5a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -975,6 +975,39 @@ pnpm changeset:publish A skills-only change is **not** internal: `skills/` ships inside the `stash` tarball, so it needs a `stash` patch changeset. +## Review guidelines + +These apply to every agent that reviews a pull request here, human-prompted +or automated (Codex, Copilot, CodeRabbit, Claude Code's `/code-review`). + +### Known non-findings + +These are deliberate design decisions, recorded in ADRs. Do not report them +as defects or security issues, and do not change code to "fix" them. If a +change seems to need one reversed, say so and link the ADR. + +- **A sealed container's membership, order and length are not + authenticated.** Each leaf is bound to its context, its map key, its role + (single value or sequence element), its type tag and the wire version. But + which entries a stored map holds, and a stored sequence's order, length + and duplicates, are deliberately not bound: stored values are database + rows and columns, which come back in any order and in part. A target that + refuses a container's shape (a missing field, say) is a shape mismatch, + `Error::OpenedType`, not tampering, `Error::Aead`. Stricter authentication + will be opt-in, through named types. See + `packages/stack-encrypt/docs/adr/0010-what-a-stored-value-authenticates.md` + and vitaminc's ADR 0002. +- **A passthrough field is not authenticated.** It is stored in the clear, + by definition, and comes back as the store holds it. A plan's context + field is the exception: every other field is sealed under it, so a change + to it is caught. Same ADR. +- **Two leaves sealed under the same context are interchangeable between + rows.** Binding a value to its row is the caller's choice of context (a + row id or tenant in it). Same ADR. + +A finding that a *leaf* is not bound to its context, key, role, type or +version is a real defect: report it. + ## Useful Links - `README.md` for quickstart and feature overview diff --git a/packages/stack-encrypt/docs/adr/0010-what-a-stored-value-authenticates.md b/packages/stack-encrypt/docs/adr/0010-what-a-stored-value-authenticates.md new file mode 100644 index 000000000..b53aed707 --- /dev/null +++ b/packages/stack-encrypt/docs/adr/0010-what-a-stored-value-authenticates.md @@ -0,0 +1,59 @@ +--- +status: accepted +date: 2026-10-18 +extends: vitaminc ADR 0002 +--- + +# What a stored value authenticates + +Automated reviews of Stack Encrypt keep reporting the same properties as +vulnerabilities. They are deliberate. This ADR says what a value Stack +Encrypt stores does and does not authenticate, so a reviewer can check a +finding against it. The sealing itself is vitaminc's, and the underlying +decision is vitaminc's ADR 0002 ("Leaves are authenticated; a container's +membership and order are not"). This applies it to what Stack Encrypt +stores. + +## Decision + +**Authenticated**, for every sealed leaf (`"c"`, and each leaf inside a +nested value): its bytes and type tag, the context it was sealed under +(including any extension parts and a record's context field), its map key +or its role as a sequence element, and the wire version. A leaf that opens +was sealed exactly so, under exactly that context. + +**Not authenticated, by design:** + +- **A container's membership, order and length.** Which entries a stored + map holds, and a stored sequence's order, length and duplicates. A + sequence of records is a set of rows, which a database returns in any + order and in part. A record's fields are a row's columns, which a query + selects in part. +- **A passthrough field.** It is stored in the clear so the record stays + whole, and comes back exactly as the store holds it. Anyone who can write + to the store can change it. The exception is a plan's context field: it + is stored as a passthrough, but every other field was sealed under it, so + a changed context field fails the expected-context check or leaves the + sealed fields unable to open. +- **Which row a leaf belongs to**, unless the context names the row. Two + leaves sealed under the same column context are interchangeable between + rows. A caller who needs a value bound to its row puts the row's identity + (or a tenant) in the context. + +## Consequences + +- **A typed open separates a failed leaf from a refused shape.** A leaf + that does not open (wrong key, wrong context, altered bytes) is + `Error::Aead`. A value whose every leaf opened but whose type or shape the + target refuses, including a container missing a field the target needs, + is a shape mismatch: `Error::OpenedType`, not `Aead`. The Go guest maps + them to `STATUS_AUTH` and `STATUS_ENCODING`. +- **Review findings of these kinds are not defects.** `AGENTS.md` lists + them under "Review guidelines", and `.github/copilot-instructions.md` + repeats them. A finding that a *leaf* is not bound to its context, key, + role, type or version is still a defect. +- **Stricter authentication is opt-in, by type.** A caller who needs a + sequence's positions or a record's field set authenticated will ask for it + by naming a type that carries that rule, the way `serde_bytes` selects a + different encoding by type (vitaminc ADR 0002). It is not a default and + not a flag, and it does not exist yet.