fix(server): audit instance-admin adapter and database-backup mutations to activity_log - #336
Open
claudegoogl-sudo wants to merge 1 commit into
Open
fix(server): audit instance-admin adapter and database-backup mutations to activity_log#336claudegoogl-sudo wants to merge 1 commit into
claudegoogl-sudo wants to merge 1 commit into
Conversation
…ns to activity_log Every mutating route in adapters.ts (install, disable, override, delete, reload, reinstall) and POST /api/instance/database-backups now writes an activity_log row naming the actor, on the success path and — for the code-execution routes (install, reload, reinstall) and the backup attempt — on the failure path with the failure reason. Instance-scoped mutations have no single company, so rows fan out per company via instanceSettingsService(db).listCompanyIds(), matching the existing precedent in instance-settings.ts and plugins.ts. An empty company list now emits a logger.warn instead of silently dropping the row. - new shared helper routes/instance-activity.ts (logInstanceActivity) - adapterRoutes(options) and instanceDatabaseBackupRoutes(service, db) now receive db from app wiring; no module-level singleton - actions: instance.adapter.installed/.disabled/.overridden/.uninstalled/ .reloaded/.reinstalled, instance.database_backup.created - details carry packageName/version/isLocalPath/type (never the raw body); backup rows carry basename(backupFile) + size/prune/duration only - provenance keeps flowing through the central actor-context (AsyncLocalStorage), so board_key writes are flagged as before Verification: vitest adapter-routes.test.ts, adapter-routes-authz.test.ts, instance-database-backups-routes.test.ts (39 passed); server tsc --noEmit clean; negative control — deleting the logActivity call in the helper fails all four new audit tests. Co-Authored-By: Paperclip <noreply@paperclip.ing>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Thinking Path
Linked Issues or Issue Description
Related (found in the duplicate search, different scope): paperclipai#9739 proposes a broader instance-scoped audit stream and privileged lifecycle auditing. This change only adds the missing
activity_logcall sites for the adapter and backup mutation routes, on the existing per-company fan-out pattern.No public issue exists for this change. The underlying problem, in the bug-report shape:
What happened?
The seven mutating instance-admin routes wrote no
activity_logrows:POST /api/adapters/install— runsnpm install, then dynamically imports the package in the server processPATCH /api/adapters/:type— enable or disable an adapterPATCH /api/adapters/:type/override— pause or resume an override of a builtinDELETE /api/adapters/:type— uninstall an external adapterPOST /api/adapters/:type/reload— bust the module cache and re-import adapter codePOST /api/adapters/:type/reinstall— re-runnpm installfor an adapterPOST /api/instance/database-backups— run a manual database backupSteps to reproduce
activity_logtable.Expected behavior
Every mutation on these routes writes at least one audit row naming the actor. Failed install, reload, and reinstall attempts also leave a row with the failure reason. A failed
npm installis still an attempted code execution, and a responder needs it.Paperclip version or commit
masterat14a4d4b59(this branch is cut from it).Deployment mode
Self-hosted. The routes require instance-admin access; the install route is additionally reachable with a board token alone, because
boardMutationGuardexemptssource === "board_key"from the trusted-origin check.Follows
CONTRIBUTING.mdPR template — sections present: Thinking Path (7 steps), Linked Issues (in-PR description, bug-report labels), What Changed, Verification, Risks, Model Used, Checklist (every box ticked or marked N/A with reason).What Changed
server/src/routes/instance-activity.ts.logInstanceActivity(db, req, input)resolves the actor withgetActorInfo, fans onelogActivityrow out per company throughinstanceSettingsService(db).listCompanyIds(), and emits alogger.warnwhen the company list is empty, so the row is no longer dropped silently.server/src/routes/adapters.ts: all six mutating routes write audit rows. Success rows use actionsinstance.adapter.installed,instance.adapter.disabled,instance.adapter.overridden,instance.adapter.uninstalled,instance.adapter.reloaded,instance.adapter.reinstalled, withentityType: "adapter". Install, reload, and reinstall also write a row from their catch branch withoutcome: "failed"and the failure reason. Failure-path writes are guarded with.catchso an audit-write failure cannot mask the original error response.server/src/routes/instance-database-backups.ts: the route factory now takesdb(instanceDatabaseBackupRoutes(service, db)), and the handler writesinstance.database_backup.createdwithentityType: "instance_database_backup"on success and on rejection (the error is re-thrown, so response codes do not change).detailscarry only bounded, relevant fields: package name, requested version,isLocalPath, resolved adapter type; backup rows carry the backup file basename, size, pruned count, and duration. The raw request body is never splatted in.server/src/app.ts: threadsdbinto both route factories.adapterRoutes(options)now requiresdbin its options object, so the audit fan-out always has a handle.AsyncLocalStorage), so board-key writes keep theboard_key_authenticated_writeflag with no extra plumbing at the call sites.Verification
pnpm vitest run src/__tests__/adapter-routes.test.ts src/__tests__/adapter-routes-authz.test.ts src/__tests__/instance-database-backups-routes.test.ts(inserver/) — 39 passed (was 34 before; 5 new audit tests).npx tsc --noEmit(inserver/) — 0 errors.logActivitycall deleted from the helper, all four new audit assertions fail. The tests prove the rows land.isLocalPath: truewith a mocked adapter loader, so no realnpmruns in tests.Risks
listCompanyIdsquery plus N row inserts (N = companies on the instance) to these admin-only, low-frequency routes. IflogActivitythrows on a success path, the mutation succeeded but the request returns 500 — the same behavior the existing instance-settings audit fan-out has. Failure-path writes are guarded so they cannot mask the original error.adapterRoutescallers must now passdb. Both in-repo test call sites are updated. External embedders of the router would see a type error, not a silent gap.companyIdstays required and non-nullable.Model Used
Claude (Opus 4.6, model id
claude-opus-4-7) via the Paperclip agent runtime, extended thinking + tool use, as the "Coder" software-engineer agent.Checklist
Fixes: #/Closes #/Refs #OR (b) described the issue in-PR following the relevant issue template#NNN/github.com/paperclipai/paperclipURLs)docs/...,fix/...) and contains no internal Paperclip ticket id or instance-derived detailsclaudegoogl-sudo/paperclip