Skip to content

Commit 10f8c4a

Browse files
DinanathDashRawJat
DinanathDash
and
RawJat
authored
Enhance approve command with visual diff and execution checks (#192)
This pull request enhances the `approve` command in the CLI by introducing an interactive visual diff for pending secret mutations, improving user experience and safety. It also significantly expands the test suite for the `approve` command and refactors test utilities to better isolate environment variables, ensuring more reliable and maintainable tests. **Approve command improvements:** * The `approve` command now fetches pending secret mutations, displays a visual diff of changes (additions, deletions, modifications), and prompts the user for interactive approval. If not in an interactive terminal, it aborts with an error. This improves clarity and prevents accidental approvals. * Added a helper function `isInteractive()` to reliably detect if the terminal is interactive, supporting robust interactive prompts and test overrides. * Refactored diff logic by extracting `computeDiffFromMap`, allowing the diff to be generated from an in-memory map, which simplifies both the command logic and testing. **Testing improvements:** * Major expansion of tests for the `approve` command, covering interactive approval, abort scenarios, non-interactive errors, and token validation. Introduced helper functions for setting up mock servers, home directories, and filtering environment variables for isolation. * Added a dedicated test for `computeDiffFromMap` to ensure diff calculation correctness. **Test utility and environment management:** * Introduced `filterEnv` and `filterEnvRoot` helper functions to consistently exclude sensitive or test-specific environment variables from test subprocesses, reducing flakiness and side effects across the test suite. Updated all affected tests to use these helpers. **Dependency and import updates:** * Updated imports in `approve.go` and related files to include new internal modules and Go standard libraries required for the new functionality. These changes together make the approval process safer and more user-friendly, while also making the codebase easier to test and maintain. --- Co-authored-by: Rajat Patra <113469515+RawJat@users.noreply.github.com>
2 parents b2e6209 + ba7473a commit 10f8c4a

11 files changed

Lines changed: 465 additions & 67 deletions

File tree

‎cli-go/cmd/approve.go‎

Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,13 @@ import (
1111
"strings"
1212
"syscall"
1313

14+
"bufio"
15+
"github.com/DinanathDash/Envault/cli-go/internal/api"
16+
"github.com/DinanathDash/Envault/cli-go/internal/crypto"
1417
"github.com/DinanathDash/Envault/cli-go/internal/ui"
1518
"github.com/spf13/cobra"
1619
"github.com/spf13/viper"
20+
"net/url"
1721
)
1822

1923
type approveResponse struct {
@@ -70,6 +74,125 @@ var approveCmd = &cobra.Command{
7074
}
7175
}()
7276

77+
getReq, err := http.NewRequestWithContext(ctx, http.MethodGet, fmt.Sprintf("%s/api/approve/%s", baseURL, approvalID), nil)
78+
if err != nil {
79+
fmt.Fprintln(os.Stderr, ui.ColorRed(fmt.Sprintf("Failed to create request: %v", err)))
80+
os.Exit(1)
81+
}
82+
getReq.Header.Set("Content-Type", "application/json")
83+
getReq.Header.Set("Authorization", "Bearer "+token)
84+
85+
getLoader := ui.NewLoader(ui.LoaderThemeSync, "Fetching pending mutations...")
86+
getLoader.Start()
87+
getResp, err := (&http.Client{}).Do(getReq)
88+
getLoader.Stop()
89+
if err != nil {
90+
fmt.Fprintln(os.Stderr, ui.ColorRed(fmt.Sprintf("Failed to fetch pending approval: %v", err)))
91+
os.Exit(1)
92+
}
93+
94+
if getResp.StatusCode >= 400 {
95+
fmt.Fprintln(os.Stderr, ui.ColorRed(fmt.Sprintf("Failed to fetch pending approval (status %d).", getResp.StatusCode)))
96+
os.Exit(1)
97+
}
98+
99+
type PayloadMutation struct {
100+
Key string `json:"key"`
101+
Value string `json:"value"`
102+
Action string `json:"action"`
103+
}
104+
type PendingApprovalData struct {
105+
ProjectID string `json:"project_id"`
106+
PayloadData struct {
107+
Environment string `json:"environment"`
108+
EnvironmentSlug string `json:"environmentSlug"`
109+
Mutations []PayloadMutation `json:"mutations"`
110+
} `json:"payload_data"`
111+
}
112+
113+
var pendingData PendingApprovalData
114+
if err := json.NewDecoder(getResp.Body).Decode(&pendingData); err != nil {
115+
fmt.Fprintln(os.Stderr, ui.ColorRed("Failed to parse pending approval data."))
116+
os.Exit(1)
117+
}
118+
getResp.Body.Close()
119+
120+
targetEnv := pendingData.PayloadData.Environment
121+
if targetEnv == "" {
122+
targetEnv = pendingData.PayloadData.EnvironmentSlug
123+
}
124+
if targetEnv == "" || pendingData.ProjectID == "" {
125+
fmt.Fprintln(os.Stderr, ui.ColorRed("Pending approval is missing environment or project context."))
126+
os.Exit(1)
127+
}
128+
129+
diffLoader := ui.NewLoader(ui.LoaderThemeCheck, "Preparing visual diff...")
130+
diffLoader.Start()
131+
132+
client := api.NewClient()
133+
path := fmt.Sprintf("/projects/%s/secrets?environment=%s", pendingData.ProjectID, url.QueryEscape(targetEnv))
134+
respBytes, err := client.GetWithContext(ctx, path)
135+
diffLoader.Stop()
136+
137+
var remote SecretsResponse
138+
if err == nil {
139+
_ = json.Unmarshal(respBytes, &remote)
140+
}
141+
142+
localEnv := make(map[string]string)
143+
for _, s := range remote.Secrets {
144+
plaintext := "<<DECRYPTION_FAILED>>"
145+
if s.Ciphertext != "<<DECRYPTION_FAILED>>" && s.Dek != "" {
146+
if p, err := crypto.DecryptAESGCM(s.Ciphertext, s.Dek); err == nil {
147+
plaintext = p
148+
}
149+
}
150+
localEnv[s.Key] = plaintext
151+
}
152+
153+
for _, m := range pendingData.PayloadData.Mutations {
154+
if m.Action == "delete" {
155+
delete(localEnv, m.Key)
156+
} else {
157+
localEnv[m.Key] = "<<PENDING_VALUE_" + m.Key + ">>"
158+
}
159+
}
160+
161+
result, err := computeDiffFromMap(ctx, pendingData.ProjectID, targetEnv, localEnv)
162+
if err != nil {
163+
fmt.Fprintln(os.Stderr, ui.ColorRed("Failed to generate diff."))
164+
os.Exit(1)
165+
}
166+
167+
fmt.Printf("\n%s %s (%s)\n", ui.ColorBold("Environment:"), targetEnv, pendingData.ProjectID)
168+
fmt.Println(ui.ColorBold("Pending Changes:"))
169+
170+
for _, k := range result.Additions {
171+
fmt.Println(ui.ColorGreen("+ " + k))
172+
}
173+
for _, k := range result.Deletions {
174+
fmt.Println(ui.ColorRed("- " + k))
175+
}
176+
for _, k := range result.Modifications {
177+
fmt.Println(ui.ColorYellow("~ " + k))
178+
}
179+
180+
if len(result.Additions) == 0 && len(result.Deletions) == 0 && len(result.Modifications) == 0 {
181+
fmt.Println(ui.ColorGreen("No differences. (Empty mutation)"))
182+
} else {
183+
if !isInteractive() {
184+
fmt.Fprintln(os.Stderr, ui.ColorRed("Error: Visual diff requires an interactive terminal."))
185+
os.Exit(1)
186+
}
187+
fmt.Printf("\nReview the changes above. Approve this mutation? (y/N): ")
188+
reader := bufio.NewReader(os.Stdin)
189+
response, _ := reader.ReadString('\n')
190+
if strings.ToLower(strings.TrimSpace(response)) != "y" {
191+
fmt.Println(ui.ColorYellow("Approval cancelled."))
192+
os.Exit(0)
193+
}
194+
}
195+
73196
req, err := http.NewRequestWithContext(
74197
ctx,
75198
http.MethodPost,
@@ -123,3 +246,14 @@ var approveCmd = &cobra.Command{
123246
func init() {
124247
rootCmd.AddCommand(approveCmd)
125248
}
249+
250+
func isInteractive() bool {
251+
if os.Getenv("ENVAULT_TEST_FORCE_TTY") == "1" {
252+
return true
253+
}
254+
if os.Getenv("ENVAULT_TEST_FORCE_NON_INTERACTIVE") == "1" {
255+
return false
256+
}
257+
stat, _ := os.Stdin.Stat()
258+
return (stat.Mode() & os.ModeCharDevice) != 0
259+
}

‎cli-go/cmd/approve_test.go‎

Lines changed: 124 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -11,22 +11,51 @@ import (
1111
"testing"
1212
)
1313

14-
func TestApproveCmd_Success(t *testing.T) {
15-
called := false
16-
mockSrv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
17-
if r.Method != http.MethodPost || !strings.HasPrefix(r.URL.Path, "/api/approve/") {
18-
w.WriteHeader(http.StatusNotFound)
14+
func filterEnv(env []string) []string {
15+
filtered := []string{}
16+
for _, e := range env {
17+
if !strings.HasPrefix(e, "ENVAULT_TOKEN=") && !strings.HasPrefix(e, "ENVAULT_SERVICE_TOKEN=") && !strings.HasPrefix(e, "ENVAULT_TEST_FORCE_TTY=") {
18+
filtered = append(filtered, e)
19+
}
20+
}
21+
return filtered
22+
}
23+
24+
func buildMockServer(t *testing.T, calls *int) *httptest.Server {
25+
return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
26+
if r.Method == http.MethodGet && strings.HasPrefix(r.URL.Path, "/api/approve/") {
27+
*calls++
28+
w.Header().Set("Content-Type", "application/json")
29+
_, _ = w.Write([]byte(`{
30+
"project_id": "proj-123",
31+
"payload_data": {
32+
"environment": "staging",
33+
"mutations": [
34+
{"key": "NEW_KEY", "value": "123", "action": "upsert"}
35+
]
36+
}
37+
}`))
1938
return
2039
}
21-
called = true
22-
if got := r.Header.Get("Authorization"); got != "Bearer envault_at_test-token" {
23-
t.Fatalf("unexpected authorization header: %q", got)
40+
41+
if r.Method == http.MethodGet && strings.HasPrefix(r.URL.Path, "/projects/proj-123/secrets") {
42+
w.Header().Set("Content-Type", "application/json")
43+
_, _ = w.Write([]byte(`{"secrets": []}`))
44+
return
45+
}
46+
47+
if r.Method == http.MethodPost && strings.HasPrefix(r.URL.Path, "/api/approve/") {
48+
*calls++
49+
w.Header().Set("Content-Type", "application/json")
50+
_, _ = w.Write([]byte(`{"success":true,"message":"Request has been approved"}`))
51+
return
2452
}
25-
w.Header().Set("Content-Type", "application/json")
26-
_, _ = w.Write([]byte(`{"success":true,"message":"Request has been approved"}`))
53+
54+
w.WriteHeader(http.StatusNotFound)
2755
}))
28-
defer mockSrv.Close()
56+
}
2957

58+
func setupHome(t *testing.T) string {
3059
tmp := t.TempDir()
3160
home := filepath.Join(tmp, "home")
3261
if err := os.MkdirAll(filepath.Join(home, ".envault"), 0o700); err != nil {
@@ -36,54 +65,117 @@ func TestApproveCmd_Success(t *testing.T) {
3665
if err := os.WriteFile(filepath.Join(home, ".envault", "config.toml"), []byte(config), 0o600); err != nil {
3766
t.Fatalf("write config: %v", err)
3867
}
68+
return home
69+
}
3970

71+
func TestApproveCmd_Success_Interactive(t *testing.T) {
72+
calls := 0
73+
mockSrv := buildMockServer(t, &calls)
74+
defer mockSrv.Close()
75+
home := setupHome(t)
4076
bin := buildBinary(t)
77+
4178
cmd := exec.Command(bin, "approve", "approval-123")
42-
cmd.Env = append(os.Environ(),
79+
cmd.Env = append(filterEnv(os.Environ()),
4380
"HOME="+home,
4481
"NEXT_PUBLIC_APP_URL="+mockSrv.URL,
82+
"ENVAULT_CLI_URL="+mockSrv.URL,
83+
"ENVAULT_ALLOW_INSECURE_HTTP=1",
84+
"ENVAULT_TEST_FORCE_NON_INTERACTIVE=1",
85+
"ENVAULT_TEST_FORCE_TTY=1",
4586
)
46-
4787
var outBuf, errBuf bytes.Buffer
4888
cmd.Stdout = &outBuf
4989
cmd.Stderr = &errBuf
90+
cmd.Stdin = strings.NewReader("y\n")
5091

5192
if err := cmd.Run(); err != nil {
52-
t.Fatalf("approve command failed: %v\nstderr:\n%s", err, errBuf.String())
93+
t.Fatalf("approve command failed: %v\nstderr:\n%s\nstdout:\n%s", err, errBuf.String(), outBuf.String())
5394
}
5495

55-
if !called {
56-
t.Fatal("expected /api/approve endpoint to be called")
57-
}
5896
if !strings.Contains(outBuf.String(), "Request has been approved") {
59-
t.Fatalf("expected success output, got stdout:\n%s\nstderr:\n%s", outBuf.String(), errBuf.String())
97+
t.Fatalf("expected success output, got stdout:\n%s", outBuf.String())
6098
}
6199
}
62100

63-
func TestApproveCmd_RejectsNonAccessToken(t *testing.T) {
64-
tmp := t.TempDir()
65-
home := filepath.Join(tmp, "home")
66-
if err := os.MkdirAll(filepath.Join(home, ".envault"), 0o700); err != nil {
67-
t.Fatalf("mkdir home/.envault: %v", err)
68-
}
69-
config := "[auth]\ntoken = \"envault_rt_refresh-only\"\n"
70-
if err := os.WriteFile(filepath.Join(home, ".envault", "config.toml"), []byte(config), 0o600); err != nil {
71-
t.Fatalf("write config: %v", err)
101+
func TestApproveCmd_Abort_Interactive(t *testing.T) {
102+
calls := 0
103+
mockSrv := buildMockServer(t, &calls)
104+
defer mockSrv.Close()
105+
home := setupHome(t)
106+
bin := buildBinary(t)
107+
108+
cmd := exec.Command(bin, "approve", "approval-123")
109+
cmd.Env = append(filterEnv(os.Environ()),
110+
"HOME="+home,
111+
"NEXT_PUBLIC_APP_URL="+mockSrv.URL,
112+
"ENVAULT_CLI_URL="+mockSrv.URL,
113+
"ENVAULT_ALLOW_INSECURE_HTTP=1",
114+
"ENVAULT_TEST_FORCE_NON_INTERACTIVE=1",
115+
"ENVAULT_TEST_FORCE_TTY=1",
116+
)
117+
var outBuf, errBuf bytes.Buffer
118+
cmd.Stdout = &outBuf
119+
cmd.Stderr = &errBuf
120+
cmd.Stdin = strings.NewReader("N\n")
121+
122+
_ = cmd.Run()
123+
124+
if !strings.Contains(outBuf.String(), "Approval cancelled.") {
125+
t.Fatalf("expected abort output, got stdout:\n%s", outBuf.String())
72126
}
127+
}
73128

129+
func TestApproveCmd_NonInteractive(t *testing.T) {
130+
calls := 0
131+
mockSrv := buildMockServer(t, &calls)
132+
defer mockSrv.Close()
133+
home := setupHome(t)
74134
bin := buildBinary(t)
75-
cmd := exec.Command(bin, "approve", "approval-123")
76-
cmd.Env = append(os.Environ(), "HOME="+home)
77135

136+
cmd := exec.Command(bin, "approve", "approval-123")
137+
cmd.Env = append(filterEnv(os.Environ()),
138+
"HOME="+home,
139+
"NEXT_PUBLIC_APP_URL="+mockSrv.URL,
140+
"ENVAULT_CLI_URL="+mockSrv.URL,
141+
"ENVAULT_ALLOW_INSECURE_HTTP=1",
142+
"ENVAULT_TEST_FORCE_NON_INTERACTIVE=1",
143+
)
78144
var outBuf, errBuf bytes.Buffer
79145
cmd.Stdout = &outBuf
80146
cmd.Stderr = &errBuf
81147

148+
pr, pw, _ := os.Pipe()
149+
pw.Close()
150+
cmd.Stdin = pr
151+
82152
err := cmd.Run()
153+
154+
t.Logf("stdout: %s\nstderr: %s", outBuf.String(), errBuf.String())
83155
if err == nil {
84-
t.Fatalf("expected approve command to fail with non envault_at_ token")
156+
t.Fatalf("expected error due to non-interactive environment")
85157
}
86-
if !strings.Contains(errBuf.String(), "envault_at_") {
87-
t.Fatalf("expected token-format validation error, got stderr:\n%s", errBuf.String())
158+
159+
if !strings.Contains(errBuf.String(), "Visual diff requires an interactive terminal") {
160+
t.Fatalf("expected visual diff error, got stderr:\n%s", errBuf.String())
161+
}
162+
}
163+
164+
func TestApproveCmd_RejectsNonAccessToken(t *testing.T) {
165+
bin := buildBinary(t)
166+
tmp := t.TempDir()
167+
home := filepath.Join(tmp, "home")
168+
os.MkdirAll(filepath.Join(home, ".envault"), 0o700)
169+
os.WriteFile(filepath.Join(home, ".envault", "config.toml"), []byte("[auth]\ntoken = \"envault_rt_refresh-only\"\n"), 0o600)
170+
171+
cmd := exec.Command(bin, "approve", "approval-123")
172+
cmd.Env = append(os.Environ(), "HOME="+home)
173+
174+
var outBuf, errBuf bytes.Buffer
175+
cmd.Stdout = &outBuf
176+
cmd.Stderr = &errBuf
177+
178+
if err := cmd.Run(); err == nil {
179+
t.Fatalf("expected failure")
88180
}
89181
}

‎cli-go/cmd/deploy.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ var deployCmd = &cobra.Command{
3737
// Fallback to check token stored in global config
3838
token = viper.GetString("auth.token")
3939
}
40-
40+
4141
if strings.HasPrefix(token, "envault_svc_") {
4242
fmt.Fprintln(os.Stderr, ui.ColorRed("Error: Deploy is disabled for Service Tokens."))
4343
fmt.Fprintln(os.Stderr, ui.ColorYellow(" CI/CD pipelines must be strictly read-only. Use 'envault run' or 'envault pull' instead."))

‎cli-go/cmd/diff.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,10 @@ func computeDiff(ctx context.Context, projectID, targetEnv, targetFile string) (
107107
if err != nil {
108108
return diffResult{}, err
109109
}
110+
return computeDiffFromMap(ctx, projectID, targetEnv, localEnv)
111+
}
112+
113+
func computeDiffFromMap(ctx context.Context, projectID, targetEnv string, localEnv map[string]string) (diffResult, error) {
110114

111115
client := api.NewClient()
112116
path := fmt.Sprintf("/projects/%s/secrets?environment=%s", projectID, url.QueryEscape(targetEnv))

0 commit comments

Comments
 (0)