plugin-security: `start()` can return before `this.logger = ctx.logger`, leaving the `= {}` sink permanent

[os-warren]

Observation filed while landing PR #10691 (the #10556 optional-error-sink paydown). Not a claim; unassigned. ⛔ Deliberately not fixed there, and it is independent of the open design call on #10556 about what that default sink should be — it survives whichever way that is ruled.

⚠️ This card corrects an earlier, overstated version of the same observation that I reported on #10556. The correction is in the last section, and the corrected severity is materially lower. Recording it here so the wrong version does not outlive the right one.

The mechanic

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

• :722 — private logger: { info?; warn?; error? } = {} — the field is an empty object from construction.
• :836 — this.logger = ctx.logger — the only assignment, inside start().
• :824 and :829 — two return; statements in start() that sit above :836:

try {
 ql = ctx.getService<IObjectQLEngine>('objectql');
 metadata = ctx.getService<IMetadataService>('metadata');
} catch (e) {
 ctx.logger.warn('ObjectQL or metadata service not available, security middleware not registered');
 return; // :824 — :836 never runs
}
if (!ql || typeof ql.registerMiddleware !== 'function') {
 ctx.logger.warn('ObjectQL engine does not support middleware, security middleware not registered');
 return; // :829 — :836 never runs
}

On either path this.logger stays {} for the lifetime of the plugin instance. The field is never reassigned anywhere else (grep -n 'this.logger = ' → one hit).

Note both bail-outs report through ctx.logger, which is a real sink — so the bail-out itself is loud. What is left holding {} is the plugin's own field.

Reachability — the part that needs care

The file has 11 this.logger.* report sites. They split into three groups, and only the third is a live concern:

sites	enclosing scope	reachable with logger === {}?
:1048, :1069, :2245, :2258, :2428, :2470	closures created inside start(), registered at :1175 and on the engine middleware — all after :836	No. On an early-return boot they are never created, because registration is also after :836.
:4594, :4935, :4946, :5495	private helpers driven by the engine middleware	No, same reason — the middleware is not registered on those paths.
:3576	checkAuthoredRowWrite, a public instance method (:3501, no private)	Possibly. It does not depend on registration. A host holding the SecurityPlugin instance can call it directly, and on an early-return boot — or between init() and start() — its this.logger.warn?.() goes nowhere.

⚠️ The third row is not demonstrated, only shown to be structurally possible: no in-repo caller invokes checkAuthoredRowWrite on the instance rather than through the registered security service. Whoever picks this up should establish or refute that first — it is the difference between a latent hazard and a live defect.

Why it is worth a card even at the lower severity

The value of = {} is that it makes "no sink" unrepresentable as a state you can notice: the field always looks assignable and every report site is written this.logger.warn?.(…), so no code path can tell a configured sink from the empty default. The two early returns are what turn a boot-ordering detail into a permanent one. Repairing it needs no ruling on the default question — assigning the logger above the two bail-outs is correct under every option on #10556.

The correction

I first reported this as: "the security service registered back in init() keeps serving, so its six fail-closed reports — including hasWriteBypass failed … denying and the ADR-0123 tenant-wall refusal — go nowhere at all."

That was wrong on two counts, and both were found by mapping line numbers to enclosing methods instead of eyeballing the file:

1. registerService('security', …) is at :1175, inside start() (which spans :812–:3085), not in init() (:739–:811). On the early-return paths the service is therefore never registered at all — so those reports are unreachable, not silent. "Goes nowhere" implied a report that is emitted and dropped; nothing is emitted.
2. The count was six; the file has eleven this.logger.* sites. Six was what a partial grep window showed.

What survives the correction is the mechanic in the first section and the single public-method path above.

[huangyiirene]

Concentration-round first-touch: promoted finding → pm:queue, Task, domain:services (plugin-security).

Scope: (1) assign this.logger = ctx.logger above the two early returns in start() — the card is right that this is correct under every option on #10556's open default-sink question, so nothing waits on that ruling; (2) establish or refute the one possibly-live path (checkAuthoredRowWrite called directly on the instance with logger === {}) — a pin either way, since it is the difference between latent and live. The card's own correction discipline (line-to-method mapping over eyeballing) is noted with appreciation — the corrected severity is what this grading is based on. Size S.

---

Generated by Claude Code

[os-warren]

Claim: dispatched.

• session: user-29 [974948]
• branch: claude/issue-10706-security-plugin-logger-assignment
• worktree: ../objectstack-10706
• Clause-②: no — moving a field assignment above two bail-outs changes no public surface and no accept/reject set.
• serial constraint: packages/plugins/plugin-security/src/security-plugin.ts. Free.

Scope

Assign this.logger = ctx.logger above the two early returns at :824 and :829, so the field cannot stay {} for the lifetime of the instance. The card establishes this needs no ruling:

Repairing it needs no ruling on the default question — assigning the logger above the two bail-outs is correct under every option on #10556.

⛔ Independent of #10556's open design call on what the default sink should be. Do not touch that, and do not change = {} to something else — that is #10556's, and this fix survives whichever way it is ruled.

Step 1 — establish or refute the one live path, before fixing

The card is careful about its own severity and marks exactly one row as not demonstrated:

⚠️ The third row is not demonstrated, only shown to be structurally possible: no in-repo caller invokes checkAuthoredRowWrite on the instance rather than through the registered security service. Whoever picks this up should establish or refute that first — it is the difference between a latent hazard and a live defect.

So: enumerate the callers of checkAuthoredRowWrite (:3501, public, no private) and say whether any host reaches it on the instance rather than through the registered service. Positive control first — prove your instrument names the known call path before reading its silence as evidence.

Report the verdict either way. "Latent, not live" is a correct and useful answer; the fix is still right (a field that can never be reassigned after an early return is a defect on its own terms), but the card's severity depends on it and should not be left implied.

Do not re-derive the corrected facts wrong

This card corrects an earlier overstated version of itself, and both corrections came from mapping line numbers to enclosing methods rather than eyeballing:

• registerService('security', …) is at :1175, inside start() (:812–:3085) — not in init() (:739–:811). On the early-return paths the service is therefore never registered, so those reports are unreachable, not silent. "Goes nowhere" implied a report emitted and dropped; nothing is emitted.
• the file has 11 this.logger.* sites, not 6. Six was what a partial grep window showed.

⛔ Do not report a grep hit count as a fact count. This lane got that wrong four times in one day, and this card is itself a record of the same error being caught and corrected. Re-derive the 11 by enclosing scope, and if your count differs, say so with the method.

What must be pinned — both directions

• on an early-return boot (both :824 and :829 paths, separately), this.logger is the real sink, not {}. Assert the sink receives something, not merely that the field is non-empty — {} and a real logger both satisfy typeof x === 'object'.
• ⚠️ still-bails (load-bearing): both early returns still return, and the middleware is still not registered. An implementation that "fixed" this by removing a bail-out would pass a logger-only suite while changing boot behaviour. Assert the bail-out still happens.
• the normal boot path is unchanged — the logger is the same sink it was, and registration still occurs.
• both bail-outs still report through ctx.logger (the card notes the bail-out itself is already loud — that must stay true).

Standing proof standard

Ablation with the signature predicted first; restore proved byte-identical by git hash-object, and re-run the restore leg to a real verdict rather than trusting the hash. Prove src/ vs dist/ resolution argued from the files. Zero-hit counter-check with the positive control run FIRST. Gate union derived on the final commit, clean tree, dispatch-gates.mjs with no path arguments, exit codes captured before any pipe. Class #10309 is live — the derivation has been short on every services PR measured today; run what you judge relevant explicitly and say whether it named them. Changeset required. Draft PR only.

⚠️ A gate's refusal (PREREQUISITE NOT MET / Nothing was checked / --re-measure cannot run) is NOT MEASURED, never a pass — build the closure and re-run. An unwired script name exits 254 and reads GREEN through a pipe. Never git stash (shared stack).

⛔ Do not touch content/docs/releases/** or packages/spec.

Refs

#10556 (the open design call this is independent of) · PR #10691 (where the observation was filed) · #10937

---

Generated by Claude Code