Skip to content

Commit fc844c4

Browse files
committed
fix(service-analytics): the queryable fallback reads no caller property, so it refuses masked-rule fields for every caller
The fallback for a security service that cannot answer getQueryableFields exempted a system context by reading its system bit, a new elevation read site. It cannot say for whom a masking rule is lifted, so it now refuses every caller alike; the contract text and both changesets say so. Claude-Session: https://claude.ai/code/session_01Sfe5YjBLwB9J3y8fvm2xq1 Co-authored-by: Claude <noreply@anthropic.com>
1 parent 03153e0 commit fc844c4

6 files changed

Lines changed: 16 additions & 15 deletions

File tree

‎.changeset/20935-analytics-masked-field-not-queryable.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ unaffected. A query that names no masked field answers as before.
2828
supplies the answer. `AnalyticsServicePlugin` wires it to the `security`
2929
service's `getQueryableFields`. When that service predates the method, or
3030
answers "no answer", the plugin treats every field that declares a
31-
`maskingRule` as not queryable, for every caller but a system one. A host that
31+
`maskingRule` as not queryable, for every caller. A host that
3232
constructs `AnalyticsService` itself with `getReadableFields` and without
3333
`getQueryableFields` is warned once at construction.
3434

‎.changeset/20935-security-service-queryable-fields.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,4 +8,4 @@ Clause-②: yes (widening)
88

99
- It is the exact complement of the fields the engine's field guards refuse when a query names them as a filter, a sort key, a group key or an aggregate input. It is a subset of `getReadableFields`, and the two differ by exactly the fields the caller is served masked: a field whose `maskingRule` applies to the caller is readable (served, its value replaced) and not queryable.
1010
- It fails soft like `getReadableFields`: `undefined` means no answer, `[]` means no field is queryable. A system context gets every field.
11-
- It is optional. A consumer checks `typeof svc.getQueryableFields === 'function'`. When the method is missing, or answers `undefined`, the consumer must treat every field that declares a `maskingRule` as not queryable (a system context excepted). Falling back to `getReadableFields` alone would admit exactly the masked fields.
11+
- It is optional. A consumer checks `typeof svc.getQueryableFields === 'function'`. When the method is missing, or answers `undefined`, the consumer must treat every field that declares a `maskingRule` as not queryable, whoever the caller is. Falling back to `getReadableFields` alone would admit exactly the masked fields.

‎packages/services/service-analytics/src/__tests__/field-query-admission-gate.test.ts‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -281,9 +281,10 @@ describe('[#20935] analytics plugin — the query-side half of the "security" br
281281
// A field that declares no rule is judged by the read projection alone, as before.
282282
await service.query(groupedPlain as never, CALLER);
283283
expect(reads).toHaveLength(1);
284-
// The contract's system bypass holds on the fallback too.
285-
await service.query(groupedMasked as never, SYSTEM);
286-
expect(reads).toHaveLength(2);
284+
// The fallback reads no caller property — it cannot say for whom a rule is
285+
// lifted — so it refuses every caller, a system one included.
286+
await expect(service.query(groupedMasked as never, SYSTEM)).rejects.toMatchObject({ code: 'PERMISSION_DENIED', status: 403, fields: ['masked_code'] });
287+
expect(reads).toHaveLength(1);
287288
});
288289

289290
it('fails CLOSED the same way when getQueryableFields answers "no answer" (undefined)', async () => {

‎packages/services/service-analytics/src/analytics-service.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -731,7 +731,8 @@ export interface AnalyticsServiceConfig {
731731
* The plugin auto-bridges this to the `security` service's
732732
* `getQueryableFields`, and when that service predates the method, or
733733
* answers `undefined`, it fails CLOSED: every field that declares a
734-
* `maskingRule` is treated as not queryable. MAY be async; a THROW refuses
734+
* `maskingRule` is treated as not queryable, whoever the caller is. MAY be
735+
* async; a THROW refuses
735736
* the query. A host that wires {@link getReadableFields} and not this judges
736737
* masked fields by the read projection alone, which admits them — the
737738
* service says so once, at construction.

‎packages/services/service-analytics/src/plugin.ts‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -814,11 +814,12 @@ export class AnalyticsServicePlugin implements Plugin {
814814
// `undefined`. The fallback is NOT the read projection alone, which counts a
815815
// masked field readable and would admit exactly the queries this half
816816
// exists to refuse. It fails CLOSED: the read projection less every field
817-
// whose declaration carries a `maskingRule`, for every caller but a system
818-
// one (the contract's own bypass). That over-refuses a caller the rule is
819-
// lifted for, which is the safe direction and the only one available: an
820-
// older reader cannot say for whom a rule is lifted, and deciding that here
821-
// would be a second copy of the masking rule.
817+
// whose declaration carries a `maskingRule`, whoever the caller is. That
818+
// over-refuses a caller the rule is lifted for — a system one included —
819+
// which is the safe direction and the only one available: an older reader
820+
// cannot say for whom a rule is lifted, and deciding that here (reading the
821+
// caller's capabilities, or its system bit) would be a second copy of the
822+
// masking rule.
822823
interface SecurityQueryableFields extends SecurityReadableFields {
823824
getQueryableFields?(object: string, context?: ExecutionContext): Promise<string[] | undefined>;
824825
}
@@ -858,7 +859,6 @@ export class AnalyticsServicePlugin implements Plugin {
858859
}
859860
const readable = await svc.getReadableFields(object, context);
860861
if (readable === undefined) return undefined;
861-
if ((context as { isSystem?: unknown } | undefined)?.isSystem === true) return readable;
862862
const masked = maskingRuleFields(object);
863863
return readable.filter((f) => !masked.has(f));
864864
};

‎packages/spec/src/contracts/security-service.ts‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -379,9 +379,8 @@ export interface ISecurityService {
379379
* (`typeof svc.getQueryableFields === 'function'`). ⛔ A consumer that cannot
380380
* get this answer — the method is absent, or it answered `undefined` — must
381381
* treat every field that declares a `maskingRule` as NOT queryable, whoever
382-
* the caller is (only a system context is exempt): the older reader cannot
383-
* say for whom a rule is lifted, and the read projection reports a masked
384-
* field as readable. Falling back to the read projection alone fails OPEN on
382+
* the caller is: the older reader cannot say for whom a rule is lifted, and
383+
* the read projection reports a masked field as readable. Falling back to the read projection alone fails OPEN on
385384
* precisely the fields this method exists for. Declaring it optional keeps
386385
* that degradation a property of the type: the unguarded call does not
387386
* compile, so a consumer cannot skip its fallback by accident.

0 commit comments

Comments
 (0)