Skip to content

Commit 799b285

Browse files
os-zhuangclaude
andauthored
fix(plugin-security): mask read-protected fields on write responses (FLS leak) (#2692)
Field-level security masking ran only on find/findOne results, so a caller with edit-but-not-field-read could PATCH/POST a record and read a read-protected field back out of the mutation response image. Field writes were already denied (detectForbiddenWrites); this closes the read leak on the echoed post-image by extending the mask to insert/update results. Found by dogfooding the permission stack end-to-end (Studio package + permission sets + real users): an employee permission set with hr_note readable:false could still read hr_note via the PATCH response. Added regression tests for both update and insert echoed records. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent 2bee609 commit 799b285

3 files changed

Lines changed: 63 additions & 2 deletions

File tree

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
---
2+
"@objectstack/plugin-security": patch
3+
---
4+
5+
Fix field-level-security read leak on mutation responses. The security
6+
middleware only masked read-protected fields on `find`/`findOne` results, so a
7+
caller with edit-but-not-field-read could `insert`/`update` a record and read a
8+
read-protected field back out of the echoed post-image (field WRITES were
9+
already blocked, but the response image was not masked). The mask now also
10+
covers `insert`/`update` results, matching read behavior.

‎packages/plugins/plugin-security/src/security-plugin.test.ts‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -796,6 +796,52 @@ describe('SecurityPlugin', () => {
796796
});
797797
});
798798

799+
it('FLS write — the record echoed back by an update is masked (no read-leak of hidden fields)', async () => {
800+
// Regression: a caller with edit-but-not-field-read must not be able to
801+
// read a read-protected field back out of the mutation response. The write
802+
// itself only touches an editable field; the engine echoes the full row.
803+
const plugin = new SecurityPlugin({ fallbackPermissionSet: 'member_default' });
804+
const harness = makeMiddlewareCtx({
805+
permissionSets: [flsPolicySet],
806+
objectFields: ['id', 'owner_id', 'name', 'salary', 'ssn'],
807+
});
808+
await plugin.init(harness.ctx);
809+
await plugin.start(harness.ctx);
810+
const opCtx: any = {
811+
object: 'task',
812+
operation: 'update',
813+
data: { name: 'edited' }, // only editable field written → passes write gate
814+
// Engine's echoed post-image (pre-set; harness `next` is a no-op).
815+
result: { id: 't1', name: 'edited', salary: 9999, ssn: 'secret-leak' },
816+
context: { userId: 'u1', tenantId: 'org-1', roles: [], permissions: ['member_default'] },
817+
};
818+
await harness.run(opCtx);
819+
// ssn (readable:false) stripped; salary (readable:true) retained.
820+
expect(opCtx.result.ssn).toBeUndefined();
821+
expect(opCtx.result.salary).toBe(9999);
822+
expect(opCtx.result.name).toBe('edited');
823+
});
824+
825+
it('FLS write — the record echoed back by an insert is masked', async () => {
826+
const plugin = new SecurityPlugin({ fallbackPermissionSet: 'member_default' });
827+
const harness = makeMiddlewareCtx({
828+
permissionSets: [flsPolicySet],
829+
objectFields: ['id', 'owner_id', 'name', 'salary', 'ssn'],
830+
});
831+
await plugin.init(harness.ctx);
832+
await plugin.start(harness.ctx);
833+
const opCtx: any = {
834+
object: 'task',
835+
operation: 'insert',
836+
data: { name: 'A' },
837+
result: { id: 't2', name: 'A', salary: 100, ssn: 'server-generated' },
838+
context: { userId: 'u1', tenantId: 'org-1', roles: [], permissions: ['member_default'] },
839+
};
840+
await harness.run(opCtx);
841+
expect(opCtx.result.ssn).toBeUndefined();
842+
expect(opCtx.result.salary).toBe(100);
843+
});
844+
799845
it('fails CLOSED when permission resolution throws — denies, never bypasses (P0-2)', async () => {
800846
const plugin = new SecurityPlugin({ fallbackPermissionSet: 'member_default' });
801847
const harness = makeMiddlewareCtx({ permissionSets: [tenantPolicySet] });

‎packages/plugins/plugin-security/src/security-plugin.ts‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -797,8 +797,13 @@ export class SecurityPlugin implements Plugin {
797797

798798
await next();
799799

800-
// 4. Field-level security: mask restricted fields in read results
801-
if (opCtx.result && ['find', 'findOne'].includes(opCtx.operation)) {
800+
// 4. Field-level security: mask restricted fields in returned records.
801+
// Covers reads AND the record echoed back by a write — otherwise a caller
802+
// with edit-but-not-field-read could PATCH a record and read a
803+
// read-protected field back out of the mutation response (FLS bypass).
804+
// Field WRITES are already blocked upstream (detectForbiddenWrites); this
805+
// closes the read leak on the response image.
806+
if (opCtx.result && ['find', 'findOne', 'insert', 'update'].includes(opCtx.operation)) {
802807
let fieldPerms = this.permissionEvaluator.getFieldPermissions(opCtx.object, permissionSets);
803808
// [ADR-0066 D3] AND-gate field-level requiredPermissions into the mask.
804809
fieldPerms = this.foldFieldRequiredPermissions(fieldPerms, secMeta.fieldRequiredPermissions, permissionSets);

0 commit comments

Comments
 (0)