Skip to content

Add credential version history and rollback for the built-in store - #333

Open
wankhede04 wants to merge 2 commits into
Infisical:mainfrom
wankhede04:feat/credential-history-rollback
Open

Add credential version history and rollback for the built-in store#333
wankhede04 wants to merge 2 commits into
Infisical:mainfrom
wankhede04:feat/credential-history-rollback

Conversation

@wankhede04

Copy link
Copy Markdown

Closes #330.

Problem

Per the credentials doc: "If STRIPE_KEY already exists, it is overwritten." There's no history and no rollback for the built-in credential store — if an operator (or an agent proposing a value back into a credential slot) overwrites a key with a bad value, the previous value is just gone.

This is scoped to the built-in credential store only — vaults backed by the Infisical credential store already have their own versioning upstream and are unaffected.

What this adds

  • credential_versions table (SQLite + Postgres, internal/store/20260723161000_credential_versions.go): archives the ciphertext/nonce a credential held before each overwrite, plus a timestamp and the actor (user/agent/session fallback) responsible. Rows are pruned per (vault, key) down to AGENT_VAULT_CREDENTIAL_HISTORY_MAX_VERSIONS (default 10, mirroring the existing AGENT_VAULT_LOGS_MAX_* env-driven retention pattern for request logs; 0 disables pruning).
  • SetCredential and ApplyProposal now archive the value they replace before overwriting it, in the same transaction — this covers both a direct credential set and an agent's proposed value being approved.
  • API: GET /v1/credentials/history and POST /v1/credentials/rollback, both gated at member+ role (never proxy-role agents) — the same rule credential get --reveal already uses. Rollback has no bespoke store method: it just calls SetCredential with the archived ciphertext, so whatever was live gets archived as a new version first — a rollback is itself just another recorded version, never destructive.
  • CLI: agent-vault vault credential history <key> [--reveal] and agent-vault vault credential rollback <key> --version N.
  • Docs: docs/self-hosting/environment-variables.mdx, docs/reference/cli.mdx, docs/learn/credentials.mdx, README.md.

