Skip to content

Commit 254559f

Browse files
committed
fix(secrets): correct diff masking and same-env path comparison
Greptile P1s on #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.
1 parent 7d905b7 commit 254559f

2 files changed

Lines changed: 91 additions & 7 deletions

File tree

packages/cmd/secrets_diff.go

Lines changed: 37 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -67,9 +67,6 @@ func diffSecrets(cmd *cobra.Command, args []string) {
6767
if env2 == "" {
6868
util.PrintErrorMessageAndExit("The --env2 flag is required to specify the environment to compare against")
6969
}
70-
if env2 == env1 {
71-
util.PrintErrorMessageAndExit("--env and --env2 must be different environments")
72-
}
7370

7471
path1, err := cmd.Flags().GetString("path")
7572
if err != nil {
@@ -80,8 +77,12 @@ func diffSecrets(cmd *cobra.Command, args []string) {
8077
if err != nil {
8178
util.HandleError(err, "Unable to parse flag")
8279
}
83-
if path2 == "" {
84-
path2 = path1
80+
path2 = resolveDiffPath2(path1, path2)
81+
82+
// Same environment is allowed when comparing different paths
83+
// (e.g. --env=staging --env2=staging --path=/app --path2=/db).
84+
if err := validateDiffTargets(env1, env2, path1, path2); err != nil {
85+
util.PrintErrorMessageAndExit(err.Error())
8586
}
8687

8788
includeImports, err := cmd.Flags().GetBool("include-imports")
@@ -225,15 +226,44 @@ func printSecretDiffTable(diff []secretDiffEntry, env1, env2 string, showValues
225226
leftValue := entry.LeftValue
226227
rightValue := entry.RightValue
227228
if !showValues {
228-
leftValue = maskDiffValue(entry.Status, secretDiffStatusRemoved, leftValue)
229-
rightValue = maskDiffValue(entry.Status, secretDiffStatusAdded, rightValue)
229+
leftValue, rightValue = maskDiffColumns(entry.Status, leftValue, rightValue)
230230
}
231231
rows = append(rows, []string{entry.Key, entry.Status, leftValue, rightValue})
232232
}
233233

234234
visualize.GenericTable(headers, rows)
235235
}
236236

237+
// resolveDiffPath2 defaults --path2 to --path when --path2 is omitted.
238+
func resolveDiffPath2(path1, path2 string) string {
239+
if path2 == "" {
240+
return path1
241+
}
242+
return path2
243+
}
244+
245+
// validateDiffTargets rejects comparisons where both environment and path are
246+
// identical (there would be nothing meaningful to compare).
247+
func validateDiffTargets(env1, env2, path1, path2 string) error {
248+
if env1 == env2 && path1 == path2 {
249+
return fmt.Errorf("--env/--env2 and --path/--path2 must not both be identical; use different environments or different paths")
250+
}
251+
return nil
252+
}
253+
254+
// maskDiffColumns returns masked left/right display values for a table row.
255+
//
256+
// Semantics when masking:
257+
// - removed: key exists only on the left → left masked, right empty
258+
// - added: key exists only on the right → left empty, right masked
259+
// - changed: both sides exist → both masked
260+
func maskDiffColumns(status, leftValue, rightValue string) (string, string) {
261+
// Left is empty when the key was added on the right (absent on the left).
262+
// Right is empty when the key was removed from the left (absent on the right).
263+
return maskDiffValue(status, secretDiffStatusAdded, leftValue),
264+
maskDiffValue(status, secretDiffStatusRemoved, rightValue)
265+
}
266+
237267
// maskDiffValue masks a value unless it's meant to be empty (i.e. the key
238268
// doesn't exist on that side of the diff).
239269
func maskDiffValue(status, emptyWhenStatus, value string) string {

packages/cmd/secrets_diff_test.go

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,3 +106,57 @@ func TestMaskDiffValue_LeavesEmptyForMissingSide(t *testing.T) {
106106
t.Errorf("expected empty value when status matches emptyWhenStatus, got %q", got)
107107
}
108108
}
109+
110+
func TestMaskDiffColumns_RemovedShowsMaskedLeftAndEmptyRight(t *testing.T) {
111+
left, right := maskDiffColumns(secretDiffStatusRemoved, "left-secret", "")
112+
if left != "******" {
113+
t.Errorf("expected masked left for removed key, got %q", left)
114+
}
115+
if right != "" {
116+
t.Errorf("expected empty right for removed key, got %q", right)
117+
}
118+
}
119+
120+
func TestMaskDiffColumns_AddedShowsEmptyLeftAndMaskedRight(t *testing.T) {
121+
left, right := maskDiffColumns(secretDiffStatusAdded, "", "right-secret")
122+
if left != "" {
123+
t.Errorf("expected empty left for added key, got %q", left)
124+
}
125+
if right != "******" {
126+
t.Errorf("expected masked right for added key, got %q", right)
127+
}
128+
}
129+
130+
func TestMaskDiffColumns_ChangedMasksBothSides(t *testing.T) {
131+
left, right := maskDiffColumns(secretDiffStatusChanged, "old", "new")
132+
if left != "******" || right != "******" {
133+
t.Errorf("expected both sides masked for changed key, got left=%q right=%q", left, right)
134+
}
135+
}
136+
137+
func TestResolveDiffPath2_DefaultsToPath1(t *testing.T) {
138+
if got := resolveDiffPath2("/app", ""); got != "/app" {
139+
t.Errorf("expected path2 to default to path1, got %q", got)
140+
}
141+
if got := resolveDiffPath2("/app", "/db"); got != "/db" {
142+
t.Errorf("expected explicit path2 to be preserved, got %q", got)
143+
}
144+
}
145+
146+
func TestValidateDiffTargets_AllowsSameEnvDifferentPath(t *testing.T) {
147+
if err := validateDiffTargets("staging", "staging", "/app", "/db"); err != nil {
148+
t.Errorf("expected same-env different-path comparison to be allowed, got %v", err)
149+
}
150+
}
151+
152+
func TestValidateDiffTargets_RejectsIdenticalEnvAndPath(t *testing.T) {
153+
if err := validateDiffTargets("staging", "staging", "/app", "/app"); err == nil {
154+
t.Errorf("expected identical env+path comparison to be rejected")
155+
}
156+
}
157+
158+
func TestValidateDiffTargets_AllowsDifferentEnvSamePath(t *testing.T) {
159+
if err := validateDiffTargets("staging", "production", "/app", "/app"); err != nil {
160+
t.Errorf("expected different-env same-path comparison to be allowed, got %v", err)
161+
}
162+
}

0 commit comments

Comments
 (0)