feat(secrets): add secrets diff to compare environments/paths - #333
feat(secrets): add secrets diff to compare environments/paths#333wankhede04 wants to merge 2 commits into
Conversation
Adds `infisical secrets diff --env=<a> --env2=<b>` to compare secrets between two environments (and optionally two paths via --path/--path2). Prints a table of added/removed/changed keys; values are masked by default and revealed with --show-values. Supports --output json/yaml/ dotenv for scripting. This reuses the existing GetAllEnvironmentVariables fetch path (same one used by `run`/`export`) and the GenericTable rendering helper, so no new API surface or storage is introduced. Fixes Infisical#330
|
| Filename | Overview |
|---|---|
| packages/cmd/secrets_diff.go | New secrets diff subcommand — two bugs found: masking arguments swapped for "added"/"removed" entries (both sides show empty), and the same-env guard fires before path2 is resolved (blocking valid same-env, different-path diffs) |
| packages/cmd/secrets_diff_test.go | Unit tests cover diff computation and sorting correctly; the masking test (TestMaskDiffValue_LeavesEmptyForMissingSide) validates the isolated helper in the direction that matches the buggy call-site, inadvertently confirming the wrong behavior rather than catching it |
Reviews (1): Last reviewed commit: "feat(secrets): add secrets diff to compa..." | Re-trigger Greptile
| if !showValues { | ||
| leftValue = maskDiffValue(entry.Status, secretDiffStatusRemoved, leftValue) | ||
| rightValue = maskDiffValue(entry.Status, secretDiffStatusAdded, rightValue) | ||
| } |
There was a problem hiding this comment.
The
emptyWhenStatus arguments are swapped between the left and right column calls, causing "removed" and "added" entries to show blank cells on the wrong side when masking is active.
secretDiffStatusRemoved means the key exists only in env1 (left) and is absent in env2 (right). So the left value should be masked ("******") and the right should be empty — but the current call passes secretDiffStatusRemoved as emptyWhenStatus to the left column, which makes it return "" for removed entries. The same inversion occurs for "added" keys in the right column.
Concrete failure: with --show-values omitted, a removed key displays two blank cells (left="" right=""), making it indistinguishable from a key with empty values on both sides. Added keys suffer the symmetric problem.
| if !showValues { | |
| leftValue = maskDiffValue(entry.Status, secretDiffStatusRemoved, leftValue) | |
| rightValue = maskDiffValue(entry.Status, secretDiffStatusAdded, rightValue) | |
| } | |
| if !showValues { | |
| leftValue = maskDiffValue(entry.Status, secretDiffStatusAdded, leftValue) | |
| rightValue = maskDiffValue(entry.Status, secretDiffStatusRemoved, rightValue) | |
| } |
There was a problem hiding this comment.
Fixed in 254559f — swapped emptyWhenStatus so removed keys mask the left column and added keys mask the right. Added maskDiffColumns + unit tests covering all three statuses.
| if env2 == env1 { | ||
| util.PrintErrorMessageAndExit("--env and --env2 must be different environments") | ||
| } |
There was a problem hiding this comment.
The
env2 == env1 guard fires before path2 is resolved, so a legitimate same-environment, different-path comparison (e.g., --env=staging --env2=staging --path=/app --path2=/db) is permanently blocked. The guard should also consider whether the paths differ before rejecting the request. Because path2 is declared after this point, the check must be moved to after path resolution (lines 83–85) to use path2 correctly.
| if env2 == env1 { | |
| util.PrintErrorMessageAndExit("--env and --env2 must be different environments") | |
| } | |
| if env2 == env1 && path2 == path1 { | |
| util.PrintErrorMessageAndExit("--env and --env2 must be different environments, or --path and --path2 must be different paths") | |
| } |
There was a problem hiding this comment.
Fixed in 254559f — path2 is resolved first, then we only reject when both env and path are identical. Same-env / different-path comparisons (e.g. --env=staging --env2=staging --path=/app --path2=/db) are now allowed.
Greptile P1s on Infisical#333: - Swap emptyWhenStatus args so removed keys mask the left column and added keys mask the right column (instead of blanking both sides). - Allow same-environment diffs when --path and --path2 differ; only reject when both env and path are identical.
|
Addressed both Greptile P1s in 254559f:
Unit tests cover the masking call-site and the validation cases. Ready for another look when convenient. |
Type of change
Summary
Closes #330.
There's currently no built-in way to compare secrets between two environments (or two paths) before promoting a change (e.g. staging → prod). Users end up hand-rolling this with two
infisical exportcalls plusdiff/yq— error-prone and doesn't mask sensitive values.Changes
Adds
infisical secrets diff:util.GetAllEnvironmentVariablespath (same onerun/exportalready use) — no new API client code.added,removed, orchangedbetween the two sides; keys with identical values on both sides are omitted.secrets set);--show-valuesreveals them.--output json|yaml|dotenvfor scripting, matching the pattern used by othersecretssubcommands.--path2defaults to--pathif omitted, so a same-path, cross-environment diff is the common one-flag case.Tests run
Added
packages/cmd/secrets_diff_test.gocovering:computeSecretDiff)Manually verified
infisical secrets diff --helpoutput and flag wiring.Note:
go vet ./packages/cmd/...fails on pre-existing, unrelated issues inrun.go(non-constant zerolog format strings) that exist onmainindependent of this change; ran tests with-vet=offto isolate this PR's changes.I confirm I have read the Contributing Guide and Code of Conduct.