diff --git a/cli/internal/cli/devnet.go b/cli/internal/cli/devnet.go index 4e5856a..750d1e0 100644 --- a/cli/internal/cli/devnet.go +++ b/cli/internal/cli/devnet.go @@ -37,7 +37,10 @@ func newDevnetCommand(commandContext *commandContext) *cobra.Command { if err := rejectExplicitTarget(commandContext); err != nil { return err } - timeout := commandContext.viper.GetDuration("timeout") + timeout, err := cmd.Flags().GetDuration("timeout") + if err != nil { + return err + } if timeout <= 0 { return fmt.Errorf("--timeout must be greater than 0") } @@ -52,7 +55,10 @@ func newDevnetCommand(commandContext *commandContext) *cobra.Command { return fmt.Errorf("load default devnet environment: %w", err) } - bare := commandContext.viper.GetBool("bare") + bare, err := cmd.Flags().GetBool("bare") + if err != nil { + return err + } result, err := manager.Up(cmd.Context(), lifecycle.UpOptions{ Bare: bare, Env: *environment, @@ -87,7 +93,10 @@ func newDevnetDownCommand(commandContext *commandContext) *cobra.Command { if err := rejectExplicitTarget(commandContext); err != nil { return err } - timeout := commandContext.viper.GetDuration("timeout") + timeout, err := cmd.Flags().GetDuration("timeout") + if err != nil { + return err + } if timeout <= 0 { return fmt.Errorf("--timeout must be greater than 0") } diff --git a/cli/internal/cli/down.go b/cli/internal/cli/down.go index c4c9682..756e239 100644 --- a/cli/internal/cli/down.go +++ b/cli/internal/cli/down.go @@ -23,8 +23,14 @@ func newDownCommand(commandContext *commandContext) *cobra.Command { if err != nil { return err } - timeout := commandContext.viper.GetDuration("timeout") - waitGone := commandContext.viper.GetBool("wait") + timeout, err := cmd.Flags().GetDuration("timeout") + if err != nil { + return err + } + waitGone, err := cmd.Flags().GetBool("wait") + if err != nil { + return err + } if waitGone && timeout <= 0 { return fmt.Errorf("--timeout must be greater than 0 when --wait is set") } diff --git a/cli/internal/cli/env_collisions_test.go b/cli/internal/cli/env_collisions_test.go new file mode 100644 index 0000000..630b299 --- /dev/null +++ b/cli/internal/cli/env_collisions_test.go @@ -0,0 +1,83 @@ +package cli + +import ( + "strings" + "testing" + "time" + + "github.com/spf13/cobra" + "github.com/spf13/viper" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestChainOverridesFromFlagsIgnoresEnv(t *testing.T) { + // An ambient YACD_OGMIOS_URL / YACD_KUPO_URL must not shadow the funding + // flags: reading them off cmd.Flags() (not viper) is the fix. + t.Setenv("YACD_OGMIOS_URL", "ws://evil:1") + t.Setenv("YACD_KUPO_URL", "http://evil:2") + + cmd := &cobra.Command{} + addChainOverrideFlags(cmd) + require.NoError(t, cmd.ParseFlags(nil)) + + overrides, err := chainOverridesFromFlags(cmd) + require.NoError(t, err) + assert.Empty(t, overrides.OgmiosURL, "YACD_OGMIOS_URL must not shadow --ogmios-url") + assert.Empty(t, overrides.KupoURL, "YACD_KUPO_URL must not shadow --kupo-url") +} + +func TestChainOverridesFromFlagsReadsFlags(t *testing.T) { + t.Parallel() + + cmd := &cobra.Command{} + addChainOverrideFlags(cmd) + require.NoError(t, cmd.ParseFlags([]string{"--ogmios-url", "ws://host:1", "--kupo-url", "http://host:2"})) + + overrides, err := chainOverridesFromFlags(cmd) + require.NoError(t, err) + assert.Equal(t, "ws://host:1", overrides.OgmiosURL) + assert.Equal(t, "http://host:2", overrides.KupoURL) +} + +// TestTimeoutFlagIgnoresEnvBleed demonstrates the cross-command bleed bug and +// its fix at the mechanism level: viper (the old read path) is shadowed by +// YACD_TIMEOUT, while cmd.Flags() (the new read path) is immune. +func TestTimeoutFlagIgnoresEnvBleed(t *testing.T) { + t.Setenv("YACD_TIMEOUT", "1s") + + cmd := &cobra.Command{} + cmd.Flags().Duration("timeout", 12*time.Minute, "") + require.NoError(t, cmd.ParseFlags(nil)) + + vp := viper.New() + vp.SetEnvPrefix("YACD") + vp.AutomaticEnv() + require.NoError(t, vp.BindPFlags(cmd.Flags())) + + assert.Equal(t, time.Second, vp.GetDuration("timeout"), + "the old viper read path is shadowed by YACD_TIMEOUT (the bug)") + + got, err := cmd.Flags().GetDuration("timeout") + require.NoError(t, err) + assert.Equal(t, 12*time.Minute, got, "cmd.Flags() is immune to YACD_TIMEOUT (the fix)") +} + +// TestHostEnvExcludesUXKeys guards repo invariant D: the CLI-injected child env +// (run.go appends connected.env to os.Environ) is built from scratch and carries +// only the documented host-access keys, never the flag-only UX keys — even when +// those are exported in the process environment. +func TestHostEnvExcludesUXKeys(t *testing.T) { + t.Setenv("YACD_OUTPUT", "json") + t.Setenv("YACD_VERBOSE", "3") + t.Setenv("YACD_QUIET", "true") + + env := hostEnvFromURLs(readyNetwork("devnet"), "ws://127.0.0.1:40001", "http://127.0.0.1:40002") + + for _, entry := range env { + for _, banned := range []string{"YACD_OUTPUT=", "YACD_VERBOSE=", "YACD_QUIET=", "YACD_NON_INTERACTIVE=", "YACD_COLOR="} { + assert.False(t, strings.HasPrefix(entry, banned), + "CLI-injected env must not carry the flag-only UX key %q", banned) + } + } +} diff --git a/cli/internal/cli/install.go b/cli/internal/cli/install.go index ce01691..fef44d6 100644 --- a/cli/internal/cli/install.go +++ b/cli/internal/cli/install.go @@ -52,9 +52,18 @@ func newInstallCommand(commandContext *commandContext) *cobra.Command { RunE: func(cmd *cobra.Command, _ []string) error { runtimeConfig := commandContext.runtimeConfig - waitReady := commandContext.viper.GetBool("wait") - timeout := commandContext.viper.GetDuration("timeout") - dryRun := commandContext.viper.GetBool("dry-run") + waitReady, err := cmd.Flags().GetBool("wait") + if err != nil { + return err + } + timeout, err := cmd.Flags().GetDuration("timeout") + if err != nil { + return err + } + dryRun, err := cmd.Flags().GetBool("dry-run") + if err != nil { + return err + } // Read the value-override flags straight off the command rather than // through viper: viper's StringArray binding mangles the Helm --set comma diff --git a/cli/internal/cli/up.go b/cli/internal/cli/up.go index 72633f7..f6a5ab5 100644 --- a/cli/internal/cli/up.go +++ b/cli/internal/cli/up.go @@ -35,10 +35,19 @@ func newUpCommand(commandContext *commandContext) *cobra.Command { if file == "" { return fmt.Errorf("--file is required") } - timeout := commandContext.viper.GetDuration("timeout") - dryRun := commandContext.viper.GetBool("dry-run") + timeout, err := cmd.Flags().GetDuration("timeout") + if err != nil { + return err + } + dryRun, err := cmd.Flags().GetBool("dry-run") + if err != nil { + return err + } allowMainnet := commandContext.viper.GetBool("allow-mainnet") - waitReady := commandContext.viper.GetBool("wait") + waitReady, err := cmd.Flags().GetBool("wait") + if err != nil { + return err + } if waitReady && timeout <= 0 { return fmt.Errorf("--timeout must be greater than 0 when --wait is set") } diff --git a/cli/internal/cli/wallet.go b/cli/internal/cli/wallet.go index c93d76c..136d7be 100644 --- a/cli/internal/cli/wallet.go +++ b/cli/internal/cli/wallet.go @@ -15,7 +15,6 @@ import ( walletstore "github.com/meigma/yacd/cli/internal/wallet" domainwallet "github.com/meigma/yacd/internal/cardano/wallet" "github.com/spf13/cobra" - "github.com/spf13/viper" ) // addChainOverrideFlags registers the funding endpoint overrides shared by the @@ -28,13 +27,23 @@ func addChainOverrideFlags(cmd *cobra.Command) { cmd.Flags().String("kupo-url", "", "Kupo URL to use for funding (default: the network's reachable endpoint)") } -// chainOverridesFromViper reads the funding endpoint overrides bound from the -// --ogmios-url / --kupo-url flags. -func chainOverridesFromViper(v *viper.Viper) chainOverrides { - return chainOverrides{ - OgmiosURL: strings.TrimSpace(v.GetString("ogmios-url")), - KupoURL: strings.TrimSpace(v.GetString("kupo-url")), +// chainOverridesFromFlags reads the funding endpoint overrides directly off the +// command flags. Reading them from the flags rather than viper keeps an ambient +// YACD_OGMIOS_URL / YACD_KUPO_URL from shadowing the --ogmios-url / --kupo-url +// the user passed (or deliberately left unset). +func chainOverridesFromFlags(cmd *cobra.Command) (chainOverrides, error) { + ogmiosURL, err := cmd.Flags().GetString("ogmios-url") + if err != nil { + return chainOverrides{}, err + } + kupoURL, err := cmd.Flags().GetString("kupo-url") + if err != nil { + return chainOverrides{}, err } + return chainOverrides{ + OgmiosURL: strings.TrimSpace(ogmiosURL), + KupoURL: strings.TrimSpace(kupoURL), + }, nil } // newWalletCommand wires the `yacd wallet` subtree: the developer-wallet verbs @@ -185,7 +194,10 @@ func newWalletAddCommand(commandContext *commandContext) *cobra.Command { result := walletAddResult{Name: created.Name, Address: created.Address} if topupRequested { - overrides := chainOverridesFromViper(commandContext.viper) + overrides, err := chainOverridesFromFlags(cmd) + if err != nil { + return err + } funded, err := commandContext.fundWallet(cmd.Context(), walletCtx.kubeClient, walletCtx.store, walletCtx.network, walletCtx.namespace, walletCtx.name, fundRequest{ destinationAddress: created.Address, lovelace: lovelace, @@ -248,7 +260,10 @@ func newWalletTopUpCommand(commandContext *commandContext) *cobra.Command { return err } - overrides := chainOverridesFromViper(commandContext.viper) + overrides, err := chainOverridesFromFlags(cmd) + if err != nil { + return err + } result, err := commandContext.fundWallet(cmd.Context(), walletCtx.kubeClient, walletCtx.store, walletCtx.network, walletCtx.namespace, walletCtx.name, fundRequest{ destinationAddress: destinationAddress, lovelace: lovelace,