Skip to content

finding: record-view auditing has no configuration path on the os serve boot path — the CLI registers AuditPlugin with no options #9863

Description

@os-steve

Observation-class finding, measured while writing the record-view auditing docs page (#9540 / PR #9860). Not a defect claim about correctness — the capability works exactly as designed when it is constructed with options. The gap is reachability from the shipped CLI boot path.

Measurement

Record-view auditing is enabled only by AuditPlugin's constructor:

new AuditPlugin({ readAudit: { objects: ['contact', 'account'] } });

packages/cli/src/commands/serve.ts:2438-2440 auto-registers the plugin like this:

const { AuditPlugin } = await import(auditPkg);
await kernel.use(new AuditPlugin());

No options, and no config-derived helper. Compare the sibling six lines above, where SecurityPlugin gets one:

await kernel.use(new SecurityPlugin(appSecurityPluginOptions(config)));

appSecurityPluginOptions exists precisely so the CLI boot and @objectstack/verify's bootStack cannot disagree (#7001). There is no appAuditPluginOptions, and grep -rn "readAudit" packages/ examples/ apps/ finds zero call sites outside plugin-audit itself — nothing in the repo ever supplies an audited object list.

⇒ A deployment served with os serve has record-view auditing off and no declared way to turn it on. The capability is reachable only from code that composes the kernel itself.

The half-path, and why it should not just be documented

An app can put new AuditPlugin({ readAudit: { … } }) in its objectstack.config.ts plugins array. That array is consumed at serve.ts:2455, i.e. after the auto-registration, and ObjectKernel.use() (packages/core/src/kernel.ts:194) stores plugins in a name-keyed map:

this.plugins.set(pluginMeta.name, pluginMeta);

Both instances share the name com.objectstack.audit, so the config-declared one overwrites the auto-registered one and the opt-in does take effect. But that is an undeclared last-wins overwrite, not a contract: nothing documents it, no test pins it, and LiteKernel.use() throws on the same input (filed separately). Documenting it would be teaching an accident, so PR #9860's page deliberately does not.

Dispositions

Not ranked — this needs a decision, not a guess.

  • Add appAuditPluginOptions(config) mirroring the security helper, reading the audited object list off the stack config. Most consistent with the existing pattern, and gives os serve and bootStack one shape.
  • Leave it composition-only and say so in the plugin README and the docs page. Defensible if the intended consumer really is @objectstack/security-enterprise composing on top — read-audit.ts's header says the policy "belongs to the caller, which in the enterprise packaging is @objectstack/security-enterprise". But the open edition then has a shipped compliance capability its own CLI cannot reach.
  • Make the auto-registration skip when the config declares its own audit plugin, using the hasPluginMatching helper already at serve.ts:2739 for exactly this purpose on other plugins. Turns the accidental overwrite into a declared one.

Refs: #9540 / PR #9860 (the docs card that measured it) · #8992 / PR #9515 (the capability) · #7001 (appSecurityPluginOptions).

Activity

  1. os-zhuang commented on Aug 20, 2026

    @os-zhuang
    Contributor

    Claim — domain:devx execution seat (#6023), PM session session_01DdCnBGcHeufjrq7drTD3wt. Branch: claude/issue-9863-audit-plugin-boot-options. pm:queue → pm:dispatched.

    fold-or-serial: solo. packages/cli/src/commands/serve.ts is touched by none of this seat's in-flight work (#9758 → comment-mask/ESLint, #9710 → ci.yml, plus #10396/#10410/#10412 in the queue on unrelated paths).

    ⚠️ Premise verified — and the code already contains the deliberation

    Confirmed on origin/main: new SecurityPlugin(appSecurityPluginOptions(config)) at :2499, new AuditPlugin() bare at :2535, and appAuditPluginOptions exists nowhere as a function.

    But the card understates the situation, and a dev reading only the card would land the wrong PR. serve.ts:2505-2531 already argues this out by issue number:

    [#9863 / #9864] Registered with NO options … The one way an app turns it on today is to put its own new AuditPlugin({ readAudit: … }) in the stack's plugins array, which this file registers further down — AFTER this line. Both instances carry the name com.objectstack.audit, so the app's supersedes this one and the opt-in takes effect.

    That is a DECLARED contract now, not the accident #9863 found it as: duplicate registration by name overwrites — last-one-wins, with a warn naming both versions — identically on both kernels … pinned against ObjectKernel and LiteKernel by plugin-registration.contract.test.ts (#9864, maintainer ruling 2026-08-19, option B).

    ⇒ This is not "add the missing helper." A working, contractual, pinned path already exists. The card's actual question is the one the comment itself names:

    whether os serve should grow an appAuditPluginOptions(config) helper like its SecurityPlugin sibling above, rather than reaching the capability only through a supersede.

    ⚠️ Two constraints the dev must not trip

    1. The order dependency is load-bearing and stated: "this registration must stay ABOVE the stack's plugins loop, or the CLI's option-less instance would supersede the app's configured one instead."
    2. Adding a configured CLI instance changes the supersede's semantics. Today it is unconfigured loses to configured. With a helper, both instances carry options — and which wins, and whether that is still the behaviour core(finding): duplicate plugin registration throws on LiteKernel and silently overwrites on ObjectKernel — a fourth instance of the two-kernel semantic split #9864's ruling pinned, is the real design question, not a detail.

    ⛔ There is a maintainer ruling adjacent to this (#9864, option B). Engage it; do not route around it.

    Model: opus. This is the second card today whose subject was already deliberated in the source it lives in — the first was #9873, where the gate's own header stated the card's thesis and the fix had to be argued against the author's reasoning rather than over it.


    Generated by Claude Code

  2. os-zhuang commented on Aug 20, 2026

    @os-zhuang
    Contributor

    os-dev-report

    {
      "issue": 9863,
      "status": "done",
      "branch": "claude/issue-9863-audit-plugin-boot-options",
      "pr": "https://github.com/objectstack-ai/objectstack/pull/10450",
      "premise_still_valid": true,
      "summary": "Ruled NO on the card's open question: `os serve` should not grow `appAuditPluginOptions(config)`. Premise re-verified on origin/main @ e502a6a8e — `new AuditPlugin()` is still bare at serve.ts:2535 and `appAuditPluginOptions` exists nowhere. But the card's harm is ERGONOMIC, not functional: since #9864 the `plugins`-array supersede is a declared, pinned path to the capability, so `os serve` CAN reach it. The four axes all point away from a helper: (1) #7001's reason does not transfer — `bootStack` constructs no AuditPlugin and `@objectstack/verify` does not depend on `@objectstack/plugin-audit`, so there is only one boot path with an opinion and no disagreement to close; (2) the helper would have nothing to read — `appSecurityPluginOptions` derives from the already-declared `config.permissions`, while there is no `audit` key in the stack schema, so this one would MINT an authorable surface; (3) that surface is the shape #8992's 2026-08-16 ruling already refused for the object-metadata spelling — a declaration surviving in a deployment that never installs the plugin, reading as coverage while recording nothing — and worse here, because the registration is AUTH-GATED (inside `if (!hasAuthPlugin && tierEnabled('auth'))`, past the host-kernel and secret guards), a fact neither the card nor serve.ts's comment had written down; (4) it would be a second config surface that silently loses to the app's own `plugins` entry by the very contract above. Measured pull: zero `readAudit` call sites repo-wide. What landed instead: the docs page's false clause (\"no knob to turn it on\") corrected, the `plugins`-array opt-in documented in the page and the published README, the ruling recorded at the registration site, and a contract test pinning the three facts the ruling rests on — including the load-bearing ordering that until now was asserted by a comment and nothing else. #9864's kernel contract is untouched.",
      "tests": "All at final head 57e4e5e5f, re-derived and re-run after every commit. Gate union: `node scripts/pm/dispatch-gates.mjs` with no args (7 paths; 21 path-matched families + 5 convention-triggered), re-derived after the roster/turbo.json edits added two paths — identical family set. All 21 green plus check:type-check-coverage, check:type-check-debt, check:engine-double-contract, check:where-matcher, check:query-options-erasure, check:i18n, check:nul-bytes, check:published-readme-exports. Verdicts quoted from each gate's own output, exit codes captured BEFORE any pipe (`cmd > log 2>&1; EXIT=$?`), never from a `| tail`. check:type-check-debt: '--re-measure: OK — 33 ledger entr(ies) re-measured in 251.4s, 1924 raw tsc error(s) total, none above its recorded number.' check:cross-package-test-inputs: 'OK: 12 package(s) read outside themselves, all declared, and turbo.json hashes every declared glob.' Suites: `pnpm --filter @objectstack/cli test` → 'Test Files 139 passed (139) · Tests 1529 passed (1529)' (139 = every *.test.ts on disk, so the new file is in the run); `pnpm --filter @objectstack/plugin-audit test` → 'Test Files 18 passed (18) · Tests 300 passed (300)'. Typecheck: `pnpm --filter @objectstack/cli --filter @objectstack/plugin-audit typecheck` — both echoed `tsc --noEmit` then 'typecheck: Done', script names verified echoed so neither was a zero-match silent pass; that clean run is also the proof the `@ts-expect-error` on the untyped .mjs import is real rather than phantom, since an unused directive is itself a tsc error. Everything that builds/typechecks/reads dist ran inside `scripts/pm/os-verify-lock.sh -c` (workspace build 72s, later batches 5-6min each, all VERDICT command-exit 0); the pure source-text scanners ran outside it — declared. ABLATIONS (3, all red in the predicted direction): NO REBUILD LEG APPLIES and here is why rather than a template answer — this scan reads `serve.ts` and `harness.ts` from disk with readFileSync at test time and imports nothing from any package's dist (only vitest, node builtins, and scripts/js-comment-mask.mjs), so there is no `exports`→dist resolution for a stale build to falsify in either direction. Each mutation was confirmed ON DISK by grep counts of both the removed and injected literals, never by an editor's exit code. (a) ORDER: moved the audit try-block below the stack `plugins` loop — confirmed `new AuditPlugin()` at line 2827 with the loop at 2543 (was 2535 vs 2551), count still 1 → 1 failed / 3 passed, `expected 68113 to be less than 60460`. (b) RULING: `new AuditPlugin()` → `new AuditPlugin(appAuditPluginOptions(config))` — confirmed removed literal count 0, injected `appAuditPluginOptions` count 2 (1 code + 1 pre-existing comment) → 2 failed / 2 passed, `expected [ 'appAuditPluginOptions(config)' ] to deeply equal [ '' ]`; the comment-only second hit not tripping the assertion is a built-in positive control for the masker. (c) BOOTSTACK: injected `kernel.use(new AuditPlugin())` into harness.ts — confirmed count 1 at line 544 → 1 failed / 3 passed, `expected [ '' ] to deeply equal []`. Both subjects restored BYTE-IDENTICALLY: sha256 re-matched the recorded baseline (serve.ts d2053cfd…, harness.ts ad7e3623…) and `git status` came back empty. ZERO-HIT WITH POSITIVE CONTROL: `grep -rn readAudit packages/ examples/ apps/` → no hit outside packages/plugins/plugin-audit/ except two prose mentions in serve.ts's own comment; same paths, same command shape, `AuditPlugin` → 20+ files including seven dogfood suites, so the zero means something. Clause ② NOT reachable: no `packages/spec/src/**` path in the diff.",
      "open_questions": [
        {
          "question": "The card's third disposition is left unimplemented and is a live, separable choice: should the CLI SKIP its option-less auto-registration when the config already declares an audit plugin (via the `Serve.providesCapability` helper it already uses for other plugins), instead of registering and being superseded? The user-visible difference is that today an app which opts in — and does not supply its own AuthPlugin — gets a `Plugin superseded: 'com.objectstack.audit'` warn on EVERY boot, carrying advice ('register the plugin once if that is not what you meant') it cannot act on, because the duplicate is the CLI's and not theirs.",
          "options": [
            "A — leave as landed: the warn stays, and the docs now name it as the opt-in working rather than a misconfiguration. Zero runtime risk; #9864 deliberately made the supersede audible and named this exact pair as its motivating example.",
            "B — skip the auto-registration when `providesCapability(plugins, ['com.objectstack.audit','AuditPlugin'])` matches. Removes the warn and the order dependency for the configured case, mints no config surface, and matches the rule the spec ALREADY declares for `requires:` ('If a capability is also provided explicitly via plugins[], the explicit instance wins and the resolver does not double-register'). Costs a runtime behaviour change on the boot path plus a changeset for packages/cli.",
            "C — file it and decide later; nothing is broken either way."
          ],
          "recommendation": "A for this PR (which is what landed), then C — file B as its own card rather than folding it in. The failure modes are asymmetric and that decides it: today's worst case is a redundant registration plus a noisy line, whereas a skip that misfires means NO AuditPlugin at all — sys_audit_log absent, the whole compliance ledger silently gone. Trading a benign failure for a severe one is a change worth its own review, not a rider on a documentation ruling. Note B is NOT blocked by #9864: it leaves the kernel contract and its two-kernel pin untouched and only stops the CLI from creating the duplicate in the first place."
        }
      ],
      "out_of_scope_findings": [
        "filed as #10451: check:slot-lookup and check:query-options-erasure fail non-deterministically (4/11 and 1/5 measured, same worktree and commit) with 'Parsing error: Maximum call stack size exceeded' on packages/spec/src/migrations/registry.ts — NOT memory pressure (11.5 GB free, load 1.5 at failure; reproduces with and without --max-old-space-size). Both are in lint.yml and in many cards' derived unions, and the steps abort the rest of the ESLint job.",
        "filed as #10452: check-cross-package-test-inputs' literal collector cannot see an escaping relative IMPORT specifier, so a test importing a module outside its package is undeclared SILENTLY — measured: the gate reported OK over this PR's new `import { maskComments } from '../../../../scripts/js-comment-mask.mjs'` with no roster entry. Declared by hand here; the class stays open.",
        "filed as #10453: #9367's naive comment strip survives in serve-verify-security-parity.contract.test.ts and serve-email-config-parity.contract.test.ts — PR #9445 converted the six scripts/check-*.mjs gates, not the two scans living beside serve.ts. Not live today (their measured constructions sit outside the swallowed span) but contagious: copying it is how this PR met it, and it silently deletes 203 code-bearing lines of serve.ts."
      ]
    }

    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions