Skip to content

feat: secret version value redaction#5392

Open
varonix0 wants to merge 4 commits intomainfrom
daniel/secret-version-value-redaction
Open

feat: secret version value redaction#5392
varonix0 wants to merge 4 commits intomainfrom
daniel/secret-version-value-redaction

Conversation

@varonix0
Copy link
Member

@varonix0 varonix0 commented Feb 6, 2026

Context

Added support for redacting secret value versions. We update the actual secret value in-place of the secret version, and mark it as redacted. This is done to combat secret spill and avoid storing secret values elsewhere than just the latest secret version.

This PR also incldues a fix for the automatic updating of secret references not incrementing secret versions correctly.

Steps to verify the change

Type

  • Fix
  • Feature
  • Improvement
  • Breaking
  • Docs
  • Chore

Checklist

  • Title follows the conventional commit format: type(scope): short description (scope is optional, e.g., fix: prevent crash on sync or fix(api): handle null response).
  • Tested locally
  • Updated docs (if needed)
  • Read the contributing guide

@varonix0 varonix0 self-assigned this Feb 6, 2026
@maidul98
Copy link
Collaborator

maidul98 commented Feb 6, 2026

Snyk checks have passed. No issues have been found so far.

Status Scanner Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Feb 6, 2026

Greptile Overview

Greptile Summary

  • Adds secret version value redaction support by introducing redaction columns on secret_versions_v2, a new EE v2 DELETE endpoint to redact a version, and corresponding audit log event.
  • Updates secret version list responses (v1 dashboard + v2 bridge) to include redaction metadata and updates UI to display redacted versions and warn on rollback.
  • Fixes secret reference update logic to increment secret versions properly and dedupe updates when multiple references exist.
  • Threads new secretVersionV2 DAL capabilities through services to support redaction checks and project/version gating.

Confidence Score: 3/5

  • This PR is mergeable after fixing a few correctness/backward-compatibility issues around redaction metadata and schemas.
  • Core idea is straightforward and migration is idempotent, but there are a couple of definite runtime/fk issues (redactedByUserId storing non-user actorId) plus likely breaking schema strictness on v1/dashboard and commit schemas. Also introduces new native regex usage against repo guidance.
  • backend/src/services/secret-v2-bridge/secret-v2-bridge-service.ts, backend/src/server/routes/v1/dashboard-router.ts, backend/src/services/folder-commit/folder-commit-schemas.ts, backend/src/services/secret-v2-bridge/secret-v2-bridge-fns.ts

Important Files Changed

Filename Overview
backend/src/db/migrations/20260205204147_redact-secret-version-values.ts Adds idempotent migration to add/remove isRedacted/redactedAt/redactedByUserId columns on secret_versions_v2.
backend/src/db/schemas/secret-versions-v2.ts Extends SecretVersionsV2Schema with redaction fields (isRedacted/redactedAt/redactedByUserId).
backend/src/ee/routes/v2/index.ts Registers new EE v2 secret-versions router under /secret-versions.
backend/src/ee/routes/v2/secret-version-router.ts Adds DELETE /api/v2/secret-versions/:versionId/redact-value endpoint calling secret.redactSecretVersionValue and emitting audit log.
backend/src/ee/services/audit-log/audit-log-types.ts Adds new audit log event type and metadata for secret version value redaction.
backend/src/server/routes/index.ts Wires secretVersionV2BridgeDAL into secret service factory as secretVersionV2DAL dependency.
backend/src/server/routes/v1/dashboard-router.ts Extends dashboard secretVersions response schema with redaction fields (isRedacted/redactedAt/redactedByUserId).
backend/src/services/folder-commit-changes/folder-commit-changes-dal.ts Extends SecretCommitChange type to include redaction fields on secretVersions.
backend/src/services/folder-commit/folder-commit-schemas.ts Updates folder commit Zod schemas to require redaction fields on secret versions.
backend/src/services/folder-commit/folder-commit-service.ts Extends internal SecretChange type to include redaction metadata per version.
backend/src/services/secret-v2-bridge/secret-v2-bridge-fns.ts Fixes reference update to increment secret version correctly and dedupe updates; still contains native regex replace usage.
backend/src/services/secret-v2-bridge/secret-v2-bridge-service.ts Adds redactSecretVersionValue implementation and includes redaction fields in version responses; sets redactedByUserId to actorId without ensuring it's a user id.
backend/src/services/secret-v2-bridge/secret-version-dal.ts Adds findOne that joins secret/folder/environment to surface projectId for a secret version.
backend/src/services/secret/secret-queue.ts Ensures inserted secret_versions_v2 rows include isRedacted=false on migration/queue paths.
backend/src/services/secret/secret-service.ts Adds secret.redactSecretVersionValue; also logs errors without required structured username pattern and adds redaction fields to version DTO.
backend/src/services/secret/secret-types.ts Adds TRedactSecretVersionValueDTO type for redaction call.
frontend/src/hooks/api/auditLogs/constants.tsx Adds UI label for REDACT_SECRET_VERSION_VALUE audit log event.
frontend/src/hooks/api/auditLogs/enums.tsx Adds REDACT_SECRET_VERSION_VALUE enum entry.
frontend/src/hooks/api/secrets/index.ts Re-exports new useRedactSecretValue mutation hook.
frontend/src/hooks/api/secrets/mutations.tsx Adds useRedactSecretValue mutation that DELETEs /api/v2/secret-versions/:versionId/redact-value and invalidates secret version query.
frontend/src/hooks/api/secrets/types.ts Extends SecretVersions type with redaction fields.
frontend/src/pages/pam/PamResourcesPage/route.tsx Formatting-only changes (quotes/trailing commas) to PAM resources route definition.
frontend/src/pages/secret-manager/CommitDetailsPage/components/SecretVersionDiffView/SecretVersionDiffView.tsx Adds tooltip warning when rolling back to a redacted secret version; excludes redaction fields from diff comparisons.
frontend/src/pages/secret-manager/SecretDashboardPage/components/SecretListView/SecretDetailSidebar.tsx Wires redaction mutation into secret version list and passes org members/canEditSecret to SecretVersionItem.
frontend/src/pages/secret-manager/SecretDashboardPage/components/SecretListView/SecretVersionItem.tsx Adds redacted state UI and redact action modal; shows 'Redacted by' using org user lookup.

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

