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
23 changes: 23 additions & 0 deletions .changeset/security-plugin-console-backed-default-sink.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
---
"@objectstack/plugin-security": minor
---

`SecurityPlugin`'s own report sink is now **console-backed by default** — loud until a host
injects one — instead of being initialised to an empty object. Its fail-closed refusals
(`getReadFilter … denying (fail-closed, #2852)` and `#4467`, `checkAuthoredRowWrite …
abstaining`, the ADR-0123 tenant-wall refusal) previously went nowhere at all on any instance
whose lifecycle had not yet reached the sink binding; they now reach `console.warn` /
`console.error`. A host that injects a logger is unaffected: `start()` assigns `ctx.logger`
over the default, above both of its early bail-outs (#10706), so a degraded boot still reports
through the host.

**Operator-visible:** a deployment that never injects a sink will begin seeing these refusals
on the console. That is the intended change — the refusal itself is not moving, only whether
anyone can see it.

Why `minor` and not `patch`: the observable output of a running deployment changes. The
declared shape changes with it — the field's `warn` channel is now non-optional, which is what
#9754 requires of a sink declaring an optional `error`, and what a default of `{}` made
impossible to state honestly. `error` deliberately stays optional (#9754 option C, falsified:
hosts do inject reduced sinks). The maintainer ruled on 2026-08-24 (#10556) that the default
becomes console-backed and that silent-by-declaration is rejected.
24 changes: 24 additions & 0 deletions .changeset/sharing-options-logger-guaranteed-warn.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
---
"@objectstack/plugin-sharing": minor
---

⚠️ **Published-contract break for external hosts.** The `logger` option on the three
PUBLICLY EXPORTED options types — `SharingServiceOptions`, `ShareLinkServiceOptions` and
`SharingRuleServiceOptions` — now **requires** a `warn` channel, and its members carry real
signatures (`(msg: any, ...rest: any[]) => void`) instead of bare `Function`. A host that
constructs any of these services with `{ logger: { info, error } }` compiles today and stops
compiling after this release; add `warn` (or drop the `logger` option) to migrate. Nothing
about the services' runtime behaviour changes, and no call site inside this package moved:
the tightening was measured at ZERO compile errors within `plugin-sharing`, so the whole cost
falls on hosts, which is why it is declared here rather than shipped as a patch.

Why: #9754 rules that a sink declaring an optional `error` must declare a NON-optional
`warn`, so a durability report always has somewhere to land — an optional `error` beside an
optional `warn` is a contract that permits silence. These three sinks were red against that
rule from the day it was written and were only invisible to its checker until #11069 taught it
to read bare `Function` as a channel. The maintainer ruled on 2026-08-24 (#10556) that they
tighten rather than stay baselined, shipped `minor` with the break named here.

The bare-`Function` spelling was also its own defect: `Function` is not assignable to a
concrete signature, so `record-orphan-cleanup.ts` could not tighten its own `MinimalLogger`
while these producers stayed loose (#10692). That producer-side blocker is now clear.
142 changes: 142 additions & 0 deletions packages/plugins/plugin-security/src/default-report-sink.test.ts
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,142 @@
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.

/**
* [#10556 (a)] `SecurityPlugin`'s OWN report sink is console-backed by default —
* loud until a host injects one.
*
* ## The shape this pins, and why it is a design call rather than a `?` deletion
*
* The plugin reports its fail-closed refusals through `this.logger`, a private
* field it owns. That field was declared `{ info?; warn?; error? }` and
* INITIALISED `= {}`, so every one of those reports — `getReadFilter … denying
* (fail-closed, #2852)`, `checkAuthoredRowWrite … abstaining`, the ADR-0123
* tenant-wall refusal — went nowhere at all until a host happened to inject a
* sink. `#9754`'s rule (an optional `error` must sit beside a NON-optional
* `warn`) cannot be satisfied by dropping a `?` here: a required `warn` on a
* field initialised `= {}` is a type that lies. The maintainer ruled
* (2026-08-24) that the DEFAULT becomes a console-backed sink, and that
* silent-by-declaration is rejected.
*
* ## Why these three cases and not a field poke
*
* Case 1 drives a REAL report site on a bare instance, so the pin is about what
* an operator sees rather than about a field's contents. `getReadFilter` with an
* on-behalf-of context is the one refusal reachable with no `ql`, no `metadata`
* and no `start()`: it fails closed on the unimplemented D10 delegator
* intersection (#2852) and reports through `this.logger.error?.()`.
*
* Case 2 pins the channel the LEDGER row is about. `error` stays optional by
* #9754's own ruling (hosts inject reduced sinks); `warn` is the channel a
* durability report degrades to, so it is the one that must exist in every
* value of the type — including the default.
*
* Case 3 is the half that keeps case 1 and 2 from being a regression: a host
* that DOES inject a sink must get its own sink, not the console default beside
* it. `start()` binds `ctx.logger` above both of its early bail-outs (#10706),
* so this holds on a degraded boot too.
*
* ⚠️ There is deliberately no `@ts-expect-error` compile-time pin here.
* `packages/plugins/plugin-security/tsconfig.json` EXCLUDES every `*.test.ts`
* file under `src`
* (TEST_DEBT ledger), so a `@ts-expect-error` in this package evaluates never —
* it is not a weak pin, it is no pin. The compile-time half is carried by
* `pnpm check:optional-error-sink`, which runs on every PR with no `paths:`
* filter and turns RED the moment `warn` goes back to optional on this sink.
*/

import { describe, expect, it, vi } from 'vitest';

import { SecurityPlugin } from './security-plugin.js';

describe('[#10556 (a)] SecurityPlugin default report sink', () => {
it('reports a fail-closed refusal to the console when no host sink was injected', async () => {
const plugin = new SecurityPlugin();
const seen: unknown[][] = [];
const spy = vi.spyOn(console, 'error').mockImplementation((...args: unknown[]) => {
seen.push(args);
});

let filter: Record<string, unknown> | undefined;
try {
// No `start()`, no `ctx` — a bare instance, which is exactly the state in
// which the `= {}` default used to swallow this refusal.
filter = await plugin.getReadFilter('sys_task', {
userId: 'agent-1',
onBehalfOf: { userId: 'delegator-1' },
});
} finally {
spy.mockRestore();
}

// The refusal itself is unchanged — this card moves where the REPORT goes,
// never whether the plugin denies.
expect(filter).toBeDefined();

expect(seen).toHaveLength(1);
expect(String(seen[0]?.[0])).toContain('denying (fail-closed, #2852)');
});

it('guarantees a `warn` channel on the default sink, and routes it to the console', () => {
const plugin = new SecurityPlugin();
const sink = (plugin as unknown as { logger: { warn?: (...a: unknown[]) => void } }).logger;

expect(typeof sink.warn).toBe('function');

const seen: unknown[][] = [];
const spy = vi.spyOn(console, 'warn').mockImplementation((...args: unknown[]) => {
seen.push(args);
});
try {
sink.warn?.('[security] a durability report with nowhere else to land');
} finally {
spy.mockRestore();
}

expect(seen).toHaveLength(1);
expect(String(seen[0]?.[0])).toContain('a durability report with nowhere else to land');
});

it('is REPLACED by the host sink `start()` binds, not kept beside it', async () => {
const plugin = new SecurityPlugin();
const hostSeen: unknown[][] = [];
const consoleSeen: unknown[][] = [];
const hostLogger = {
debug: () => {},
info: () => {},
warn: (...args: unknown[]) => { hostSeen.push(args); },
error: () => {},
};
const ctx = {
logger: hostLogger,
// Both `getService` calls in `start()` throw here, so `start()` takes its
// FIRST early bail-out. #10706 hoisted the sink binding ABOVE both bails,
// and this case is what keeps that hoist from silently regressing: on a
// degraded boot the host sink must still be the one bound.
getService: () => { throw new Error('no services in this test kernel'); },
registerService: () => {},
};

await plugin.start(ctx as unknown as Parameters<SecurityPlugin['start']>[0]);

// The bail-out itself reports through `ctx.logger` directly; count from
// here so this case measures only where the FIELD now points.
const beforeCount = hostSeen.length;

const spy = vi.spyOn(console, 'warn').mockImplementation((...args: unknown[]) => {
consoleSeen.push(args);
});
try {
const sink = (plugin as unknown as { logger: { warn?: (...a: unknown[]) => void } }).logger;
sink.warn?.('[security] routed to the host, not to the console');
} finally {
spy.mockRestore();
}

expect(hostSeen).toHaveLength(beforeCount + 1);
expect(String(hostSeen[beforeCount]?.[0])).toContain('routed to the host, not to the console');
// ⚠️ Asserted through a captured array, never `expect(spy).not.toHaveBeenCalled()`:
// `mockRestore()` CLEARS a vitest spy's call history, so that spelling passes
// whether the console was written to or not.
expect(consoleSeen).toHaveLength(0);
});
});
66 changes: 65 additions & 1 deletion packages/plugins/plugin-security/src/security-plugin.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -542,6 +542,65 @@ function requiredCapsForOperation(
return caps.length > 0 ? [...new Set(caps)] : [];
}

/**
* The plugin's OWN report sink — the channel its fail-closed refusals reach an
* operator through, distinct from `ctx.logger`, which exists only once a host
* has driven this plugin's lifecycle.
*
* ## `warn` is NON-optional, and the default below is what stops that being a lie
*
* #9754: a sink declaring an optional `error` must declare a NON-optional
* `warn`, so a durability report always has somewhere to land. This field could
* not satisfy that by dropping a `?` while it was initialised `= {}` — a
* required member on an empty object literal is a type that lies, and every
* report site here is spelled `this.logger.warn?.(…)`, so an unbound sink is
* not a state any caller can notice. It reports nothing, quietly. #10556
* escalated the question rather than answering it to make a checker green, and
* the maintainer ruled (2026-08-24): the DEFAULT becomes a console-backed sink,
* loud until a host injects one; silent-by-declaration is REJECTED.
*
* `error` stays OPTIONAL, deliberately, by the same ruling that created the
* rule (#9754 option C, falsified): hosts do inject reduced sinks, so requiring
* `error` forecloses the legitimate `{ warn }`-only host. What changed is that
* its ABSENCE now has a guaranteed destination.
*/
interface SecurityReportSink {
info?: (...a: any[]) => void;
/**
* The GUARANTEED fallback channel (#9754). A durability report degrades to
* `warn` and no further — `info` is the level AGENTS.md → "Degradation log
* levels" calls the reassuring half-truth — so this is the one member every
* value of this type must carry.
*/
warn: (...a: any[]) => void;
error?: (...a: any[]) => void;
}

/**
* The default {@link SecurityReportSink}: console-backed, so a plugin nobody
* injected a sink into still REPORTS (#10556 limb (a), maintainer ruling
* 2026-08-24).
*
* ## What it carries, and what it deliberately does not
*
* The two REPORT channels only. `info` is left undefined: it carries no
* durability report, so a host-less boot spraying routine chatter at stdout
* would be noise bought with nothing. `info` being optional in the type above
* is the same statement from the other side.
*
* ## Why one shared frozen value
*
* It holds no state and closes over no instance, so a fresh object per plugin
* would differ only in identity; `Object.freeze` records that the plugin does
* not mutate its own default. Nothing here replaces a host sink — `start()`
* ASSIGNS `ctx.logger` over this field, above both of its early bail-outs
* (#10706), so a degraded boot reports through the host too.
*/
const CONSOLE_SECURITY_SINK: SecurityReportSink = Object.freeze({
warn: (...a: any[]) => { console.warn(...a); },
error: (...a: any[]) => { console.error(...a); },
});

export interface SecurityPluginOptions {
/**
* Additional permission sets to register with the metadata service on
Expand DownExpand Up@@ -844,7 +903,12 @@ export class SecurityPlugin implements Plugin {
* why the guard exists and what it is guarding against.
*/
private writeEpoch = 0;
private logger: { info?: (...a: any[]) => void; warn?: (...a: any[]) => void; error?: (...a: any[]) => void } = {};
/**
* This plugin's report sink. Console-backed until a host injects one — see
* {@link SecurityReportSink} and {@link CONSOLE_SECURITY_SINK} for the ruling
* and for why `warn` is the member that is guaranteed (#9754 / #10556 (a)).
*/
private logger: SecurityReportSink = CONSOLE_SECURITY_SINK;

constructor(options: SecurityPluginOptions = {}) {
this.bootstrapPermissionSets =
Expand Down
22 changes: 21 additions & 1 deletion packages/plugins/plugin-sharing/src/share-link-service.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -342,7 +342,27 @@ export interface ShareLinkServiceOptions {
context: ExecutionContext,
) => Promise<boolean>;
/** [#5190] Optional logger for the record-delete cascade / orphan sweep. */
logger?: { info?: Function; warn?: Function; error?: Function; debug?: Function };
logger?: {
info?: (msg: any, ...rest: any[]) => void;
/**
* The GUARANTEED fallback channel (#9754, ruled on #10556). `error` stays
* optional — hosts do inject reduced sinks — so `warn` is where a
* durability report lands when `error` is absent, and a fallback that may
* itself be missing is not a fallback. Call sites keep the
* `logger?.warn?.(…)` spelling as the backstop for hosts the TYPE cannot
* reach; `SweepLogger` in plugin-email's `outbox-sweep.ts` carries the full
* reasoning and the measurement.
*
* ⚠️ This member is REQUIRED, and `ShareLinkServiceOptions` is PUBLICLY EXPORTED
* from this package's `index.ts` — so a host passing `{ info, error }`
* compiled before this landed and does not after. That break is declared,
* shipped `minor`, and named in the changeset (maintainer ruling
* 2026-08-24, #10556 limb (c)).
*/
warn: (msg: any, ...rest: any[]) => void;
error?: (msg: any, ...rest: any[]) => void;
debug?: (msg: any, ...rest: any[]) => void;
};
}

/**
Expand Down
22 changes: 21 additions & 1 deletion packages/plugins/plugin-sharing/src/sharing-rule-service.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -73,7 +73,27 @@ function rowFromRule(row: any): SharingRuleRow {
export interface SharingRuleServiceOptions {
engine: SharingEngine;
sharing: SharingService;
logger?: { info?: Function; warn?: Function; error?: Function; debug?: Function };
logger?: {
info?: (msg: any, ...rest: any[]) => void;
/**
* The GUARANTEED fallback channel (#9754, ruled on #10556). `error` stays
* optional — hosts do inject reduced sinks — so `warn` is where a
* durability report lands when `error` is absent, and a fallback that may
* itself be missing is not a fallback. Call sites keep the
* `logger?.warn?.(…)` spelling as the backstop for hosts the TYPE cannot
* reach; `SweepLogger` in plugin-email's `outbox-sweep.ts` carries the full
* reasoning and the measurement.
*
* ⚠️ This member is REQUIRED, and `SharingRuleServiceOptions` is PUBLICLY EXPORTED
* from this package's `index.ts` — so a host passing `{ info, error }`
* compiled before this landed and does not after. That break is declared,
* shipped `minor`, and named in the changeset (maintainer ruling
* 2026-08-24, #10556 limb (c)).
*/
warn: (msg: any, ...rest: any[]) => void;
error?: (msg: any, ...rest: any[]) => void;
debug?: (msg: any, ...rest: any[]) => void;
};
}

/**
Expand Down
12 changes: 11 additions & 1 deletion packages/plugins/plugin-sharing/src/sharing-service.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -1582,7 +1582,17 @@ describe('[#6428] fail-closed: an unresolvable verdict is DENY, never abstain',
logged = [];
svc = new SharingService({
engine,
logger: { error: (...args: any[]) => { logged.push(args); } },
// [#10556 (c)] `warn` is a REQUIRED member of this options type as of the
// #9754 repair, so an `{ error }`-only spy no longer satisfies it. Both
// channels land in the SAME `logged` array deliberately, and that is
// coverage rather than a formality: #9754's whole rule is that a
// fail-closed report DEGRADES to `warn` when `error` is absent, so a
// double capturing only `error` is one that would not notice the
// degradation this test exists to assert on.
logger: {
error: (...args: any[]) => { logged.push(args); },
warn: (...args: any[]) => { logged.push(args); },
},
});
engine._tables.account = [{ id: 'a1', name: 'Acme', owner_id: 'alice' }];
});
Expand Down
22 changes: 21 additions & 1 deletion packages/plugins/plugin-sharing/src/sharing-service.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -277,7 +277,27 @@ export interface SharingServiceOptions {
*/
tenancy?: () => SharingTenancyProbe | null | undefined;
/** [#5103] Optional logger for the record-delete cascade / orphan sweep. */
logger?: { info?: Function; warn?: Function; error?: Function; debug?: Function };
logger?: {
info?: (msg: any, ...rest: any[]) => void;
/**
* The GUARANTEED fallback channel (#9754, ruled on #10556). `error` stays
* optional — hosts do inject reduced sinks — so `warn` is where a
* durability report lands when `error` is absent, and a fallback that may
* itself be missing is not a fallback. Call sites keep the
* `logger?.warn?.(…)` spelling as the backstop for hosts the TYPE cannot
* reach; `SweepLogger` in plugin-email's `outbox-sweep.ts` carries the full
* reasoning and the measurement.
*
* ⚠️ This member is REQUIRED, and `SharingServiceOptions` is PUBLICLY EXPORTED
* from this package's `index.ts` — so a host passing `{ info, error }`
* compiled before this landed and does not after. That break is declared,
* shipped `minor`, and named in the changeset (maintainer ruling
* 2026-08-24, #10556 limb (c)).
*/
warn: (msg: any, ...rest: any[]) => void;
error?: (msg: any, ...rest: any[]) => void;
debug?: (msg: any, ...rest: any[]) => void;
};
}

/**
Expand Down
14 changes: 14 additions & 0 deletions packages/services/service-settings/src/settings-service.types.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -289,6 +289,20 @@ export type SettingsActionHandler = (input: {
* assignment fails contravariantly. Declaring only what is actually called
* keeps `Logger`, `ctx.logger`, `console.error` and a one-line spy all
* assignable.
*
* ## Why `warn` is OPTIONAL here, on the record (#10556, maintainer 2026-08-24)
*
* #9754 rules that a sink declaring an optional `error` must declare a
* NON-optional `warn`. This sink is the ruled EXCEPTION to that, not an
* oversight and not a repair anyone still owes: the assignability argument
* above is the reason of record, and it is load-bearing in a way the rule
* cannot see. Requiring `warn` costs 10 single-member `{ error }` spies across
* 6 test files their assignability and buys nothing, because this module does
* not depend on the TYPE for its fallback — every report site is written
* `if (this.logger?.error) … else console.error(…)`, so the guaranteed channel
* is in the CODE. The exception is recorded, with that reasoning, in
* `scripts/optional-error-sink-contract.baseline.json`; read that row before
* treating this `?` as debt.
*/
export interface SettingsDiagnosticsLogger {
error?: (message: string) => void;
Expand Down
Loading
Loading