Skip to content

Commit e9e24aa

Browse files
committed
docs(qa): deliver the two security-sensitive write-ups (D9, D10) per the #7463 ruling
The maintainer ruling on #7463 (2026-08-11) formally requested the private write-up of the cross-persona data-disclosure finding, on the D1 precedent. Delivered here as D9, together with D10 from the platform-core run (#7514), which was held back for the same reason. D9 expand bypasses the CRUD gate and the OWD scope on the sub-read — a contributor reads a contact they are 403'd from reading directly, byte -identical to the admin's response. The #2850 waiver keys on access.default while objects declare sharingModel, and access is null for every object in the built artifact, so the waiver fires for every referenced object. The unit pin asserts only the re-entry tag, never the authorization outcome. D10 encrypted settings are echoed in plaintext by the namespace read. Storage is correct (sys_secret, aes-256-gcm); the REST read decrypts and repeats the secret in cascadeChain. Admin-gated, so defense-in-depth not escalation. Each carries impact, a 2/2 reproduction, the located root cause, why the existing pin stayed green, and a non-prescriptive fix shape. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YD9f6FYyMraUWYeJf53V43
1 parent 97b6658 commit e9e24aa

1 file changed

Lines changed: 73 additions & 0 deletions

File tree

‎docs/qa/platform-checklist/FOLLOW-UPS.md‎

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,79 @@ gaps. The one security-sensitive finding (D1) has since been fixed in #6683.
2323
| D6 | **`/api/v1/datasources` admin CRUD has no route ledger** — mounted by serve.ts, absent from rest-route-ledger.ts (tranche-3 discipline gap). | packages/services/service-datasource/src/admin-routes.ts | integration-system.datasource-admin-lifecycle (source note) | low — internal discipline |
2424
| D7 | **Parent-only PATCH does not revalidate a stale dependent child** — `evaluateOptionVisibility` skips fields absent from the payload, so changing only the parent leaves a now-invalid child value in place server-side; integrity rests entirely on the client clear. | packages/objectql/src/validation/rule-validator.ts (`!(name in data) continue`) | records-forms.cascading-multilevel-and-clear (knownGap) | integrity — safe to file |
2525
| D8 | **Lookup cascade scope is existence-only server-side** — `assertReferencesResolve` accepts any EXISTING id regardless of `lookupFilters` scope (a cross-account contact that exists is accepted on direct POST). May be by-design (filters = UI courtesy) — needs a maintainer ruling: declared ≠ enforced, or documented courtesy. | packages/objectql/src/engine.ts (assertReferencesResolve) | records-forms.cascading-multilevel-and-clear (knownGap) | integrity/design — needs ruling |
26+
| D9 | **`expand` discloses a record the caller is 403'd from reading** — the `#2850` expand waiver keys on the wrong axis, so it fires for *every* referenced object including ones the caller holds no grant on. | packages/plugins/plugin-security/src/security-plugin.ts (`expandSkipCrud`) | api-backend.query-contract-matrix clause 4 | **SECURITY — write-up below (§1a)** |
27+
| D10 | **Encrypted settings are echoed in plaintext on read** — storage is correct (`sys_secret`, aes-256-gcm) but the REST read decrypts and returns the secret verbatim, and repeats it in `cascadeChain`. | packages/services/service-settings/src/{settings-service.ts,settings-routes.ts} | platform-core.settings-hub-roundtrip clause 7 | **SECURITY (admin-gated) — write-up below (§1a)** |
28+
29+
### §1a — Security-sensitive write-ups (delivered 2026-08-11, per the maintainer ruling on #7463)
30+
31+
Held out of the public run cards (#7463, #7514) pending a disclosure decision, and delivered
32+
here on the D1 precedent. Both were found by real runs against a live server; neither is
33+
reachable from a unit test, which is why both pins stayed green.
34+
35+
#### D9 — `expand` bypasses the CRUD gate and the OWD scope on the sub-read
36+
37+
**Impact.** A low-privilege authenticated user reads records they are explicitly denied.
38+
Not an admin-only weakness: the probing persona held only the `contributor` position.
39+
40+
**Reproduction (2/2).** As a user holding only `contributor`:
41+
1. `GET /api/v1/data/showcase_contact/<contactId>` → **403 PERMISSION_DENIED**
42+
(`operation 'findOne' on object 'showcase_contact' is not permitted for positions
43+
[org_member, contributor, everyone]`).
44+
2. Same session, on an invoice **they own** that references that contact:
45+
`GET /api/v1/data/showcase_invoice?$expand=contact` → **200 with the contact fully
46+
materialised** — all 18 fields including `email` — **byte-identical to the admin's
47+
response** (`JSON.stringify` equality true).
48+
3. Also via the body form: `POST /api/v1/data/showcase_invoice/query`
49+
`{"expand":{"contact":{"object":"showcase_contact","fields":["id","name"]}}}`.
50+
51+
`showcase_contact` is `sharingModel:'private'` and the row is admin-owned, so **both** the
52+
CRUD gate and the OWD row scope were bypassed on the expand sub-read. RLS on the *direct*
53+
path still works (a contributor's query for a foreign invoice returns 200 with 0 rows), so
54+
this is specific to the expand seam.
55+
56+
**Root cause (located).** `expandSkipCrud = operation === 'find' && context.__expandRead && !secMeta.isPrivate`,
57+
where `secMeta.isPrivate` reads `obj.access?.default === 'private'`. That is a **different
58+
axis** from `sharingModel`, and `access` is `null` for every object in the built artifact —
59+
so the waiver fires for every referenced object, including ones the caller holds no grant
60+
on. The waiver's premise ("already broadly readable") does not hold.
61+
62+
**Why the pin didn't catch it.** The `#2850` unit pin asserts only that the sub-read
63+
re-enters `find()` tagged `__expandRead`; it never asserts the end-to-end authorization
64+
outcome, so it passes while the live server discloses.
65+
66+
**Suggested shape of a fix (not prescriptive).** Either evaluate the waiver against the
67+
same axis the object actually declares (`sharingModel`), or drop the waiver and let the
68+
sub-read carry the caller's context through the normal gate. Any fix wants an end-to-end
69+
both-personas assertion, not a re-tagged unit pin.
70+
71+
#### D10 — `GET /api/settings/:namespace` returns the plaintext of encrypted specifiers
72+
73+
**Impact.** Stored credentials (SMTP password, API keys) are readable back over the API by
74+
any caller holding `setup.access`. Admin-gated (anonymous → 403), so this is a
75+
defense-in-depth failure rather than a privilege escalation — but it defeats the point of
76+
encrypting them at rest.
77+
78+
**Reproduction (2/2, on both specifier flavours).** Write a secret through
79+
`PUT /api/settings/mail`, then observe the split:
80+
- **Storage is correct** — `sys_setting` row has `value: null`, `encrypted: true`,
81+
`value_enc: 'sec_<hex>'`, and the matching `sys_secret` row holds the aes-256-gcm
82+
ciphertext (`kms_key_id: 'local:v1'`, `alg: 'aes-256-gcm'`).
83+
- **The read echoes it** — `GET /api/settings/mail` returns the written plaintext in
84+
`values.<key>.value`, and repeats it inside every `cascadeChain` entry.
85+
86+
Affects both `type: 'password'` and `encrypted: true` specifiers.
87+
88+
**Root cause (located).** `settings-service.ts materialiseRow()` dereferences `sec_` handles
89+
and **decrypts** to plaintext; `getNamespace()` copies that plaintext into every cascade
90+
entry; `settings-routes.ts` applies **no redaction** on the way out. The service-layer
91+
round-trip is deliberately pinned (`settings-service.test.ts`: *"Round-trip read returns the
92+
plaintext"*), so the missing boundary is the **REST read**, not the service.
93+
94+
**Suggested shape of a fix (not prescriptive).** Redact at the route: return a set-flag /
95+
masked marker instead of the value for any specifier that is `encrypted` or
96+
`type: 'password'`, and scrub `cascadeChain` the same way. If some internal caller genuinely
97+
needs the plaintext, that should be an explicit service-layer call, not the namespace read
98+
the settings UI uses.
2699

27100
## 2. Docs promise capabilities the runtime doesn't deliver (PD#10, docs side — file docs issues)
28101

0 commit comments

Comments
 (0)