From 88bf16da2f5bc5145026e04b493c1a24aead0b7a Mon Sep 17 00:00:00 2001 From: Nimish Date: Thu, 6 Aug 2026 13:07:59 +0530 Subject: [PATCH 1/2] fix: show app and environment when a path filter matches no secrets MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `phase run` and `phase shell` derived the Application and Environment labels from the fetched secret rows. When the fetch returned nothing they printed blank names: 🚀 Injected 0 secrets from Application: , Environment: This is easy to hit: both commands default to `--path /`, paths are matched exactly, so an app whose secrets all live in folders injects zero secrets at the default path. The blank names then read as a failure to resolve the app — e.g. an unreadable .phase.json — rather than a path filter that matched nothing. Resolve the names from the account's app list when no result row carries them, so the queried app and environment are always reported. The lookup only runs on the empty path, leaving the normal path free of extra requests. `phase secrets list` had the same root cause: its header read `secrets[0]`, so an empty result printed a bare "No secrets to display." with no context at all. It now renders the resolved context too. Also say why the result was empty, since exact-path matching is the usual reason: `run`, `shell`, `secrets list` and `secrets export` point at `--path ""` to search all paths, and `secrets get` says which path it searched. The `export` note goes to stderr so piped output stays clean. `phase shell` additionally left PHASE_APP and PHASE_ENV unset whenever zero secrets were loaded; they now follow the resolved context. --- src/cmd/context.go | 53 ++++++++++++++++++++++ src/cmd/context_names_test.go | 51 ++++++++++++++++++++++ src/cmd/run.go | 32 ++++---------- src/cmd/secrets_export.go | 7 +++ src/cmd/secrets_get.go | 3 ++ src/cmd/secrets_list.go | 3 +- src/cmd/shell.go | 33 +++++--------- src/pkg/display/tree.go | 22 ++++++---- src/pkg/display/tree_test.go | 82 +++++++++++++++++++++++++++++++++++ src/pkg/phase/phase.go | 16 +++++++ 10 files changed, 248 insertions(+), 54 deletions(-) create mode 100644 src/cmd/context.go create mode 100644 src/cmd/context_names_test.go create mode 100644 src/pkg/display/tree_test.go diff --git a/src/cmd/context.go b/src/cmd/context.go new file mode 100644 index 00000000..92ee55d7 --- /dev/null +++ b/src/cmd/context.go @@ -0,0 +1,53 @@ +package cmd + +import ( + "fmt" + "sort" + "strings" + + "github.com/phasehq/cli/pkg/phase" + "github.com/phasehq/cli/pkg/util" + sdk "github.com/phasehq/golang-sdk/v2/phase" +) + +func mapKeys(m map[string]bool) []string { + var keys []string + for k := range m { + keys = append(keys, k) + } + sort.Strings(keys) + return keys +} + +// contextNames returns the application and environment names to display for a +// fetch. They are read off the returned secrets when possible, and resolved +// from the account's app list when the fetch came back empty — otherwise a +// path that matches no secrets would report a blank app and environment and +// look like a broken .phase.json. +func contextNames(p *sdk.Phase, secrets []sdk.SecretResult, appName, envName, appID string) (string, string) { + apps := map[string]bool{} + envs := map[string]bool{} + for _, s := range secrets { + if s.Application != "" { + apps[s.Application] = true + } + if s.Environment != "" { + envs[s.Environment] = true + } + } + if len(apps) > 0 && len(envs) > 0 { + return strings.Join(mapKeys(apps), ", "), strings.Join(mapKeys(envs), ", ") + } + return phase.ResolveNames(p, appName, envName, appID) +} + +// emptyPathHint explains why a path filter matched nothing. Secrets are filtered +// by exact path, so secrets kept in folders are invisible at the default '/'. +// Returns "" when no path filter was applied, since every path was searched. +func emptyPathHint(path, verb string) string { + if path == "" { + return "" + } + return fmt.Sprintf("💡 No secrets found at path %s. Secrets under other paths are not included — pass %s to %s secrets from all paths.\n", + util.BoldYellowErr(path), util.BoldErr(`--path ""`), verb) +} diff --git a/src/cmd/context_names_test.go b/src/cmd/context_names_test.go new file mode 100644 index 00000000..fa47a284 --- /dev/null +++ b/src/cmd/context_names_test.go @@ -0,0 +1,51 @@ +package cmd + +import ( + "strings" + "testing" + + sdk "github.com/phasehq/golang-sdk/v2/phase" +) + +func TestContextNamesReadsNamesFromSecrets(t *testing.T) { + secrets := []sdk.SecretResult{ + {Key: "A", Application: "my-app", Environment: "Development"}, + {Key: "B", Application: "my-app", Environment: "Development"}, + } + + // A nil client is safe here: names come off the rows, so no lookup is made. + app, env := contextNames(nil, secrets, "my", "dev", "") + if app != "my-app" { + t.Fatalf("unexpected application: got %q want %q", app, "my-app") + } + if env != "Development" { + t.Fatalf("unexpected environment: got %q want %q", env, "Development") + } +} + +func TestContextNamesIgnoresBlankNamesOnRows(t *testing.T) { + secrets := []sdk.SecretResult{{Key: "A"}} + + // With no usable names on the rows, contextNames falls back to a lookup. + // The zero-value client has no host, so the lookup fails at request time + // without any network I/O and the selectors are returned as-is. + app, env := contextNames(&sdk.Phase{}, secrets, "my-app", "Development", "") + if app != "my-app" || env != "Development" { + t.Fatalf("unexpected fallback: got %q/%q want %q/%q", app, env, "my-app", "Development") + } +} + +func TestEmptyPathHint(t *testing.T) { + // No path filter means every path was already searched — nothing to suggest. + if got := emptyPathHint("", "inject"); got != "" { + t.Fatalf("expected no hint for an empty path filter, got %q", got) + } + + hint := emptyPathHint("/", "inject") + if !strings.Contains(hint, `--path ""`) { + t.Fatalf("hint should suggest --path \"\", got %q", hint) + } + if !strings.Contains(hint, "inject") { + t.Fatalf("hint should use the caller's verb, got %q", hint) + } +} diff --git a/src/cmd/run.go b/src/cmd/run.go index 77f83d74..4ff45b96 100644 --- a/src/cmd/run.go +++ b/src/cmd/run.go @@ -85,30 +85,22 @@ func runRun(cmd *cobra.Command, args []string) error { // Print injection stats to stderr (matches Python CLI behavior) secretCount := len(resolvedSecrets) - apps := map[string]bool{} - envs := map[string]bool{} - for _, s := range allSecrets { - if _, ok := resolvedSecrets[s.Key]; ok { - if s.Application != "" { - apps[s.Application] = true - } - envs[s.Environment] = true - } - } - appNames := mapKeys(apps) - envNames := mapKeys(envs) + appLabel, envLabel := contextNames(p, allSecrets, appName, envName, appID) if path != "" && path != "/" { fmt.Fprintf(os.Stderr, "🚀 Injected %s secrets from Application: %s, Environment: %s, Path: %s\n", util.BoldMagentaErr(fmt.Sprintf("%d", secretCount)), - util.BoldCyanErr(strings.Join(appNames, ", ")), - util.BoldGreenErr(strings.Join(envNames, ", ")), + util.BoldCyanErr(appLabel), + util.BoldGreenErr(envLabel), util.BoldYellowErr(path)) } else { fmt.Fprintf(os.Stderr, "🚀 Injected %s secrets from Application: %s, Environment: %s\n", util.BoldMagentaErr(fmt.Sprintf("%d", secretCount)), - util.BoldCyanErr(strings.Join(appNames, ", ")), - util.BoldGreenErr(strings.Join(envNames, ", "))) + util.BoldCyanErr(appLabel), + util.BoldGreenErr(envLabel)) + } + if secretCount == 0 { + fmt.Fprint(os.Stderr, emptyPathHint(path, "inject")) } // Build environment: inherit current env and append secrets @@ -138,11 +130,3 @@ func runRun(cmd *cobra.Command, args []string) error { } return nil } - -func mapKeys(m map[string]bool) []string { - var keys []string - for k := range m { - keys = append(keys, k) - } - return keys -} diff --git a/src/cmd/secrets_export.go b/src/cmd/secrets_export.go index f2c7bbc4..8bb19b75 100644 --- a/src/cmd/secrets_export.go +++ b/src/cmd/secrets_export.go @@ -111,6 +111,13 @@ func runSecretsExport(cmd *cobra.Command, args []string) error { } } + // An export that produces nothing is otherwise silent; say why on stderr so + // it can't be mistaken for a broken app or environment. stderr keeps the + // exported document on stdout intact for piping. + if len(secretsList) == 0 { + fmt.Fprint(os.Stderr, emptyPathHint(path, "export")) + } + switch format { case "json": util.ExportJSON(secretsList) diff --git a/src/cmd/secrets_get.go b/src/cmd/secrets_get.go index dd969967..e153bc38 100644 --- a/src/cmd/secrets_get.go +++ b/src/cmd/secrets_get.go @@ -88,6 +88,9 @@ func runSecretsGet(cmd *cobra.Command, args []string) error { } if len(results) == 0 { + if path != "" { + return fmt.Errorf("🔍 No matching secrets found at path %s — secrets under other paths are not searched. Pass --path \"\" to search all paths", path) + } return fmt.Errorf("🔍 No matching secrets found") } diff --git a/src/cmd/secrets_list.go b/src/cmd/secrets_list.go index 949deef1..58ae143b 100644 --- a/src/cmd/secrets_list.go +++ b/src/cmd/secrets_list.go @@ -64,7 +64,8 @@ func listSecrets(p *sdk.Phase, envName, appName, appID, tags, path string, show, return err } - display.RenderSecretsTree(secrets, show) + appLabel, envLabel := contextNames(p, secrets, appName, envName, appID) + display.RenderSecretsTree(secrets, show, appLabel, envLabel, path) return nil } diff --git a/src/cmd/shell.go b/src/cmd/shell.go index ebb8af13..5333cbfd 100644 --- a/src/cmd/shell.go +++ b/src/cmd/shell.go @@ -4,7 +4,6 @@ import ( "fmt" "os" "os/exec" - "strings" "github.com/phasehq/cli/pkg/ai" "github.com/phasehq/cli/pkg/phase" @@ -79,18 +78,7 @@ func runShell(cmd *cobra.Command, args []string) error { } // Collect env/app info for display - apps := map[string]bool{} - envs := map[string]bool{} - for _, s := range allSecrets { - if _, ok := resolvedSecrets[s.Key]; ok { - if s.Application != "" { - apps[s.Application] = true - } - envs[s.Environment] = true - } - } - appNames := mapKeys(apps) - envNames := mapKeys(envs) + appLabel, envLabel := contextNames(p, allSecrets, appName, envName, appID) // Build environment: inherit current env, add secrets and shell markers envSlice := os.Environ() @@ -98,11 +86,11 @@ func runShell(cmd *cobra.Command, args []string) error { envSlice = append(envSlice, fmt.Sprintf("%s=%s", k, v)) } envSlice = append(envSlice, "PHASE_SHELL=true") - if len(envNames) > 0 { - envSlice = append(envSlice, fmt.Sprintf("PHASE_ENV=%s", envNames[0])) + if envLabel != "" { + envSlice = append(envSlice, fmt.Sprintf("PHASE_ENV=%s", envLabel)) } - if len(appNames) > 0 { - envSlice = append(envSlice, fmt.Sprintf("PHASE_APP=%s", appNames[0])) + if appLabel != "" { + envSlice = append(envSlice, fmt.Sprintf("PHASE_APP=%s", appLabel)) } if os.Getenv("TERM") == "" { envSlice = append(envSlice, "TERM=xterm-256color") @@ -128,15 +116,18 @@ func runShell(cmd *cobra.Command, args []string) error { fmt.Fprintf(os.Stderr, "🐚 Initialized %s with %s secrets from Application: %s, Environment: %s, Path: %s\n", util.BoldGreenErr(shellName), util.BoldMagentaErr(fmt.Sprintf("%d", secretCount)), - util.BoldCyanErr(strings.Join(appNames, ", ")), - util.BoldGreenErr(strings.Join(envNames, ", ")), + util.BoldCyanErr(appLabel), + util.BoldGreenErr(envLabel), util.BoldYellowErr(path)) } else { fmt.Fprintf(os.Stderr, "🐚 Initialized %s with %s secrets from Application: %s, Environment: %s\n", util.BoldGreenErr(shellName), util.BoldMagentaErr(fmt.Sprintf("%d", secretCount)), - util.BoldCyanErr(strings.Join(appNames, ", ")), - util.BoldGreenErr(strings.Join(envNames, ", "))) + util.BoldCyanErr(appLabel), + util.BoldGreenErr(envLabel)) + } + if secretCount == 0 { + fmt.Fprint(os.Stderr, emptyPathHint(path, "load")) } fmt.Fprintf(os.Stderr, "%s Secrets are only available in this session. Type %s or press %s to exit.\n", util.BoldYellowErr("Remember:"), diff --git a/src/pkg/display/tree.go b/src/pkg/display/tree.go index 84f5c357..4fac1593 100644 --- a/src/pkg/display/tree.go +++ b/src/pkg/display/tree.go @@ -199,18 +199,24 @@ func renderSecretRow(pathPrefix string, s sdk.SecretResult, show bool, keyWidth, } } -// RenderSecretsTree renders secrets in a tree view with path hierarchy -func RenderSecretsTree(secrets []sdk.SecretResult, show bool) { +// RenderSecretsTree renders secrets in a tree view with path hierarchy. +// appName and envName are the resolved context the secrets were fetched for; +// they are rendered even when nothing matched, so an empty result is clearly a +// filter miss rather than an unresolved app or environment. path is the path +// filter that was applied, or "" if none was. +func RenderSecretsTree(secrets []sdk.SecretResult, show bool, appName, envName, path string) { + bold, cyan, green, magenta, reset := util.AnsiCodes() + if len(secrets) == 0 { - fmt.Println("No secrets to display.") + fmt.Printf(" %s No secrets found for Application: %s%s%s%s, Environment: %s%s%s%s\n", + "🔮", bold, cyan, appName, reset, bold, green, envName, reset) + if path != "" { + fmt.Printf(" 💡 Nothing at path %s%s%s. Secrets under other paths are not listed — pass %s--path \"\"%s to list secrets from all paths.\n", + bold, path, reset, bold, reset) + } return } - appName := secrets[0].Application - envName := secrets[0].Environment - - bold, cyan, green, magenta, reset := util.AnsiCodes() - fmt.Printf(" %s Secrets for Application: %s%s%s%s, Environment: %s%s%s%s\n", "🔮", bold, cyan, appName, reset, bold, green, envName, reset) diff --git a/src/pkg/display/tree_test.go b/src/pkg/display/tree_test.go new file mode 100644 index 00000000..92b9bd60 --- /dev/null +++ b/src/pkg/display/tree_test.go @@ -0,0 +1,82 @@ +package display + +import ( + "io" + "os" + "strings" + "testing" + + sdk "github.com/phasehq/golang-sdk/v2/phase" +) + +// captureStdout runs fn and returns everything it wrote to stdout. +func captureStdout(t *testing.T, fn func()) string { + t.Helper() + + r, w, err := os.Pipe() + if err != nil { + t.Fatalf("create pipe: %v", err) + } + orig := os.Stdout + os.Stdout = w + defer func() { os.Stdout = orig }() + + fn() + + if err := w.Close(); err != nil { + t.Fatalf("close pipe: %v", err) + } + out, err := io.ReadAll(r) + if err != nil { + t.Fatalf("read pipe: %v", err) + } + return string(out) +} + +// An empty result set must still name the app and environment that was queried, +// otherwise a path filter that matches nothing reads as a broken .phase.json. +func TestRenderSecretsTreeEmptyShowsContext(t *testing.T) { + out := captureStdout(t, func() { + RenderSecretsTree(nil, false, "my-app", "Development", "/") + }) + + for _, want := range []string{"my-app", "Development", `--path ""`} { + if !strings.Contains(out, want) { + t.Fatalf("expected output to contain %q, got:\n%s", want, out) + } + } +} + +// Without a path filter every path was searched, so suggesting --path "" would +// be a dead end. +func TestRenderSecretsTreeEmptyWithoutPathFilterHasNoHint(t *testing.T) { + out := captureStdout(t, func() { + RenderSecretsTree(nil, false, "my-app", "Production", "") + }) + + if !strings.Contains(out, "my-app") || !strings.Contains(out, "Production") { + t.Fatalf("expected output to name the app and environment, got:\n%s", out) + } + if strings.Contains(out, `--path ""`) { + t.Fatalf("did not expect a path hint when no path filter was applied, got:\n%s", out) + } +} + +// The header comes from the resolved context, not from the first row. +func TestRenderSecretsTreeHeaderUsesResolvedContext(t *testing.T) { + secrets := []sdk.SecretResult{{Key: "SECRET_1", Value: "v", Path: "/one"}} + + out := captureStdout(t, func() { + RenderSecretsTree(secrets, false, "my-app", "Development", "/one") + }) + + if !strings.Contains(out, "Application: my-app") { + t.Fatalf("expected resolved application in header, got:\n%s", out) + } + if !strings.Contains(out, "Environment: Development") { + t.Fatalf("expected resolved environment in header, got:\n%s", out) + } + if !strings.Contains(out, "SECRET_1") { + t.Fatalf("expected the secret row to render, got:\n%s", out) + } +} diff --git a/src/pkg/phase/phase.go b/src/pkg/phase/phase.go index b7be89af..633ed6d3 100644 --- a/src/pkg/phase/phase.go +++ b/src/pkg/phase/phase.go @@ -124,6 +124,22 @@ func PhaseGetContext(userData *misc.AppKeyResponse, appName, envName, appID stri return misc.PhaseGetContext(userData, appName, envName, appID) } +// ResolveNames resolves the canonical application and environment names for the +// given selectors. Display code needs these even when a fetch returns no +// secrets, since there is no result row to read the names off in that case. +// Falls back to the supplied selectors if the lookup fails. +func ResolveNames(p *sdk.Phase, appName, envName, appID string) (string, string) { + userData, err := Init(p) + if err != nil { + return appName, envName + } + app, _, env, _, _, err := misc.PhaseGetContext(userData, appName, envName, appID) + if err != nil { + return appName, envName + } + return app, env +} + // GetConfig fills in appName/envName/appID from .phase.json when not provided via flags. func GetConfig(appName, envName, appID string) (string, string, string) { if appID == "" && appName == "" { From ff4526ad2442a7c1d6df40a8298605105491fdb0 Mon Sep 17 00:00:00 2001 From: rohan Date: Fri, 7 Aug 2026 14:15:56 +0530 Subject: [PATCH 2/2] fix: resolve names from offline cache, make empty hints tag-aware - ResolveNames reads the SDK's userdata.json cache before the network, never hits the network in offline mode, and falls back to the app ID so labels are never blank - empty-result hints name a --tags filter when one was applied - keyed secrets export miss names the searched path - httptest coverage for the name resolution success paths --- src/cmd/context.go | 30 +++++++-- src/cmd/context_names_test.go | 37 +++++++++-- src/cmd/run.go | 2 +- src/cmd/secrets_export.go | 14 +++- src/cmd/secrets_list.go | 2 +- src/cmd/shell.go | 2 +- src/pkg/display/tree.go | 12 ++-- src/pkg/display/tree_test.go | 16 ++--- src/pkg/phase/phase.go | 55 ++++++++++++++-- src/pkg/phase/phase_test.go | 118 ++++++++++++++++++++++++++++++++++ 10 files changed, 250 insertions(+), 38 deletions(-) create mode 100644 src/pkg/phase/phase_test.go diff --git a/src/cmd/context.go b/src/cmd/context.go index 92ee55d7..519474f3 100644 --- a/src/cmd/context.go +++ b/src/cmd/context.go @@ -41,13 +41,29 @@ func contextNames(p *sdk.Phase, secrets []sdk.SecretResult, appName, envName, ap return phase.ResolveNames(p, appName, envName, appID) } -// emptyPathHint explains why a path filter matched nothing. Secrets are filtered -// by exact path, so secrets kept in folders are invisible at the default '/'. -// Returns "" when no path filter was applied, since every path was searched. -func emptyPathHint(path, verb string) string { - if path == "" { +// emptyResultHint explains why a fetch matched nothing. Secrets are filtered by +// exact path, so secrets kept in folders are invisible at the default '/'; a +// --tags filter can also empty the result, in which case suggesting a broader +// path alone would misdirect. Returns "" when neither filter was applied — +// the environment is then genuinely empty and there is nothing to suggest. +// Set forStdout when the hint is printed to stdout so ANSI styling is gated on +// the right stream. +func emptyResultHint(path, tags, verb string, forStdout bool) string { + bold, yellow := util.BoldErr, util.BoldYellowErr + if forStdout { + bold, yellow = util.Bold, util.BoldYellow + } + switch { + case tags != "" && path != "": + return fmt.Sprintf("💡 No secrets matched tag filter %s at path %s. Adjust %s, or pass %s to %s secrets from all paths.\n", + yellow(tags), yellow(path), bold("--tags"), bold(`--path ""`), verb) + case tags != "": + return fmt.Sprintf("💡 No secrets matched tag filter %s. Adjust or drop %s to %s all secrets.\n", + yellow(tags), bold("--tags"), verb) + case path != "": + return fmt.Sprintf("💡 No secrets found at path %s. Secrets under other paths are not included — pass %s to %s secrets from all paths.\n", + yellow(path), bold(`--path ""`), verb) + default: return "" } - return fmt.Sprintf("💡 No secrets found at path %s. Secrets under other paths are not included — pass %s to %s secrets from all paths.\n", - util.BoldYellowErr(path), util.BoldErr(`--path ""`), verb) } diff --git a/src/cmd/context_names_test.go b/src/cmd/context_names_test.go index fa47a284..e3a4f89d 100644 --- a/src/cmd/context_names_test.go +++ b/src/cmd/context_names_test.go @@ -27,25 +27,48 @@ func TestContextNamesIgnoresBlankNamesOnRows(t *testing.T) { secrets := []sdk.SecretResult{{Key: "A"}} // With no usable names on the rows, contextNames falls back to a lookup. - // The zero-value client has no host, so the lookup fails at request time + // PHASE_SERVICE_TOKEN disables the on-disk user-data cache (see + // getCacheDir), so a developer's real cache can't leak into the test, and + // the zero-value client has no host, so the lookup fails at request time // without any network I/O and the selectors are returned as-is. + t.Setenv("PHASE_SERVICE_TOKEN", "test") + t.Setenv("PHASE_OFFLINE", "") app, env := contextNames(&sdk.Phase{}, secrets, "my-app", "Development", "") if app != "my-app" || env != "Development" { t.Fatalf("unexpected fallback: got %q/%q want %q/%q", app, env, "my-app", "Development") } } -func TestEmptyPathHint(t *testing.T) { - // No path filter means every path was already searched — nothing to suggest. - if got := emptyPathHint("", "inject"); got != "" { - t.Fatalf("expected no hint for an empty path filter, got %q", got) +func TestEmptyResultHint(t *testing.T) { + // No filters means the environment is genuinely empty — nothing to suggest. + if got := emptyResultHint("", "", "inject", false); got != "" { + t.Fatalf("expected no hint without filters, got %q", got) } - hint := emptyPathHint("/", "inject") + hint := emptyResultHint("/", "", "inject", false) if !strings.Contains(hint, `--path ""`) { - t.Fatalf("hint should suggest --path \"\", got %q", hint) + t.Fatalf("path hint should suggest --path \"\", got %q", hint) } if !strings.Contains(hint, "inject") { t.Fatalf("hint should use the caller's verb, got %q", hint) } + + // A tag filter is a likelier cause than the path, so it must be named; the + // path suggestion alone would misdirect. + hint = emptyResultHint("/", "backend", "inject", false) + if !strings.Contains(hint, "backend") || !strings.Contains(hint, "--tags") { + t.Fatalf("tag+path hint should name the tag filter, got %q", hint) + } + if !strings.Contains(hint, `--path ""`) { + t.Fatalf("tag+path hint should still mention the path filter, got %q", hint) + } + + // Tags with no path filter: the path suggestion would be a dead end. + hint = emptyResultHint("", "backend", "list", false) + if !strings.Contains(hint, "--tags") { + t.Fatalf("tag hint should name the tag filter, got %q", hint) + } + if strings.Contains(hint, `--path ""`) { + t.Fatalf("tag-only hint should not suggest a path change, got %q", hint) + } } diff --git a/src/cmd/run.go b/src/cmd/run.go index 4ff45b96..683ddf3b 100644 --- a/src/cmd/run.go +++ b/src/cmd/run.go @@ -100,7 +100,7 @@ func runRun(cmd *cobra.Command, args []string) error { util.BoldGreenErr(envLabel)) } if secretCount == 0 { - fmt.Fprint(os.Stderr, emptyPathHint(path, "inject")) + fmt.Fprint(os.Stderr, emptyResultHint(path, tags, "inject", false)) } // Build environment: inherit current env and append secrets diff --git a/src/cmd/secrets_export.go b/src/cmd/secrets_export.go index 8bb19b75..f0dddae6 100644 --- a/src/cmd/secrets_export.go +++ b/src/cmd/secrets_export.go @@ -91,7 +91,17 @@ func runSecretsExport(cmd *cobra.Command, args []string) error { } } if len(missingKeys) > 0 { - return fmt.Errorf("🥡 failed to export — the following secret(s) do not exist: %s", strings.Join(missingKeys, ", ")) + missing := strings.Join(missingKeys, ", ") + switch { + case tags != "" && path != "": + return fmt.Errorf("🥡 failed to export — the following secret(s) were not found at path %s with tag filter %s: %s. Adjust --tags, or pass --path \"\" to export from all paths", path, tags, missing) + case tags != "": + return fmt.Errorf("🥡 failed to export — the following secret(s) did not match tag filter %s: %s. Adjust or drop --tags", tags, missing) + case path != "": + return fmt.Errorf("🥡 failed to export — the following secret(s) were not found at path %s: %s. Secrets under other paths are not searched — pass --path \"\" to export from all paths", path, missing) + default: + return fmt.Errorf("🥡 failed to export — the following secret(s) do not exist: %s", missing) + } } // Export only the requested keys (in the order they were specified) for _, key := range filterKeys { @@ -115,7 +125,7 @@ func runSecretsExport(cmd *cobra.Command, args []string) error { // it can't be mistaken for a broken app or environment. stderr keeps the // exported document on stdout intact for piping. if len(secretsList) == 0 { - fmt.Fprint(os.Stderr, emptyPathHint(path, "export")) + fmt.Fprint(os.Stderr, emptyResultHint(path, tags, "export", false)) } switch format { diff --git a/src/cmd/secrets_list.go b/src/cmd/secrets_list.go index 58ae143b..52c7f1a2 100644 --- a/src/cmd/secrets_list.go +++ b/src/cmd/secrets_list.go @@ -65,7 +65,7 @@ func listSecrets(p *sdk.Phase, envName, appName, appID, tags, path string, show, } appLabel, envLabel := contextNames(p, secrets, appName, envName, appID) - display.RenderSecretsTree(secrets, show, appLabel, envLabel, path) + display.RenderSecretsTree(secrets, show, appLabel, envLabel, emptyResultHint(path, tags, "list", true)) return nil } diff --git a/src/cmd/shell.go b/src/cmd/shell.go index 5333cbfd..3693d804 100644 --- a/src/cmd/shell.go +++ b/src/cmd/shell.go @@ -127,7 +127,7 @@ func runShell(cmd *cobra.Command, args []string) error { util.BoldGreenErr(envLabel)) } if secretCount == 0 { - fmt.Fprint(os.Stderr, emptyPathHint(path, "load")) + fmt.Fprint(os.Stderr, emptyResultHint(path, tags, "load", false)) } fmt.Fprintf(os.Stderr, "%s Secrets are only available in this session. Type %s or press %s to exit.\n", util.BoldYellowErr("Remember:"), diff --git a/src/pkg/display/tree.go b/src/pkg/display/tree.go index 4fac1593..f07ff2f3 100644 --- a/src/pkg/display/tree.go +++ b/src/pkg/display/tree.go @@ -202,17 +202,17 @@ func renderSecretRow(pathPrefix string, s sdk.SecretResult, show bool, keyWidth, // RenderSecretsTree renders secrets in a tree view with path hierarchy. // appName and envName are the resolved context the secrets were fetched for; // they are rendered even when nothing matched, so an empty result is clearly a -// filter miss rather than an unresolved app or environment. path is the path -// filter that was applied, or "" if none was. -func RenderSecretsTree(secrets []sdk.SecretResult, show bool, appName, envName, path string) { +// filter miss rather than an unresolved app or environment. emptyHint is an +// optional newline-terminated line explaining why the result is empty, printed +// only when there are no secrets; pass "" for none. +func RenderSecretsTree(secrets []sdk.SecretResult, show bool, appName, envName, emptyHint string) { bold, cyan, green, magenta, reset := util.AnsiCodes() if len(secrets) == 0 { fmt.Printf(" %s No secrets found for Application: %s%s%s%s, Environment: %s%s%s%s\n", "🔮", bold, cyan, appName, reset, bold, green, envName, reset) - if path != "" { - fmt.Printf(" 💡 Nothing at path %s%s%s. Secrets under other paths are not listed — pass %s--path \"\"%s to list secrets from all paths.\n", - bold, path, reset, bold, reset) + if emptyHint != "" { + fmt.Printf(" %s", emptyHint) } return } diff --git a/src/pkg/display/tree_test.go b/src/pkg/display/tree_test.go index 92b9bd60..bb66d61f 100644 --- a/src/pkg/display/tree_test.go +++ b/src/pkg/display/tree_test.go @@ -35,21 +35,21 @@ func captureStdout(t *testing.T, fn func()) string { // An empty result set must still name the app and environment that was queried, // otherwise a path filter that matches nothing reads as a broken .phase.json. +// The caller-supplied hint line renders below the context. func TestRenderSecretsTreeEmptyShowsContext(t *testing.T) { out := captureStdout(t, func() { - RenderSecretsTree(nil, false, "my-app", "Development", "/") + RenderSecretsTree(nil, false, "my-app", "Development", "💡 No secrets found at path /.\n") }) - for _, want := range []string{"my-app", "Development", `--path ""`} { + for _, want := range []string{"my-app", "Development", "No secrets found at path /"} { if !strings.Contains(out, want) { t.Fatalf("expected output to contain %q, got:\n%s", want, out) } } } -// Without a path filter every path was searched, so suggesting --path "" would -// be a dead end. -func TestRenderSecretsTreeEmptyWithoutPathFilterHasNoHint(t *testing.T) { +// With no hint supplied (no filters were applied), only the context renders. +func TestRenderSecretsTreeEmptyWithoutHint(t *testing.T) { out := captureStdout(t, func() { RenderSecretsTree(nil, false, "my-app", "Production", "") }) @@ -57,8 +57,8 @@ func TestRenderSecretsTreeEmptyWithoutPathFilterHasNoHint(t *testing.T) { if !strings.Contains(out, "my-app") || !strings.Contains(out, "Production") { t.Fatalf("expected output to name the app and environment, got:\n%s", out) } - if strings.Contains(out, `--path ""`) { - t.Fatalf("did not expect a path hint when no path filter was applied, got:\n%s", out) + if strings.Contains(out, "💡") { + t.Fatalf("did not expect a hint line when none was supplied, got:\n%s", out) } } @@ -67,7 +67,7 @@ func TestRenderSecretsTreeHeaderUsesResolvedContext(t *testing.T) { secrets := []sdk.SecretResult{{Key: "SECRET_1", Value: "v", Path: "/one"}} out := captureStdout(t, func() { - RenderSecretsTree(secrets, false, "my-app", "Development", "/one") + RenderSecretsTree(secrets, false, "my-app", "Development", "") }) if !strings.Contains(out, "Application: my-app") { diff --git a/src/pkg/phase/phase.go b/src/pkg/phase/phase.go index 633ed6d3..7f357999 100644 --- a/src/pkg/phase/phase.go +++ b/src/pkg/phase/phase.go @@ -4,6 +4,7 @@ import ( "encoding/json" "fmt" "os" + "path/filepath" "runtime" "strings" @@ -127,19 +128,63 @@ func PhaseGetContext(userData *misc.AppKeyResponse, appName, envName, appID stri // ResolveNames resolves the canonical application and environment names for the // given selectors. Display code needs these even when a fetch returns no // secrets, since there is no result row to read the names off in that case. -// Falls back to the supplied selectors if the lookup fails. +// +// The SDK refreshes its on-disk user-data cache on every successful online +// fetch, so by the time display code runs the cache is normally fresh and no +// extra request is needed. The network is consulted only when no cache is +// available, and never in offline mode. If nothing resolves, falls back to the +// supplied selectors, with the app ID standing in for a missing app name. func ResolveNames(p *sdk.Phase, appName, envName, appID string) (string, string) { - userData, err := Init(p) - if err != nil { - return appName, envName + return resolveNames(p, appName, envName, appID, getCacheDir()) +} + +func resolveNames(p *sdk.Phase, appName, envName, appID, cacheDir string) (string, string) { + userData := readCachedUserData(cacheDir) + if userData == nil { + if offline.IsOffline() { + return fallbackNames(appName, envName, appID) + } + var err error + userData, err = Init(p) + if err != nil { + return fallbackNames(appName, envName, appID) + } } app, _, env, _, _, err := misc.PhaseGetContext(userData, appName, envName, appID) if err != nil { - return appName, envName + return fallbackNames(appName, envName, appID) } return app, env } +// readCachedUserData reads the AppKeyResponse the SDK caches at +// {cacheDir}/userdata.json on every successful online fetch. Returns nil when +// the cache is absent or unreadable. +func readCachedUserData(cacheDir string) *misc.AppKeyResponse { + if cacheDir == "" { + return nil + } + data, err := os.ReadFile(filepath.Join(cacheDir, "userdata.json")) + if err != nil { + return nil + } + var userData misc.AppKeyResponse + if err := json.Unmarshal(data, &userData); err != nil { + return nil + } + return &userData +} + +// fallbackNames is the degraded label set when no lookup is possible: the raw +// selectors, with the app ID standing in for a missing app name so callers +// never render a blank Application label. +func fallbackNames(appName, envName, appID string) (string, string) { + if appName == "" { + appName = appID + } + return appName, envName +} + // GetConfig fills in appName/envName/appID from .phase.json when not provided via flags. func GetConfig(appName, envName, appID string) (string, string, string) { if appID == "" && appName == "" { diff --git a/src/pkg/phase/phase_test.go b/src/pkg/phase/phase_test.go new file mode 100644 index 00000000..3c94f028 --- /dev/null +++ b/src/pkg/phase/phase_test.go @@ -0,0 +1,118 @@ +package phase + +import ( + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "sync/atomic" + "testing" + + sdk "github.com/phasehq/golang-sdk/v2/phase" +) + +// userDataJSON is a minimal AppKeyResponse as served by +// GET {host}/service/secrets/tokens/ and cached at {cacheDir}/userdata.json. +const userDataJSON = `{ + "apps": [ + { + "id": "8e977d18-3fdc-45a5-91a9-3e6a9e5b3f11", + "name": "my-app", + "environment_keys": [ + {"environment": {"id": "env-1", "name": "Development"}}, + {"environment": {"id": "env-2", "name": "Production"}} + ] + } + ] +}` + +func userDataServer(t *testing.T) *httptest.Server { + t.Helper() + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/service/secrets/tokens/" { + http.NotFound(w, r) + return + } + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(userDataJSON)) + })) + t.Cleanup(srv.Close) + return srv +} + +// The core promise of the empty-fetch label fix: partial, case-insensitive +// selectors resolve to the canonical names from the account's app list. +func TestResolveNamesResolvesViaNetworkWhenNoCache(t *testing.T) { + t.Setenv("PHASE_OFFLINE", "") + srv := userDataServer(t) + p := &sdk.Phase{Host: srv.URL} + + app, env := resolveNames(p, "my", "dev", "", "") + if app != "my-app" || env != "Development" { + t.Fatalf("selector resolution: got %q/%q want %q/%q", app, env, "my-app", "Development") + } + + app, env = resolveNames(p, "", "prod", "8e977d18-3fdc-45a5-91a9-3e6a9e5b3f11", "") + if app != "my-app" || env != "Production" { + t.Fatalf("app-id resolution: got %q/%q want %q/%q", app, env, "my-app", "Production") + } +} + +// The SDK refreshes {cacheDir}/userdata.json on every successful online fetch, +// so by the time labels are resolved the cache satisfies the lookup without a +// second request. The reachable server must stay untouched — a network-first +// implementation would reintroduce the duplicate user-data fetch. +func TestResolveNamesPrefersCache(t *testing.T) { + t.Setenv("PHASE_OFFLINE", "") + dir := t.TempDir() + if err := os.WriteFile(filepath.Join(dir, "userdata.json"), []byte(userDataJSON), 0600); err != nil { + t.Fatalf("write cache: %v", err) + } + var contacted atomic.Bool + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + contacted.Store(true) + http.NotFound(w, r) + })) + t.Cleanup(srv.Close) + + app, env := resolveNames(&sdk.Phase{Host: srv.URL}, "my", "dev", "", dir) + if contacted.Load() { + t.Fatal("cache was populated — the network must not be contacted") + } + if app != "my-app" || env != "Development" { + t.Fatalf("cache resolution: got %q/%q want %q/%q", app, env, "my-app", "Development") + } +} + +// Offline mode must never touch the network: with no cache available the +// labels degrade to the selectors, with the app ID standing in for a missing +// app name so nothing renders blank. +func TestResolveNamesOfflineSkipsNetwork(t *testing.T) { + t.Setenv("PHASE_OFFLINE", "1") + var contacted atomic.Bool + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + contacted.Store(true) + })) + t.Cleanup(srv.Close) + + app, env := resolveNames(&sdk.Phase{Host: srv.URL}, "", "Development", "8e977d18-3fdc-45a5-91a9-3e6a9e5b3f11", "") + if contacted.Load() { + t.Fatal("offline mode must not make network requests") + } + if app != "8e977d18-3fdc-45a5-91a9-3e6a9e5b3f11" || env != "Development" { + t.Fatalf("offline fallback: got %q/%q, want app ID and selector env", app, env) + } +} + +// When the lookup fails online (unreachable host) the app ID substitutes for a +// missing app name — the label must never be blank in the .phase.json flow, +// where only the app ID is known. +func TestResolveNamesFallsBackToAppIDOnLookupFailure(t *testing.T) { + t.Setenv("PHASE_OFFLINE", "") + + // The zero-value client has no host, so the request fails before any I/O. + app, env := resolveNames(&sdk.Phase{}, "", "Development", "8e977d18-3fdc-45a5-91a9-3e6a9e5b3f11", "") + if app != "8e977d18-3fdc-45a5-91a9-3e6a9e5b3f11" || env != "Development" { + t.Fatalf("failure fallback: got %q/%q, want app ID and selector env", app, env) + } +}