Deliberately out of scope (per the issue's own suggested phasing): versioning on credential delete (soft-delete). Happy to follow up separately if useful.

Testing

  • New store-level tests (internal/store/sqlite_test.go): archiving on overwrite, ordering, pruning (including the 0 = unbounded case), proposal-apply archiving with actor attribution, and a rollback round-trip that confirms the "rolled-back-from" value survives.
  • New handler tests (internal/server/server_test.go): member+ gating on both endpoints (proxy role blocked), history listing/ordering, --reveal decryption, rollback restoring the target version and archiving the current one, and 404 on an unknown version.
  • New CLI registration test (cmd/cmd_test.go).
  • go build ./..., go vet ./..., and go test ./... all pass (one pre-existing, unrelated failure in internal/isolation — a local Docker-socket-path test, fails the same way on main).

If a credential is overwritten with a bad value today, the previous value
is just gone — there's no history and no way back short of remembering
it yourself. SetCredential and proposal apply now archive the value they
replace (ciphertext, timestamp, and the actor responsible) into a new
credential_versions table before writing the new one, so overwrites are
always recoverable.

- New credential_versions table (SQLite + Postgres), pruned per (vault,
  key) to AGENT_VAULT_CREDENTIAL_HISTORY_MAX_VERSIONS (default 10; 0 =
  unbounded) — mirroring request_logs' env-driven retention knobs.
- GET /v1/credentials/history and POST /v1/credentials/rollback,
  gated at member+ (never proxy-role agents), same rule as credential
  get/--reveal. Rollback is implemented as a normal SetCredential call
  with the archived value, so it's itself just another recorded version
  — never destructive.
- CLI: `vault credential history <key> [--reveal]` and
  `vault credential rollback <key> --version N`.
- Docs: environment-variables.mdx, cli.mdx, credentials.mdx, README.

Fixes Infisical#330
@greptile-apps

greptile-apps Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds history and rollback for credentials in the built-in store. The main changes are:

  • A credential-version table and configurable per-key retention.
  • Transactional archiving for direct writes and approved proposals.
  • Member-only history and rollback API endpoints.
  • New CLI commands, documentation, and test coverage.

Confidence Score: 4/5

Concurrent PostgreSQL credential writes can fail during history allocation and should be fixed before merging.

  • Per-key versions are allocated with an unlocked MAX(version) + 1 query.
  • Direct sets and rollbacks are not serialized by the vault lock.
  • The authorization and vault-scoping checks for the new endpoints are consistent with existing member-only operations.

internal/store/sql_store.go

Important Files Changed

Filename Overview
internal/store/sql_store.go Adds transactional credential archiving, retention, history queries, rollback support, and proposal integration; concurrent PostgreSQL version allocation can collide.
internal/store/20260723161000_credential_versions.go Adds SQLite and PostgreSQL schemas for encrypted credential history.
internal/store/credential_history.go Adds environment-driven history retention with ten versions by default and zero for unbounded retention.
internal/server/handle_credentials.go Adds member-gated history and rollback handlers with optional decryption and actor attribution.
internal/server/handle_proposals.go Passes the approving actor into proposal application for version attribution.
cmd/credential.go Adds credential history and rollback CLI commands.

Reviews (1): Last reviewed commit: "Add credential version history and rollb..." | Re-trigger Greptile

Comment on lines +752 to +754
nowStr := s.dialect.FormatTime(time.Now().UTC())
_, err = tx.ExecContext(ctx,
s.dialect.Rebind(`INSERT INTO credential_versions (id, vault_id, key, version, ciphertext, nonce, actor_type, actor_id, created_at)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Concurrent Version Allocation Collides

On PostgreSQL, two credential sets or rollbacks for the same vault and key can both compute the same MAX(version) + 1. The second insert then violates UNIQUE(vault_id, key, version), rolls back the transaction, and returns an internal error for an otherwise valid write.

On Postgres, two concurrent overwrites of the same (vault, key) — e.g. a
manual credential set racing a proposal apply — could both read the same
MAX(version) and try to insert the same next version number, so the
second commit would fail on the UNIQUE(vault_id, key, version)
constraint instead of the write succeeding as expected.

Add the dialect's FOR UPDATE clause (already used for the analogous
proposal-ID race in CreateProposal) to the SELECT that snapshots the
current value, serializing concurrent overwrites of the same credential.
No-op on SQLite, which already serializes writers itself.
@wankhede04

Copy link
Copy Markdown
Author

Addressed the Greptile comment on the concurrent version allocation race (`internal/store/sql_store.go`):

  • `archiveCredentialVersionTx`'s initial `SELECT` (which reads the current value before archiving it) now appends the dialect's `FOR UPDATE` clause, the same mechanism already used in `CreateProposal` for the analogous proposal-ID race. On Postgres this locks the `credentials` row for `(vault_id, key)` for the duration of the transaction, so two concurrent overwrites of the same credential (e.g. a manual `credential set` racing a proposal apply) are serialized instead of both computing the same `MAX(version) + 1` and one failing on the `UNIQUE(vault_id, key, version)` constraint. No-op on SQLite, which already serializes writers itself.

`go build ./...`, `go vet ./...`, and `go test ./...` all still pass (same pre-existing, unrelated `internal/isolation` failure as before, present on `main` too).

@wankhede04

Copy link
Copy Markdown
Author

Greptile's comment has been addressed properly — please see the fix and details above. All checks are green (`go build`, `go vet`, `go test ./...`) aside from one pre-existing, unrelated failure in `internal/isolation` that also fails on `main`.

This is ready for review — maintainers, please take a look and merge at your convenience whenever it suits you. Thanks!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Credential version history and rollback for the built-in credential store

1 participant