Skip to content

Commit 3c54bc9

Browse files
committed
test(plugin-security): pin the reorder's own consequences on both legs
Moving the two strips ahead of the seam also moves them ahead of the credential loop, and that changes what happens to a caller-supplied value on a `readonly` credential column. Both directions are now measured against the reviewed head's `engine.ts` rather than argued: - a caller-forged `readonly` `secret` field was ENCRYPTED AND STORED on cd09d3b (`token: "secret:sec_1"`, one encrypt call, one sys_secret row): the credential channel ran first and replaced the row's value with a reference, so the strip's `Object.is` value test compared a ref against the caller's plaintext, read the difference as a hook write, and kept the forgery. Pre-existing on 17.3.0; closed by the reorder, and now pinned. - an empty string on a `readonly` `password` field answered VALIDATION_ERROR there and is stripped here. The 2026-08-13 ruling's guarantee is intact — `""` reaches the store on neither order — but the refusal a caller sees moves, so it is recorded rather than left to be discovered. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37
1 parent c676b9d commit 3c54bc9

1 file changed

Lines changed: 91 additions & 0 deletions

File tree

packages/plugins/plugin-security/src/insert-check-post-image.test.ts

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,20 @@ const OBJECTS = [
185185
token: { name: 'token', type: 'secret' },
186186
},
187187
},
188+
// The contract review's F1 reorder moves both strips ahead of the credential
189+
// channel. These two columns are where that is OBSERVABLE rather than argued:
190+
// an author-declared `readonly` on a credential field.
191+
{
192+
name: 'qa_ro_cred',
193+
label: 'Readonly credential columns',
194+
sharingModel: 'public_read_write',
195+
fields: {
196+
id: { name: 'id', type: 'text', primaryKey: true },
197+
name: { name: 'name', type: 'text' },
198+
token: { name: 'token', type: 'secret', readonly: true },
199+
pw: { name: 'pw', type: 'password', readonly: true },
200+
},
201+
},
188202
// The secret store the credential channel writes into, declared here so
189203
// `syncSchemas()` creates a real table for it and "no secret was minted" can
190204
// be READ rather than inferred.
@@ -222,6 +236,9 @@ const EMPLOYER_ADMIN: PermissionSet = PermissionSetSchema.parse({
222236
qa_ro_member_default: { allowRead: true, allowCreate: true, allowEdit: true },
223237
qa_unevaluable_member: { allowRead: true, allowCreate: true, allowEdit: true },
224238
qa_cost_member: { allowRead: true, allowCreate: true, allowEdit: true },
239+
// No `rowLevelSecurity` policy for this one on purpose: its cells are about
240+
// the ENGINE's pass order, not about the check gate.
241+
qa_ro_cred: { allowRead: true, allowCreate: true, allowEdit: true },
225242
},
226243
rowLevelSecurity: [
227244
{
@@ -906,3 +923,77 @@ for (const [driverName, makeDriver] of DRIVERS) {
906923
});
907924
});
908925
}
926+
927+
/**
928+
* [contract review F1, the reorder's OWN consequences — measured, both legs]
929+
*
930+
* Moving the two strips ahead of the seam also moves them ahead of the
931+
* credential loop (`refuseEmptyPasswordFields` + `encryptSecretFields`), which
932+
* used to run first. That is not a detail to wave at: it changes what happens
933+
* to a caller-supplied value on a `readonly` CREDENTIAL column, in two
934+
* different directions, and both were measured on `cd09d3b99` (the reviewed
935+
* head, `engine.ts` checked out over this tree) and on the fix.
936+
*
937+
* ⚠️ These cells run without a `check` policy and outside the RLS gate
938+
* entirely — they are about the ENGINE's pass order, which is what the F1 fix
939+
* moved. They live here because this file is where that reorder is justified.
940+
*/
941+
describe('[#16608 F1] what moving the strips ahead of the credential channel changes', () => {
942+
const OBJ = 'qa_ro_cred';
943+
// The same non-system caller the cells above use — granted create on this
944+
// object, and governed by NO `check`, so nothing here is the RLS gate's doing.
945+
946+
it('a caller-forged `readonly` `secret` field is stripped, so nothing is stored and no secret is minted', async () => {
947+
// MEASURED on the reviewed head cd09d3b99, same harness:
948+
// stored `token: "secret:sec_1"` · encrypt calls 1 · sys_secret rows 1
949+
// The forgery REACHED THE STORE. `encryptSecretFields` ran first and
950+
// replaced the row's value with a reference, so the strip's `Object.is`
951+
// value test then compared a REF against the caller's plaintext, read the
952+
// difference as "a hook rewrote this key", and KEPT it — the one input
953+
// where that test inverts. Pre-existing on 17.3.0, closed by the reorder.
954+
const booted = await boot(DRIVERS[0]![1]);
955+
956+
const outcome = await attempt(() =>
957+
booted.engine.insert(
958+
OBJ,
959+
{ id: 'cred_1', name: 'n', token: 'forged-plaintext' },
960+
{ context: CALLER } as never,
961+
),
962+
);
963+
expect(outcome.ok, `expected the insert to be admitted with the forgery dropped: ${outcome.message}`).toBe(true);
964+
965+
const rows = await booted.table(OBJ, ['id', 'token']);
966+
expect(rows).toHaveLength(1);
967+
expect(rows[0]!.token, 'a caller may not seed a `readonly` credential column').toBeNull();
968+
expect(booted.crypto.encrypt, 'nothing was encrypted for a value the strip discards').toBe(0);
969+
expect(await booted.table('sys_secret', ['id']), 'and no sys_secret row was minted').toEqual([]);
970+
});
971+
972+
it('an empty string on a `readonly` `password` field is stripped rather than refused — and `""` still never reaches the store', async () => {
973+
// ⚠️ THE ONE DIRECTION OF THE REORDER THAT IS NOT A NARROWING, recorded
974+
// here so it is visible rather than discovered. MEASURED on the reviewed
975+
// head cd09d3b99, same harness:
976+
// VALIDATION_ERROR — 'Empty string refused for password field
977+
// "qa_ro_cred.pw"' · nothing stored
978+
// and on the fix: admitted, `pw` stored as NULL.
979+
//
980+
// What the 2026-08-13 ruling guarantees is that a masked credential column
981+
// never holds `""` while every read reports "a password is set". That
982+
// guarantee is INTACT — `""` is discarded on both orders; only which
983+
// refusal a caller sees moved, on a payload the caller was never allowed to
984+
// send. ⛔ The seam's 403 deliberately still precedes this: moving
985+
// `refuseEmptyPasswordFields` up too would let a field-level validation
986+
// verdict answer a write that RLS refuses, which is the wrong precedence
987+
// for a security gate.
988+
const booted = await boot(DRIVERS[0]![1]);
989+
990+
const outcome = await attempt(() =>
991+
booted.engine.insert(OBJ, { id: 'cred_2', name: 'n', pw: '' }, { context: CALLER } as never),
992+
);
993+
expect(outcome.ok).toBe(true);
994+
995+
const rows = await booted.table(OBJ, ['id', 'pw']);
996+
expect(rows).toHaveLength(1);
997+
expect(rows[0]!.pw, 'the empty credential is not stored — the ruling’s guarantee, unchanged').toBeNull();
998+
});
999+
});

0 commit comments

Comments
 (0)