Skip to content

feat(dashboards): audit dashboard writes, or revisit the hard-delete decision #63

Description

@xe-nvdk

Context

Found while reviewing #21's implementation, and it reopens a decision rather than just adding a feature.

The justification for hard delete does not exist

src/lib/server/dashboards.ts documents deleteDashboard as:

Not a soft delete: nothing un-deletes or purges a dashboard, so a tombstone would keep up to LIMITS.maxVersionHistory blobs alive forever and make the ON DELETE CASCADE dead code. The deletion is audited instead.

It is not. There is no audit call anywhere under src/routes/api/v1/orgs/[org_id]/dashboards/.

So today a member can permanently destroy a shared dashboard and up to 20 versions of its history, and nothing anywhere records that it happened or who did it. The hard-delete decision was made on the basis of the audit trail existing.

Why logOperatorAction is the wrong tool

src/lib/server/audit.ts writes operator_audit_log with an operator_id and targetType: 'user' | 'org' | 'instance'. Every existing caller is an is_operator action under /admin.

An org member deleting their own dashboard is not an operator action. Writing it there makes operator_id mean two different things in one column, so "did an operator touch tenant data?" gains false positives — and the table has no reader yet, so the corruption would go unnoticed until someone builds one. target_type also has no CHECK constraint, so 'dashboard' would insert cleanly while the TypeScript union claims the set is closed.

Scope

Add an org-scoped audit table:

CREATE TABLE IF NOT EXISTS org_audit_log (
  id INTEGER PRIMARY KEY,
  org_id TEXT NOT NULL REFERENCES organizations(id) ON DELETE CASCADE,
  actor_user_id TEXT NOT NULL,
  action TEXT NOT NULL,
  target_type TEXT NOT NULL,
  target_id TEXT NOT NULL,
  details TEXT,
  created_at TEXT NOT NULL
);
CREATE INDEX IF NOT EXISTS idx_org_audit_org ON org_audit_log(org_id, created_at DESC);

Log dashboard.create | update | delete | restore with {uid, fromVersion, toVersion, panelCount}.

Audit restore, not only delete. Restore is the only write that resurrects content current admins may never have seen, and it is the natural persistence mechanism after a malicious edit is "fixed".

Never log model_json. details is JSON.stringifyd into an unbounded TEXT column; the model is up to 1 MiB and targets[].sql is arbitrary SQL, so logging it multiplies database growth by save count and copies query text, table names and embedded literals into a second, differently-permissioned table. Title is borderline — 200 chars of user-controlled text; if it is logged, any future audit UI must escape it.

Alternative

Revert to soft delete plus a real purge job in the existing 6-hourly task in src/hooks.server.ts, alongside purgeExpiredInstances. That keeps recoverability but needs the purge to be real — note that purgeExpiredInstances itself only sets status = 'purged' and deletes nothing, so "we soft-delete like instances do" would inherit a purge that does not purge.

Pick one. The current state is the worst of both: irreversible, and unrecorded.

Acceptance criteria

  • Deleting, creating, updating and restoring a dashboard each write an org-scoped audit row naming the actor
  • No audit row contains dashboard model content
  • operator_audit_log is unchanged and still means "an operator did something"
  • The claim in dashboards.ts matches the code, whichever way the decision goes

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    dashboardsDashboarding and visualizationenhancementNew feature or request

    Type

    No type

    Projects

    No projects

      Milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions