Skip to content

fix(plugin-audit): drop record_views' always-empty ip_address column, replace with actor - #9956

Merged
os-warren merged 1 commit into
mainfrom
claude/issue-9539-record-views-ip-column
Aug 19, 2026
Merged

fix(plugin-audit): drop record_views' always-empty ip_address column, replace with actor#9956
os-warren merged 1 commit into
mainfrom
claude/issue-9539-record-views-ip-column

Conversation

@os-warren

Copy link
Copy Markdown
Collaborator

Fixes#9539

What was wrong

sys_audit_log's record_views list view (the "who viewed this record" screen shipped by
#8992) declared an ip_address column, but buildRow in read-audit.ts never stamps that
key on a read row — client-fingerprint fields are populated on auth events only. The
column was structurally empty on every row this view can ever show. On a compliance
screen that reads as "we captured the fingerprint and this request had none" rather than
"not captured" — a stronger and wrong claim, and the same 审计面宁窄勿谎
(narrow-not-untruthful) defect class #7675 / #8147 / #8315 retired from this object's
action enum, one layer down on a column instead of an enum value.

Fix (Option 1, per both rulings)

  • Dropped ip_address from record_views's column list in sys-audit-log.object.ts.
  • Replaced it with actor, which the read writer DOES stamp on every row (conditionally
    present, always populated when the field is declared) and which is the one column that
    attributes a service principal (svc:<name>) rather than falling back to a null
    user_id — endorsed by both rulings as a good same-line improvement, dev's call taken.
  • Option 2 (stamping viewer IP) is explicitly not commissioned here.

Maintainer ruling 2026-08-18 + triage auto-adjudication 2026-08-19 — both Option 1.

Pin

sys-audit-log-record-views-columns.test.ts derives the read writer's actually-stamped
key set at runtime, from a real ObjectQL engine run of installReadAuditWriter
(never a hand-copied list that can drift from buildRow), then asserts every
record_views column is a member of that set. A principal carrying both userId and a
service actor label, on a record carrying organization_id, makes every conditionally-
stamped key present at once so the derived set is the writer's full vocabulary, not just
today's default path through it.

Ablation (standing lane clause): put ip_address back into the column list, confirmed
the pin goes RED with the exact predicted signature (record_views declares column 'ip_address', but the read writer's buildRow() ... never sets that key), then restored
the file byte-identically (sha256 hash-verified pre/post).

Docs coupling

Deleted the one README bullet (from #9517 / PR #9541) that documented the shipped
record_views view as carrying an always-empty ip_address column — it no longer
applies. The ip_address/user_agent-populated-on-auth-events-only note itself is
unchanged (still true). The separate docs page from PR #9860 ("No IP address and no user
agent") was not touched — it becomes correct by construction, per the
auto-adjudication.

Tests run

  • pnpm --filter @objectstack/plugin-audit test — 285/285 passed (18 test files,
    including the new pin, 7 assertions).
  • pnpm --filter @objectstack/plugin-audit typecheck — clean.
  • node scripts/pm/dispatch-gates.mjs (re-derived against the actual diff) — every named
    local gate + convention-triggered gate green: check:cross-package-test-inputs,
    check:slot-lookup, check:test-source-alias, check:type-source-resolution,
    check:changeset-gate-self-tests, check:objectui-changeset,
    check-adr-0087-registration, check-changeset-no-major, check-empty-changeset,
    scripts/docs-audit/check-affected-docs.mjs, check:i18n, check:query-options-erasure,
    check:engine-double-contract, check:where-matcher,
    check:type-check-coverage (structural + --re-measure ratchet, full workspace closure
    built first — 33 ledger entries re-measured, none above their recorded number).
  • node scripts/check-nul-bytes.mjs — clean.

Gate + test run at 633841efd (this PR's HEAD).

Changeset

@objectstack/plugin-audit patch — published package, user-visible list-view column
change.


Generated by Claude Code

… replace with actor
sys_audit_log's record_views list view declared an ip_address column that no
read-path writer ever stamps: buildRow in read-audit.ts stamps action,
created_at, user_id, object_name, record_id, old_value, new_value, tenant_id,
and conditionally organization_id/actor -- never ip_address, since client-
fingerprint fields are populated on auth events only. On a compliance
screen an always-empty column reads as "captured, and none" rather than
"not captured" -- the same narrow-not-untruthful defect class #7675/#8147/
#8315 retired from this object's action enum, one layer down on a column.
Replaced with actor, which the read writer DOES stamp on every row and which
attributes a service principal that user_id structurally cannot hold.
Pinned by sys-audit-log-record-views-columns.test.ts: the stamped key set is
derived at runtime from a real engine run of the writer, never hand-copied,
so the class can't regrow silently. Ablated (put ip_address back, confirmed
red, restored byte-identically) per the standing lane clause.
Deleted the one README bullet (from #9517/PR #9541) that documented the
column as always-empty, since it no longer applies.
Maintainer ruling 2026-08-18 + triage auto-adjudication 2026-08-19 (both
Option 1). Stamping viewer IP (Option 2) is explicitly NOT commissioned.
Fixes#9539
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PnJHU45vPJj5UQrxe946Bx
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/plugin-audit, touching 5 documentable anchor(s).

19 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: node scripts/docs-audit/affected-docs.mjs --json 420804d1099bfac80e8572fbd97157c849f75ae3.

1 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails.

What this run could not see
  • 1 changed file(s) yielded no anchor (packages/plugins/plugin-audit/README.md) — pages documenting those are invisible to this run
  • 1 anchor(s) matched too much of the corpus to be a work list: created_at (literal, 33 pages)
  • the SDK route bridge reached 45 of 221 client-bound route-ledger rows — the other 176 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run: node scripts/docs-audit/affected-docs.mjs --bridge-coverage

Coarse fallback — 6 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 420804d1099bfac80e8572fbd97157c849f75ae3packageMentionDocs.

Which tree this was computed on

This run read content/docs from 98a5f770b0b850b931f1642e0b089f07f7f6d89d — the merge of head 633841efddb7a8438ec4de7f6ee0b63dc0504a41 into base 420804d1099bfac80e8572fbd97157c849f75ae3, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 98a5f770b0b850b931f1642e0b089f07f7f6d89d && git checkout 98a5f770b0b850b931f1642e0b089f07f7f6d89d
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 420804d1099bfac80e8572fbd97157c849f75ae3 633841efddb7a8438ec4de7f6ee0b63dc0504a41 && git checkout -B drift-repro 420804d1099bfac80e8572fbd97157c849f75ae3 && git merge --no-ff 633841efddb7a8438ec4de7f6ee0b63dc0504a41
node scripts/docs-audit/affected-docs.mjs --json 420804d1099bfac80e8572fbd97157c849f75ae3

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 420804d1099bfac80e8572fbd97157c849f75ae3 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

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

Labels

documentationImprovements or additions to documentationsize/mteststooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

record_views lists an ip_address column that no read-path writer ever stamps — a declared-but-unwritten column on a shipped compliance view

2 participants

@os-warren@claude