Repository navigation
✨ Every admin change is logged, Harvest's and Windmill's too (VOTE-AUDIT, SW-F-0364) - #3928
Merged
Merged
Conversation
SW-F-0364 asks for all system activity to be logged. The audit_change() trigger logged only what a person changed through Hasura; most event-level actions run in Harvest and Windmill with their own SQL and wrote no AdminChange. - audit_change() also logs the writes of a transaction whose admin Step names with the setting sequent.audit_actor (audit_log::act_for and Harvest's audit::act_for), with old and new values and its origin. A Hasura session still takes precedence. - The trigger on the tables only Step writes for an admin: keys_ceremony, messaging_account, messaging_event_config, monitoring_config, signing_checks, staff_certificate, tally_results_publication, tally_sheet_import(_item). webhook_key is a secret. - Every state-changing admin route names its admin; Windmill's results publication, monitoring settings and signing approval take it from the caller. - Hand-written entries where the trigger cannot see the change: imports, exports, the approval matrix, report and document passwords, console queries, voter information letters, the creator of a tenant. - A Harvest test fails when a route file commits without logging. - /encrypt-report no longer logs the report's password; /import-candidates checks its permission before writing. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review: 2
Please check the settings in the CodeRabbit UI or the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
|
From the EMS-AUDIT session's dropped tenant-level work (877171f0d5), now carried here: - Deleting a tenant stages an AdminChange in the deleting transaction, in the log of the tenant whose admin deleted it; the route passes the admin as an optional task input. - Limiting access by countries records the country lists before and after, once Cloudflare set them. - /miru/upload-signature no longer records its input, password included, in its tracing span. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ereslibre
marked this pull request as ready for review
October 16, 2026 21:08
- Routes record their after-the-fact entries through audit::record with Harvest's services (record_after removed); act_for's invalid-tenant and failed-transaction paths are tested. - audit::json_error replaces the per-route closures that turned an audit error into a JSON error. - The approval matrix's entry is built by matrix_change, unit-tested. - Windmill: stage_tenant_deletion on the migrated test database. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- audit_changes() treats a nested key set to JSON null as a key left out, as diff_values does: saving a form that writes every key no longer logs "(none) -> (none)" changes. - audit::queue_error and audit::json_queue_error replace the per-route closures for an entry that cannot be queued; import-areas builds its entry inside its transaction. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
14 of 18 tasks
…eta-14259-audit-completion/ovcs
… by tenant id The windmill coverage gate found the new actor paths untested: the monitoring configuration tests now save, reset and switch modes for an admin, and act_for_tenant moves next to act_for with tests for a valid and an invalid tenant id. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ereslibre
added a commit
that referenced
this pull request
Oct 17, 2026
…dmin keys after commit, new tenants' realms (part 2) (#3929) Parent issue: sequentech/meta#13781 Design package [EMS-AUDIT](https://project-1.sequent.sh/#package/EMS-AUDIT), second part. #3902 gave each tenant a signed tenant log; this pull request closes the gaps it listed or left open, apart from the route-by-route entries (see "Split with #3928" below). ## What changes | Area | Change | | --- | --- | | Imports | Importing an election event or a tenant configuration records the file's SHA-256, computed by the task from the file it read, and compares it with the checksum the admin declared (`undeclared`, `matches` or `mismatch`). The entry also counts what the file held: elections, contests, candidates, areas and areas per contest, or configuration sections. A failed commit of a tenant configuration import no longer records the import. | | Entry popup | A new **The imported file** section shows the checksum verdict, the SHA-256, the counts and the document, instead of raw extra details. The deleted election event section names **The log's system key**. Translated into the portal's 8 languages. | | Deleted events in exports | One account's export with "every election event" now also reads the logs of deleted events from their retired databases, merged by time; their rows name `retired event <ID>`. Each retired database is checked against the catalog, opened read-only under the event database lock, and closed to connections again, including when the read fails. The deletion entry keeps the retired log's system public key, since the event's keys are deleted with it. | | Admin public keys | `AdminPublicKey` is posted to the tenant log only once the transaction that saved the key commits. A rollback leaves nothing behind (before, it could leave a public key in the log whose key was then lost). The delivery id is per account and key, so the key is posted once. The tenant log migration backfills keys created before keys went to the tenant log, and running it again does nothing. The outbox can't carry this statement (it only posts `Audit` bodies), so a small post-commit task does it instead. | | New tenants | Creating a tenant prepares its Keycloak realm for the tenant log whatever the realm template: `custom-event-listener`, admin events with details, and the `tenant-logs-read` / `tenant-logs-export` roles granted to the `admin` group. | | Monitoring | Every audit outbox worker pass writes `{"target":"audit_log_outbox","pending_entries":N,"oldest_pending_seconds":S}`, zeros included, so security monitoring can alert on entries waiting more than 5 minutes. This replaces the `waiting`/`oldest_seconds` names. It is documented in the developer page. | | Permissions | `/export-tenant-logs` needs `tenant-logs-read` as well as `tenant-logs-export` (403 otherwise). The portal shows the tenant log's Export only to people who have both. | | Fix | Tenant log exports could not be downloaded: `fetch-document` built a private document's S3 key from the event the caller named. The tenant log asks with an empty event, so the link pointed at `tenant-…/event-/document-…` (NoSuchKey). The key now comes from the document's own event. | | Docs | Settings > Logs reference: the popup's new sections, deleted events in account exports, the export permissions. Developer page: the backlog line and its alert threshold. Auditor tutorial: retired logs, which have no system key in the manifest. | ## Split with #3928 #3928 (VOTE-AUDIT, meta#14259) closes the "routes write no AdminChange" gap from #3902 for every Harvest route, including messaging accounts, certificate authorities, countries and deleting a tenant. It does this by letting the `audit_change()` trigger see Step's own writes. To avoid logging each change twice, this pull request adds no per-route entries. The two pull requests touch different files. ## Acceptance (meta#13781) Ticked where #3902 (merged) and this pull request together meet the criterion. Items 2, 3 and 5 also need #3928. - [x] Signing in to the Admin Portal, failing (wrong password, wrong authenticator code, unknown account), being locked out, setting up or removing an authenticator, resetting or changing a password and signing out each write one KeycloakUserEvent entry to the tenant log, with the client, IP address and authentication used. An unknown username is recorded as unknown. Token refreshes and code exchanges are not logged. *(#3902; this PR adds the listener to new tenants' realms)* - [ ] Every tenant-level change writes one AdminChange entry to the tenant log with the person, the object, the action and each changed field's old and new value, and no secret values. This covers the Settings tabs, the tenant record, election types, trustees, templates, staff accounts, roles and permissions, countries, backup and restore, and creating, importing, archiving, unarchiving and deleting election events. *(Imports now record the file's checksum and counts here. Messaging accounts, countries with old values and deleting a tenant are in #3928.)* - [ ] Event-level create, update, delete, import, configure, approve and schedule operations write AdminChange entries with old and new values to their event's log (baseline). *(Changes made through Hasura are covered since #3902; the Harvest routes are in #3928.)* - [x] A change made through Hasura to any table an admin role can write is logged in the same transaction. The coverage test fails when such a table has no trigger. *(#3902)* - [ ] A change whose transaction rolls back leaves no entry. A committed change always gets exactly one entry, even when immudb or the worker is down for a while: its entries wait and are posted in order afterwards. *(The outbox does this, and this PR moves AdminPublicKey to after commit. Certificate-authority routes still post their own entry after commit, on a best-effort basis; with #3928 they also get an AdminChange through the outbox.)* - [x] A change to the tenant realm made in Keycloak's console is logged as KeycloakAdminEvent, with the operator and the new value. Changes Step makes through its service account appear as SYSTEM entries next to the admin's AdminChange. *(#3902; new tenants' realms have admin events with details since this PR)* - [x] Windmill puts an event without an election event in the tenant log. An event it can't place becomes an ERROR entry and the rest of its batch is still posted. An unreadable message goes to the dead-letter queue, and later messages are processed. *(#3902)* - [x] Deleting an election event writes its tenant log entry, with the event log's last entry and state hash, before anything is removed. The event's log database is unloaded, not deleted. *(#3902; the entry also keeps the retired log's system key since this PR. Checked on the local stack.)* - [x] Settings shows the Logs tab only with tenant-logs-read, and its other tabs only with tenant-write: an Auditor sees only Logs, a Security Officer every tab, a Configuration Manager every tab but Logs, and an SBEI no Settings. Without tenant-logs-read, listElectoralLog and the exports refuse (403), including direct API calls. *(#3902; this PR adds the tenant-logs-read check to the tenant export)* - [x] Settings > Logs has the same header, date range, filters, columns, details popup and export menu as Election Event > Logs, and differs only in its subtitle, timezone, statement kinds and export scopes. *(#3902)* - [x] Times show in the tenant log's timezone (PhST for COMELEC) and in my time. Filters by username, statement kind, event type, log type and created range (inclusive, to the minute, with PhST boundary cases verified) return exactly the matching entries in the tenant log and in event logs. *(#3902)* - [x] Clicking an entry in either list opens a popup (the shared Dialog, not a side drawer) with who, when (the log's zone, my time and UTC), what, the old and new values, the sign-in details or the Keycloak change, the log and the signing keys; Close and Escape dismiss it. *(#3902; this PR adds the imported file and the deleted log's system key)* - [x] Exports reconcile exactly with the list for the same scope and range. They cover the tenant log (whole, or one account with or without its election event entries) and event logs (All Posts, one Post, one country, one account), over a date range, as CSV, SQL or PDF. Each export writes LogsExported with the file's SHA-256. The event export checks logs-export and the tenant export checks tenant-logs-export. *(#3902. This PR adds deleted events' logs to one account's export and fixes the tenant export's download. CSV was checked on the local stack; PDF was not, because it does not render in the dev image.)* - [x] The SQL export loads into PostgreSQL, and every entry's signatures verify with the exported public keys. *(#3902; test `postgres_audit_log_outbox`)* - [x] Every posted entry appears as one JSON line in Windmill's output, without old or new values or secrets. The outbox's waiting time is available to VOTE-SECOPS's alerts. *(#3902; this PR writes the waiting time on every worker pass)* - [x] The one-off task gives every existing tenant its log and key, and AdminPublicKey entries are written to the tenant log. *(#3902; this PR posts keys after commit and backfills keys made earlier)* - [x] The second organization's tenant log works by configuration only (its timezone, accounts and labels); no code refers to COMELEC. *(#3902; nothing in this PR names a client)* - [ ] Apply every linked source requirement, preserving its exact targets, alternatives and lifecycle stage. This package estimate is shared and must not be summed per row. *(A rule for the whole package, not something one pull request can tick)* ## Not in this pull request - EMS-SBEI-QUORUM's signing entries still use their own `signing_log_outbox`. - Tenant log checkpoints and an audit task for the tenant log (VOTE-AUDIT). - Erasing or reopening a retired event database (OPS-RETENTION). - Beyond's client-setup chart still configures only `jboss-logging` for the tenant realm. Existing realms are set up by the one-off task, and new tenants by this pull request. ## Upgrade notes - Monitoring that parsed the outbox line's `waiting`/`oldest_seconds` must read `pending_entries`/`oldest_pending_seconds`. - Roles that exported the tenant log with only `tenant-logs-export` need `tenant-logs-read` too. ## Tests - electoral-log: 124 tests with the database tests (`--include-ignored`), including reading a retired database and closing it again. - windmill: 1705 lib tests pass. `decrypted_archives_use_private_owned_files_and_leave_no_plaintext` needs an `openssl` binary on the host, and passes with one. `postgres_audit_log_outbox` (8, including the post-commit key announcement, the backfill and retired events in exports) and `postgres_audit_change` (18) pass. - harvest: 561 pass, including the new 403 for an export without read permission. - admin-portal: 2233 jest tests, `LogEntryDialog` stories (17, with a new import story), story inventory, `typecheck:stories`, and the journeys `settings/logs`, `settings/access` and `events/operations` (29). - End to end on a local stack built from this branch (Keycloak, Hasura, Harvest, Windmill, PostgreSQL, MinIO, production portals), using the backend E2E bootstrap. All 11 UI journeys pass. The stack also checked, by hand: the import entry's checksum and counts; retired databases read and closed again; the account export's download and its retired rows; a new tenant's realm settings and roles; the backlog line. That run found the export download bug fixed here. Not measured locally: the per-package coverage gates. ## Demo Video with voiceover, to upload to meta#13781: import, then an event edit, deletion, the account export with the retired log, a new tenant's realm, and the backlog line. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
A dependency's generic code compiled into an instrumented binary names files in the cargo registry. package_directories took such a crate for a measured package, and relative_to(ROOT) then failed the run after every journey had passed (tracing-0.1.44 in run 38982215384). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…eta-14259-audit-completion/ovcs # Conflicts: # docs/docusaurus/docs/07-developers/13-electoral-log/04-electoral-log-tenant-log.md
…nd custody for a tally The harvest coverage gate found the regions this branch added to the voting status routes and the server custody route untested. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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
1 participant
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.
Parent issue: https://github.com/sequentech/meta/issues/14259
Requirement SW-F-0364: log all system activity from start to finish. #3900 and #3902 log what a person changes through Hasura, accounts and roles, Keycloak events and the tenant-level actions. #3902's "Not in this pull request" left the rest open: most event-level routes (areas, elections, dates, voting status, ceremonies, tallies, publications, scheduled events, reports, applications, the approval matrix, monitoring, signing settings) write with Harvest's or Windmill's own SQL, which
audit_change()skipped, so they wrote noAdminChange. This pull request closes that gap.What changes
1791800000000_audit_changes_step_makes:audit_change()also logs the writes of a transaction whose admin Step names in the settingsequent.audit_actor({user_id, username, role, origin, tenant_id}), with old and new values; a Hasura session still takes precedence. The trigger on the tables only Step writes for an admin:keys_ceremony,messaging_account,messaging_event_config,monitoring_config,signing_checks,staff_certificate,tally_results_publication,tally_sheet_import,tally_sheet_import_item.webhook_keyis a secret (SQL and Rust rules)audit_log::act_for(&tx, &actor, origin, tenant_id). Results publication (publish, revoke, website policy), monitoring settings (save_as,reset_to_preset_as,set_mode_as) and a signing approval (as the signer whose signature runs the action) name their admin.insert_tenanttakes the creating adminaudit::act_for(&tx, &claims)right after the transaction opens in every state-changing admin route: areas, elections, election dates and schedule recompute, keys and tally ceremonies, trustee custody, ballot publication, voting status, tally sheets and their imports, signing certificates, issuers, checks and rules, messaging accounts and settings, applications, scheduled events, lifecycle policies, uploads and previews, certificate authority import and deleteAdminChangestaged in the deleting transaction, in the log of the tenant whose admin deleted it (the route passes the admin as an optional task input)harvest/tests/support/audited_routes.rsfails when a route file commits a transaction without naming its admin or staging an entry. Exemptions, with their reasons, intests/fixtures/unaudited-committing-routes.txt(voting portal routes, reads that commit, callbacks)Also fixed on the way:
/encrypt-reportlogged the report's password (info!("body …")and the tracing span)./miru/upload-signaturerecorded its input, password included, in its tracing span./import-candidateswrote its task row before checking the permission; it now checks first (itsroute_permissionscase is updated)./delete-election-eventpasses the admin's role like every other route.Not in this pull request
/fetch-documentdownloads write no entry.Tests
postgres_audit_change(24 tests; 6 new: an admin named by Harvest or Windmill, the setting ending with the transaction, Hasura's precedence, the new tables, a keys ceremony,webhook_key).act_foron the migrated test database, import and export entries, and the route permission inventory.reuse lintpasses.Demo
A narrated video of the feature on a local stack is attached to meta#14259.
🤖 Generated with Claude Code