25 files reviewed, 6 comments

Edit Code Review Agent Settings | Greptile

Comment on lines 1682 to 1684
secretVersions: secretRawSchema
.omit({ secretValue: true })
.extend({
Copy link
Contributor

Choose a reason for hiding this comment

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

Breaking API schema

The v1 dashboard response schema now requires isRedacted, redactedAt, and redactedByUserId on every secretVersions element (no .optional()), which will break older server responses and any clients expecting the prior shape if this endpoint can return versions without these fields (e.g., pre-migration rows, older data paths).

Fix: mark these fields optional in the schema (and/or ensure all producers always include them) to keep v1 API backwards-compatible.

Comment on lines 29 to +34
skipMultilineEncoding: z.boolean().nullable().optional(),
tags: z.array(z.string()).nullable().optional(),
metadata: z.unknown().nullable().optional(),
secretValue: z.string()
secretValue: z.string(),
isRedacted: z.boolean(),
redactedAt: z.date().nullable(),
Copy link
Contributor

Choose a reason for hiding this comment

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

Schema now requires fields

secretVersionSchema and secretResourceChangeSchema now require isRedacted/redactedAt/redactedByUserId. If older commits or older stored commit payloads don’t include these fields, parsing will fail at runtime when viewing commit details.

Fix: make these fields optional with sensible defaults (e.g. isRedacted: z.boolean().optional().default(false)) to preserve backward compatibility with existing commit records.

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Feb 6, 2026

Additional Comments (4)

frontend/src/hooks/api/secrets/mutations.tsx
Unused mutation parameter

useRedactSecretValue declares variables { versionId: string; secretId: string }, but secretId is never used in the request and is only used to invalidate secretKeys.getSecretVersion(secretId). If the backend response already includes secretId, prefer using that for invalidation to avoid relying on caller-supplied IDs (and to keep the mutation signature minimal). Alternatively, remove secretId from the mutation variables and invalidate more broadly/appropriately.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!


backend/src/services/secret-v2-bridge/secret-v2-bridge-service.ts
Wrong foreign key stored

redactedByUserId is being set to actorId in redactSecretVersionValue, but actorId can be an identity or other actor type (you pass actor: req.permission.type). Since the migration defines redactedByUserId as a FK to Users.id, redacting as a non-user actor will violate the FK (or store incorrect data if FK enforcement differs).

Fix: only set redactedByUserId when actor is a user (or store a separate redactedByActorId + redactedByActorType).


backend/src/services/secret-v2-bridge/secret-v2-bridge-fns.ts
Native regex used

destinationSecretPath.slice(1).replace(/\//g, ".") introduces a new native RegExp usage. Repo guidance requires using re2 for new regex to avoid ReDoS risks. Even though this pattern is simple, it still violates the policy.

Fix: replace this with a non-regex split/join (e.g. destinationSecretPath.slice(1).split("/").join(".")) or use re2.


backend/src/ee/routes/v2/secret-version-router.ts
Response schema mismatch

The route response is declared as SecretVersionsV2Schema.omit({ encryptedValue: true, encryptedComment: true }), but SecretVersionsV2Schema includes encryptedValue as a required field in the DB schema (and the service returns updatedSecretVersion from DAL). If encryptedValue is still present on the returned object, Fastify Zod response validation can fail (depending on how strict response validation is configured).

Fix: explicitly map/strip the returned secretVersion object to the response shape before returning, or adjust the schema to match the actual returned fields.

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.

2 participants