From 8f3ed63f39720321f9724643bcfa4cc0a315dc0f Mon Sep 17 00:00:00 2001 From: stjiao14 Date: Mon, 25 May 2026 15:41:13 -0700 Subject: [PATCH 1/2] Route SplitProvider List by prefix --- internal/storage/split_provider.go | 11 +++++------ internal/storage/split_provider_test.go | 18 +++++++++++++----- 2 files changed, 18 insertions(+), 11 deletions(-) diff --git a/internal/storage/split_provider.go b/internal/storage/split_provider.go index 3aeeacb..29d101e 100644 --- a/internal/storage/split_provider.go +++ b/internal/storage/split_provider.go @@ -76,13 +76,12 @@ func (s *SplitProvider) Exists(ctx context.Context, key string) (bool, error) { return s.route(key).Exists(ctx, key) } -// List always uses Data. The only paths a bearer peer enumerates are -// compartment prefixes (rclone scanning a vol). The control-plane -// keys are individual objects accessed by name; nothing lists them. -// If a future use case needs Control-side List, route by the prefix -// argument here (left as a TODO when that lands). +// List routes by prefix the same way single-key operations route by +// key. This keeps behavior consistent for any control-plane list use +// case (for example listing under .drift/peers/) while preserving the +// existing compartment-data behavior on Data. func (s *SplitProvider) List(ctx context.Context, prefix string) ([]string, error) { - return s.Data.List(ctx, prefix) + return s.route(prefix).List(ctx, prefix) } func (s *SplitProvider) GetWithETag(ctx context.Context, key string) ([]byte, string, error) { diff --git a/internal/storage/split_provider_test.go b/internal/storage/split_provider_test.go index 29326a8..dc61c77 100644 --- a/internal/storage/split_provider_test.go +++ b/internal/storage/split_provider_test.go @@ -154,23 +154,31 @@ func TestSplitProvider_writeOnControlPath_routesToControl(t *testing.T) { } } -// TestSplitProvider_listAlwaysData: List is only used for compartment -// enumeration in current drift code; always routes to Data. -func TestSplitProvider_listAlwaysData(t *testing.T) { +// TestSplitProvider_listRoutesByPrefix: List should route by prefix the +// same way key-based operations route by key. +func TestSplitProvider_listRoutesByPrefix(t *testing.T) { ctx := context.Background() data := newLabeled("DATA") control := newLabeled("CTRL") sp := NewSplitProvider(data, control) _ = data.Put(ctx, "compartments/main/a", []byte("a")) - _ = control.Put(ctx, domain.ManifestKey, []byte("m")) + _ = control.Put(ctx, domain.PeersDir+"dev1/refresh.enc", []byte("r")) got, err := sp.List(ctx, "compartments/") if err != nil { t.Fatal(err) } if len(got) != 1 || got[0] != "compartments/main/a" { - t.Errorf("List should have returned compartment-data only from Data, got: %v", got) + t.Errorf("List(compartments/) should route to Data, got: %v", got) + } + + got, err = sp.List(ctx, domain.PeersDir) + if err != nil { + t.Fatal(err) + } + if len(got) != 1 || got[0] != domain.PeersDir+"dev1/refresh.enc" { + t.Errorf("List(%q) should route to Control, got: %v", domain.PeersDir, got) } } From f78e5726f4c857ddb9c6360f32c3c922f14ff385 Mon Sep 17 00:00:00 2001 From: stjiao14 Date: Mon, 25 May 2026 18:26:24 -0700 Subject: [PATCH 2/2] harden credential update flow and token TTL limits --- internal/cli/config.go | 46 ++++++++++++++++-------------------- internal/cli/grant.go | 2 +- internal/cli/parent.go | 16 +++++++------ internal/token/issue.go | 9 ++++++- internal/token/token_test.go | 1 + 5 files changed, 39 insertions(+), 35 deletions(-) diff --git a/internal/cli/config.go b/internal/cli/config.go index 80f3e91..9f3f436 100644 --- a/internal/cli/config.go +++ b/internal/cli/config.go @@ -1,13 +1,12 @@ package cli import ( - "errors" + "context" "fmt" "os" "github.com/spf13/cobra" - "github.com/sufforest/drift/internal/credentials" "github.com/sufforest/drift/internal/workspace" ) @@ -26,53 +25,48 @@ func configSetParentCmd() *cobra.Command { Short: "Deprecated alias for `drift parent set` (kept for script compatibility)", Hidden: true, Long: `Deprecated: prefer 'drift parent set'. This command remains for -scripts that pre-date the 'drift parent' namespace but does NOT run the -live HEAD verification step. New code should call 'drift parent set'.`, +scripts that pre-date the 'drift parent' namespace and now reuses the +same verified credential-replacement flow as 'drift parent set'.`, RunE: runConfigSetParent, } cmd.Flags().String("access-key", "", "New access key ID (defaults to $DRIFT_ACCESS_KEY_ID)") - cmd.Flags().String("secret-key", "", "New secret access key (defaults to $DRIFT_SECRET_ACCESS_KEY)") return cmd } func runConfigSetParent(cmd *cobra.Command, _ []string) error { - dir, err := stateDir(cmd) - if err != nil { - return err + ctx := cmd.Context() + if ctx == nil { + ctx = context.Background() } - state, err := workspace.NewState(dir) + ws, err := loadWorkspace(ctx, cmd) if err != nil { return err } - existing, err := state.LoadParent() - if err != nil { - return fmt.Errorf("load existing parent cred: %w", err) - } ak, _ := cmd.Flags().GetString("access-key") if ak == "" { ak = os.Getenv("DRIFT_ACCESS_KEY_ID") } - sk, _ := cmd.Flags().GetString("secret-key") + sk := os.Getenv("DRIFT_SECRET_ACCESS_KEY") if sk == "" { - sk = os.Getenv("DRIFT_SECRET_ACCESS_KEY") - } - if ak == "" || sk == "" { - return errors.New("set-parent: missing access-key or secret-key (pass flags or export DRIFT_ACCESS_KEY_ID + DRIFT_SECRET_ACCESS_KEY)") + prompted, perr := promptPassphrase("New secret access key (input hidden): ") + if perr != nil { + return perr + } + sk = prompted } - - updated := &credentials.Parent{ - Provider: existing.Provider, + res, err := ws.ParentSet(ctx, workspace.ParentSetOptions{ AccessKeyID: ak, SecretAccessKey: sk, - } - if err := state.SaveParent(updated); err != nil { - return fmt.Errorf("save parent: %w", err) + SkipVerify: false, + }) + if err != nil { + return err } fmt.Fprintf(cmd.OutOrStdout(), "✓ Parent credential updated.\n"+ - " Provider: %s (unchanged)\n"+ + " Provider: %s\n"+ " Access key: %s…\n", - updated.Provider, abbrev(ak, 6)) + res.Provider, abbrev(ak, 6)) return nil } diff --git a/internal/cli/grant.go b/internal/cli/grant.go index f72cf31..80d3b23 100644 --- a/internal/cli/grant.go +++ b/internal/cli/grant.go @@ -35,7 +35,7 @@ ssh config (no flags are forwarded — use ~/.ssh/config for identity/port).`, } cmd.Flags().StringSlice("scope", nil, "Vols to grant (comma-separated)") cmd.Flags().String("mode", "rw", "Access mode: rw or ro") - cmd.Flags().Duration("expires", 24*time.Hour, "Token TTL (≤24h recommended)") + cmd.Flags().Duration("expires", 24*time.Hour, "Token TTL (max 24h)") cmd.Flags().Bool("token-only", false, "Print ONLY the token on stdout (status to stderr)") cmd.Flags().String("out", "", "Write token to this file at chmod 0600 instead of stdout") cmd.Flags().String("ssh", "", "Deliver via SSH: runs `drift open --stdin --background` on user@host") diff --git a/internal/cli/parent.go b/internal/cli/parent.go index e7381cd..8b37451 100644 --- a/internal/cli/parent.go +++ b/internal/cli/parent.go @@ -33,9 +33,8 @@ func parentSetCmd() *cobra.Command { cmd := &cobra.Command{ Use: "set", Short: "Replace the parent S3 credential with a new AK/SK pair", - Long: `Reads the new AK/SK from --access-key + $DRIFT_SECRET_ACCESS_KEY (or ---secret-key, though passing secrets as CLI flags is visible in process -listings — env var is recommended) and overwrites the locally-stored + Long: `Reads the new AK/SK from --access-key + $DRIFT_SECRET_ACCESS_KEY (or an +interactive no-echo prompt if env var is unset) and overwrites the locally-stored parent credential. By default, the new credential is verified by issuing a HEAD against @@ -50,7 +49,6 @@ running set themselves.`, RunE: runParentSet, } cmd.Flags().String("access-key", "", "New access key ID (or $DRIFT_ACCESS_KEY_ID)") - cmd.Flags().String("secret-key", "", "New secret access key (PREFER $DRIFT_SECRET_ACCESS_KEY to keep secrets out of argv)") cmd.Flags().String("provider", "", "Override the provider id (default: keep existing — typically 'r2')") cmd.Flags().Bool("skip-verify", false, "Skip the live HEAD probe (dangerous: a wrong secret will silently brick this device's R2 access)") return cmd @@ -67,12 +65,16 @@ func runParentSet(cmd *cobra.Command, _ []string) error { if ak == "" { ak = os.Getenv("DRIFT_ACCESS_KEY_ID") } - sk, _ := cmd.Flags().GetString("secret-key") + sk := os.Getenv("DRIFT_SECRET_ACCESS_KEY") if sk == "" { - sk = os.Getenv("DRIFT_SECRET_ACCESS_KEY") + prompted, perr := promptPassphrase("New secret access key (input hidden): ") + if perr != nil { + return perr + } + sk = prompted } if ak == "" || sk == "" { - return errors.New("drift parent set: missing access-key or secret-key (pass flags or export DRIFT_ACCESS_KEY_ID + DRIFT_SECRET_ACCESS_KEY)") + return errors.New("drift parent set: missing access-key or secret-key (pass --access-key / DRIFT_ACCESS_KEY_ID and DRIFT_SECRET_ACCESS_KEY or use interactive secret prompt)") } provider, _ := cmd.Flags().GetString("provider") skipVerify, _ := cmd.Flags().GetBool("skip-verify") diff --git a/internal/token/issue.go b/internal/token/issue.go index 7c10532..eab9408 100644 --- a/internal/token/issue.go +++ b/internal/token/issue.go @@ -56,9 +56,13 @@ type IssueRequest struct { Scope []string // compartment names to grant Mode string // TokenModeRW / TokenModeRO - TTL time.Duration // <= 24h recommended + TTL time.Duration // max TokenMaxTTL } +// TokenMaxTTL caps issued bearer-token lifetime. Short-lived creds reduce +// exposure if a token string leaks from chat/log/history. +const TokenMaxTTL = 24 * time.Hour + // IssueResult is what Issue returns to the caller. Both scoped creds are // included so the CLI / workspace layer can show their expiry, etc. type IssueResult struct { @@ -220,6 +224,9 @@ func validateIssue(req IssueRequest) error { if req.TTL <= 0 { return errors.New("token: TTL must be > 0") } + if req.TTL > TokenMaxTTL { + return fmt.Errorf("token: TTL %s exceeds max %s", req.TTL, TokenMaxTTL) + } for _, name := range req.Scope { if err := domain.ValidCompartmentName(name); err != nil { return err diff --git a/internal/token/token_test.go b/internal/token/token_test.go index b99e439..6329c75 100644 --- a/internal/token/token_test.go +++ b/internal/token/token_test.go @@ -225,6 +225,7 @@ func TestIssue_validationFailures(t *testing.T) { "empty cprk": func(r *IssueRequest) { r.CPRK = nil }, "empty wid": func(r *IssueRequest) { r.WorkspaceID = "" }, "zero ttl": func(r *IssueRequest) { r.TTL = 0 }, + "ttl too long": func(r *IssueRequest) { r.TTL = TokenMaxTTL + time.Second }, "bad mode": func(r *IssueRequest) { r.Mode = "execute" }, } for name, mutate := range cases {