Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 75 additions & 0 deletions packages/spec/src/contracts/sharing-service.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -51,3 +51,78 @@ describe('Sharing Service Contract — recipient vocabularies (#4539)', () => {
expect(ruleRecipient).toBe(notAuthorable);
});
});

/**
* [#5125] The three WRITE gates must all document the `modifyAllRecords`
* super-user bypass.
*
* #4647 made the bypass EXPLICIT on the enforcement side: `canEdit`,
* `canDelete` and `canManageShares` all fold through the one
* `ISecurityService.hasWriteBypass` predicate (`hasModifyAllBypass` in
* `@objectstack/plugin-sharing`'s `SharingService`). Two of the three
* docstrings said so; `canEdit`'s did not, and its omission read as a
* deliberate exclusion — the exact opposite of the implementation, and
* strictly worse than silence because `canDelete` sits four lines below
* naming the bypass it supposedly does not share.
*
* A prose pin rather than a behavioural one, because prose is what drifted:
* nothing type-checks a doc comment, and this interface's whole job is to be
* the thing cross-package callers read instead of the plugin. Deleting the
* sentence again turns this red.
*/
describe('[#5125] ISharingService write-gate bypass documentation parity', () => {
it('canEdit / canDelete / canManageShares each name the `modifyAllRecords` bypass', async () => {
const ts = (await import('typescript')).default;
const { readFileSync } = await import('node:fs');
const { dirname, resolve } = await import('node:path');
const { fileURLToPath } = await import('node:url');

const file = resolve(dirname(fileURLToPath(import.meta.url)), 'sharing-service.ts');
const source = ts.createSourceFile(
file,
readFileSync(file, 'utf8'),
ts.ScriptTarget.Latest,
/* setParentNodes */ true,
);

const iface = source.statements.find(
(s): s is import('typescript').InterfaceDeclaration =>
ts.isInterfaceDeclaration(s) && s.name.text === 'ISharingService',
);
expect(iface, 'ISharingService must still be an interface in this file').toBeDefined();

// `getFullText` carries a member's LEADING TRIVIA — its doc comment — so
// the pin reads exactly what an IDE shows on hover, with no assumption
// about how the comment is wrapped.
const docOf = new Map<string, string>();
for (const member of iface!.members) {
if (!ts.isMethodSignature(member) || !member.name || !ts.isIdentifier(member.name)) continue;
docOf.set(member.name.text, member.getFullText(source));
}

// Anti-vacuity 1: the enumeration found the real contract surface, so a
// rename cannot quietly empty the assertions below.
expect([...docOf.keys()].sort()).toEqual([
'buildReadFilter',
'canDelete',
'canEdit',
'canManageShares',
'grant',
'listShares',
'revoke',
]);

for (const gate of ['canEdit', 'canDelete', 'canManageShares'] as const) {
expect(docOf.get(gate), `${gate} must document the modifyAllRecords bypass`)
.toContain('modifyAllRecords');
}

// Anti-vacuity 2: the search DISCRIMINATES — it is not matching text every
// member happens to carry. `buildReadFilter` is the honest negative: the
// read path has no `hasWriteBypass` branch at all (a View/Modify All Data
// holder reaches every row because the security layer resolves read DEPTH
// to `org`, which short-circuits the filter before sharing is consulted),
// so naming the write bypass there would itself be drift.
expect(docOf.get('buildReadFilter')).not.toContain('modifyAllRecords');
});
});
13 changes: 11 additions & 2 deletions packages/spec/src/contracts/sharing-service.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -126,9 +126,18 @@ export interface ISharingService {

/**
* Return `true` when the principal in `context` may UPDATE the record
* `(object, recordId)`. Ownership (widened by write DEPTH) OR a write-level
* ({@link ShareAccessLevel} `edit`) share. Always true for system context,
* `(object, recordId)`. Ownership (widened by write DEPTH), a write-level
* ({@link ShareAccessLevel} `edit`) share, OR — [#4647] — the
* `modifyAllRecords` super-user bypass. Always true for system context,
* `public` objects, and objects with no owner field.
*
* The bypass is the same `ISecurityService.hasWriteBypass` predicate
* {@link canDelete} and {@link canManageShares} consult, so the three write
* gates cannot drift from each other or from what `security/explain`
* reports. It is asked LAST — after ownership and shares — so an ordinary
* write costs no extra resolution, and it **fails CLOSED** (ADR-0111 D2):
* no security service, a throwing probe, or a principal-less /
* on-behalf-of context leaves the answer at owner-plus-share only.
*/
canEdit(
object: string,
Expand Down
Loading