feat(assets): audited secret reveal, no plaintext in lists, tests (PMS-71) #128

Merged

longjacksonle merged 1 commit from feat/pms-71-asset-secret-reveal-and-tests into main

2026-06-07 06:48:03 +02:00

longjacksonle commented

2026-06-07 06:37:50 +02:00

Owner

What

Completes the Assets / CMDB story (PMS-71). All CRUD (asset types, assets, relationships, configuration items, credential vault, audit log) was already wired and returning real data - the "/api/v1/assets returns 501" note was stale. This PR closes the one acceptance criterion that was actually unmet (secrets were leaked in list views) and adds the integration tests AC5 requires.

Closes PMS-71.

The security gap (AC4)

list_credentials and list_configuration_items decrypted and returned every secret in the list response (username / password / notes, and the config value). The credential list also bulk-audited a read for the whole asset. AC4 requires secrets to be "encrypted at rest, never leaked in lists, and reveal is authz-gated and audited" - so list-returns-plaintext is a violation.

Split read from reveal

Lists return secret-free summaries: CredentialSummary (id, name, company, type, url, last_rotated - no username/password/notes) and ConfigurationItemSummary (no value). Neither decrypts anything, so a list can never carry plaintext.
New reveal endpoints: GET /api/v1/credentials/:id and GET /api/v1/configuration-items/:id decrypt a single item, are gated by RequireAssets, and write an asset_audit_log row (credential_read / config_item_revealed). Every decryption is now traceable to a single deliberate action.
Removed the per-list audit on credentials (the list no longer reveals anything).

Tests - `tests/assets.rs`

Asset-type + asset CRUD with company/name filtering, plus an audit-log assertion (mutations write rows).
An asset relationship (hosts).
Credential round-trip: encrypted at rest (raw column != plaintext), the list carries neither the plaintext nor a password field, the reveal decrypts it, and the reveal writes an audited credential_read event.
Configuration-item round-trip: same contract on the value.

Breaking change

list_credentials / list_configuration_items now return summary DTOs without secrets. Callers that need the plaintext must call the new reveal endpoint.

Verification

cargo test --test assets -> 4 passed; cargo test --lib -> 110 passed; cargo test --test rmm still green (it shares the encryption path).
cargo fmt --check clean; cargo clippy --all-targets -- -D warnings clean.

🤖 Generated with Claude Code

## What Completes the Assets / CMDB story (PMS-71). All CRUD (asset types, assets, relationships, configuration items, credential vault, audit log) was already wired and returning real data - the "/api/v1/assets returns 501" note was stale. This PR closes the one acceptance criterion that was actually unmet (secrets were leaked in list views) and adds the integration tests AC5 requires. Closes PMS-71. ## The security gap (AC4) `list_credentials` and `list_configuration_items` decrypted and returned every secret in the list response (`username` / `password` / `notes`, and the config `value`). The credential list also bulk-audited a read for the whole asset. AC4 requires secrets to be "encrypted at rest, **never leaked in lists**, and reveal is authz-gated and audited" - so list-returns-plaintext is a violation. ## Split read from reveal - **Lists return secret-free summaries:** `CredentialSummary` (id, name, company, type, url, last_rotated - no username/password/notes) and `ConfigurationItemSummary` (no value). Neither decrypts anything, so a list can never carry plaintext. - **New reveal endpoints:** `GET /api/v1/credentials/:id` and `GET /api/v1/configuration-items/:id` decrypt a single item, are gated by `RequireAssets`, and write an `asset_audit_log` row (`credential_read` / `config_item_revealed`). Every decryption is now traceable to a single deliberate action. - Removed the per-list audit on credentials (the list no longer reveals anything). ## Tests - `tests/assets.rs` - Asset-type + asset CRUD with company/name filtering, plus an audit-log assertion (mutations write rows). - An asset relationship (`hosts`). - Credential round-trip: encrypted at rest (raw column != plaintext), the list carries neither the plaintext nor a `password` field, the reveal decrypts it, and the reveal writes an audited `credential_read` event. - Configuration-item round-trip: same contract on the `value`. ## Breaking change `list_credentials` / `list_configuration_items` now return summary DTOs without secrets. Callers that need the plaintext must call the new reveal endpoint. ## Verification - `cargo test --test assets` -> 4 passed; `cargo test --lib` -> 110 passed; `cargo test --test rmm` still green (it shares the encryption path). - `cargo fmt --check` clean; `cargo clippy --all-targets -- -D warnings` clean. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

longjacksonle added 1 commit

2026-06-07 06:37:50 +02:00

feat(assets): stop leaking secrets in lists; add audited reveal endpoints + tests (PMS-71)

E2E (staging) / Playwright against staging (pull_request) Successful in 25s

Details

Check / fmt + clippy + compile + tests (pull_request) Successful in 1m0s

Details

Build OCI container / Build and push mokosh-api image (push) Successful in 3m15s

Details

Create release / Create release from merged PR (pull_request) Has been skipped

Details

10b37604f8

The assets / CMDB module already had full CRUD for asset types, assets, relationships, configuration items, the credential vault, and the audit log, all wired and returning real data (the story's "/api/v1/assets returns 501" note was stale). But it did not meet the security acceptance criterion: list_credentials and list_configuration_items decrypted and returned every secret in the list response. The credential list also bulk-audited a "read" for the whole asset on each call. That leaks plaintext in list views, which AC4 explicitly forbids ("encrypted at rest, never leaked in lists, and reveal is authz-gated and audited").

This splits read from reveal. The list endpoints now return secret-free summaries: CredentialSummary (id, name, company, type, url, last_rotated) with no username/password/notes, and ConfigurationItemSummary with no value. Neither decrypts anything, so a list can never carry plaintext. Two new single-item reveal endpoints, GET /api/v1/credentials/:id and GET /api/v1/configuration-items/:id, decrypt one item, are gated by RequireAssets, and write an asset_audit_log row (credential_read / config_item_revealed) so every decryption is traceable. The per-list audit on credentials is removed since the list no longer reveals anything.

Adds tests/assets.rs (the integration coverage AC5 calls for, previously absent): asset-type + asset CRUD with company/name filtering and an audit-log assertion; an asset relationship; and the credential + configuration-item round-trips that pin the security contract - the secret is encrypted at rest (raw column != plaintext), the list response contains neither the plaintext nor a password/value field, the reveal endpoint decrypts it, and the reveal writes an audited read event.

Note: list_credentials / list_configuration_items now return summary DTOs without secrets; callers that need the plaintext must call the reveal endpoint.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>