diff --git a/internal/openshell/sdkclient/auth.go b/internal/openshell/sdkclient/auth.go new file mode 100644 index 0000000..2c302b3 --- /dev/null +++ b/internal/openshell/sdkclient/auth.go @@ -0,0 +1,75 @@ +package sdkclient + +import ( + "fmt" + "path/filepath" + + gw "github.com/NVIDIA/OpenShell/sdk/go/openshell/v1/gateway" + "github.com/NVIDIA/OpenShell/sdk/go/openshell/v1/types" + + "github.com/stackrox/harness-openshell/internal/openshell" +) + +// EnvLookup is an injectable environment variable lookup function. +// Production code passes os.Getenv; tests inject a closure. +type EnvLookup func(string) string + +// connBranch represents the auth mode branch decision for connection setup. +type connBranch int + +const ( + branchDefault connBranch = iota // none/plaintext/cloudflare_jwt/oidc-human → gateway.NewClient(name) + branchMTLS // WithAuth(NoAuth()) + WithTLS(certs derived from cfg.Dir) + branchSAOIDC // oidc + OPENSHELL_OIDC_CLIENT_SECRET present +) + +// connPlan holds the connection setup parameters resolved from auth mode and environment. +type connPlan struct { + name string + address string + mode gw.AuthMode + branch connBranch + tls *types.TLSConfig // set ONLY for branchMTLS; nil otherwise +} + +// planConnection determines which auth branch and TLS configuration to use. +// It is pure: no disk access, no network I/O, and no secret material in its +// output or error messages. Environment lookups are injected for testability. +func planConnection(cfg *gw.Config, env EnvLookup) (connPlan, error) { + plan := connPlan{ + name: cfg.Name, + address: cfg.Endpoint, + mode: cfg.AuthMode, + } + + switch cfg.AuthMode { + case gw.AuthModeMTLS: + plan.branch = branchMTLS + mtlsDir := filepath.Join(cfg.Dir, "mtls") + plan.tls = &types.TLSConfig{ + CertFile: filepath.Join(mtlsDir, "tls.crt"), + KeyFile: filepath.Join(mtlsDir, "tls.key"), + CAFile: filepath.Join(mtlsDir, "ca.crt"), + } + + case gw.AuthModeNone, gw.AuthModePlaintext, gw.AuthModeCloudflareJWT: + plan.branch = branchDefault + plan.tls = nil + + case gw.AuthModeOIDC: + secret := env("OPENSHELL_OIDC_CLIENT_SECRET") + if secret == "" { + plan.branch = branchDefault + plan.tls = nil + } else { + // TODO(PR8): audience/scopes unverified — needs OIDC gateway + plan.branch = branchSAOIDC + plan.tls = nil + } + + default: + return connPlan{}, fmt.Errorf("%w: unsupported auth mode %q", openshell.ErrConfig, cfg.AuthMode) + } + + return plan, nil +} diff --git a/internal/openshell/sdkclient/auth_test.go b/internal/openshell/sdkclient/auth_test.go new file mode 100644 index 0000000..b8074ff --- /dev/null +++ b/internal/openshell/sdkclient/auth_test.go @@ -0,0 +1,225 @@ +package sdkclient + +import ( + "errors" + "fmt" + "path/filepath" + "strings" + "testing" + + gw "github.com/NVIDIA/OpenShell/sdk/go/openshell/v1/gateway" + + "github.com/stackrox/harness-openshell/internal/openshell" +) + +func TestPlanConnection(t *testing.T) { + tests := []struct { + name string + cfg *gw.Config + env EnvLookup + expectBranch connBranch + expectMode gw.AuthMode + expectTLS bool // true if plan.tls is non-nil, false if nil + expectError bool + }{ + { + name: "mtls auth mode", + cfg: &gw.Config{ + Name: "test-gateway", + Endpoint: "localhost:9876", + AuthMode: gw.AuthModeMTLS, + Dir: "/fake/gwdir", + }, + env: func(string) string { return "" }, + expectBranch: branchMTLS, + expectMode: gw.AuthModeMTLS, + expectTLS: true, + expectError: false, + }, + { + name: "none auth mode", + cfg: &gw.Config{ + Name: "test-gateway", + Endpoint: "localhost:9876", + AuthMode: gw.AuthModeNone, + Dir: "/fake/gwdir", + }, + env: func(string) string { return "" }, + expectBranch: branchDefault, + expectMode: gw.AuthModeNone, + expectTLS: false, + expectError: false, + }, + { + name: "plaintext auth mode", + cfg: &gw.Config{ + Name: "test-gateway", + Endpoint: "localhost:9876", + AuthMode: gw.AuthModePlaintext, + Dir: "/fake/gwdir", + }, + env: func(string) string { return "" }, + expectBranch: branchDefault, + expectMode: gw.AuthModePlaintext, + expectTLS: false, + expectError: false, + }, + { + name: "cloudflare_jwt auth mode", + cfg: &gw.Config{ + Name: "test-gateway", + Endpoint: "localhost:9876", + AuthMode: gw.AuthModeCloudflareJWT, + Dir: "/fake/gwdir", + }, + env: func(string) string { return "" }, + expectBranch: branchDefault, + expectMode: gw.AuthModeCloudflareJWT, + expectTLS: false, + expectError: false, + }, + { + name: "oidc auth mode without secret", + cfg: &gw.Config{ + Name: "test-gateway", + Endpoint: "localhost:9876", + AuthMode: gw.AuthModeOIDC, + Dir: "/fake/gwdir", + }, + env: func(string) string { return "" }, + expectBranch: branchDefault, + expectMode: gw.AuthModeOIDC, + expectTLS: false, + expectError: false, + }, + { + name: "oidc auth mode with secret", + cfg: &gw.Config{ + Name: "test-gateway", + Endpoint: "localhost:9876", + AuthMode: gw.AuthModeOIDC, + Dir: "/fake/gwdir", + }, + env: func(key string) string { + if key == "OPENSHELL_OIDC_CLIENT_SECRET" { + return "SUPER-SECRET-abc123" + } + return "" + }, + expectBranch: branchSAOIDC, + expectMode: gw.AuthModeOIDC, + expectTLS: false, + expectError: false, + }, + { + name: "unsupported auth mode", + cfg: &gw.Config{ + Name: "test-gateway", + Endpoint: "localhost:9876", + AuthMode: gw.AuthMode("bogus"), + Dir: "/fake/gwdir", + }, + env: func(string) string { return "" }, + expectError: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + plan, err := planConnection(tt.cfg, tt.env) + + if tt.expectError { + if err == nil { + t.Errorf("expected error, got nil") + } + if !errors.Is(err, openshell.ErrConfig) { + t.Errorf("expected error to wrap openshell.ErrConfig, got: %v", err) + } + if plan != (connPlan{}) { + t.Errorf("expected zero connPlan on error, got: %+v", plan) + } + return + } + + if err != nil { + t.Errorf("unexpected error: %v", err) + } + + // Verify name, address, and mode are always set. + if plan.name != tt.cfg.Name { + t.Errorf("expected name %q, got %q", tt.cfg.Name, plan.name) + } + if plan.address != tt.cfg.Endpoint { + t.Errorf("expected address %q, got %q", tt.cfg.Endpoint, plan.address) + } + if plan.mode != tt.expectMode { + t.Errorf("expected mode %q, got %q", tt.expectMode, plan.mode) + } + + // Verify branch. + if plan.branch != tt.expectBranch { + t.Errorf("expected branch %v, got %v", tt.expectBranch, plan.branch) + } + + // Verify TLS config. + if tt.expectTLS { + if plan.tls == nil { + t.Errorf("expected non-nil TLS config, got nil") + } else { + expectedCertFile := filepath.Join(tt.cfg.Dir, "mtls", "tls.crt") + if plan.tls.CertFile != expectedCertFile { + t.Errorf("expected CertFile %q, got %q", expectedCertFile, plan.tls.CertFile) + } + expectedKeyFile := filepath.Join(tt.cfg.Dir, "mtls", "tls.key") + if plan.tls.KeyFile != expectedKeyFile { + t.Errorf("expected KeyFile %q, got %q", expectedKeyFile, plan.tls.KeyFile) + } + expectedCAFile := filepath.Join(tt.cfg.Dir, "mtls", "ca.crt") + if plan.tls.CAFile != expectedCAFile { + t.Errorf("expected CAFile %q, got %q", expectedCAFile, plan.tls.CAFile) + } + } + } else { + if plan.tls != nil { + t.Errorf("expected nil TLS config, got: %+v", plan.tls) + } + } + }) + } +} + +func TestPlanConnectionSecretNonLeak(t *testing.T) { + // Verify that the secret value does not appear in the plan output. + secret := "SUPER-SECRET-abc123" + cfg := &gw.Config{ + Name: "test-gateway", + Endpoint: "localhost:9876", + AuthMode: gw.AuthModeOIDC, + Dir: "/fake/gwdir", + } + env := func(key string) string { + if key == "OPENSHELL_OIDC_CLIENT_SECRET" { + return secret + } + return "" + } + + plan, err := planConnection(cfg, env) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + // The key assertion: the secret must NOT appear in the plan's stringified + // form. planConnection never stores it, so branch selection is the only + // observable effect of the secret's presence. + planStr := fmt.Sprintf("%+v", plan) + for _, substr := range []string{secret, "SUPER-SECRET", "abc123"} { + if strings.Contains(planStr, substr) { + t.Errorf("secret material %q leaked into plan string %q", substr, planStr) + } + } + + if plan.branch != branchSAOIDC { + t.Errorf("expected branchSAOIDC, got %v", plan.branch) + } +} diff --git a/internal/openshell/sdkclient/client.go b/internal/openshell/sdkclient/client.go index e5f5e67..4ee47a0 100644 --- a/internal/openshell/sdkclient/client.go +++ b/internal/openshell/sdkclient/client.go @@ -2,19 +2,19 @@ // OpenShell Go SDK. It translates between the harness-owned internal/openshell // vocabulary and the SDK, keeping every SDK type behind the firewall. // -// Slice S1 scope: construct a client for an mTLS gateway (the auth mode all our -// managed gateways use) and report Health. The full auth-mode resolver -// (planConnection) lands in S2; provider mapping and error translation in S3. +// Construction routes through planConnection (auth.go), the pure resolver that +// decides the dial branch and derives mTLS cert paths. Only the mTLS branch is +// wired to a live dial today; the remaining branches, provider mapping, and +// error translation land in S3. package sdkclient import ( "context" "fmt" - "path/filepath" + "os" v1 "github.com/NVIDIA/OpenShell/sdk/go/openshell/v1" "github.com/NVIDIA/OpenShell/sdk/go/openshell/v1/gateway" - "github.com/NVIDIA/OpenShell/sdk/go/openshell/v1/types" "github.com/stackrox/harness-openshell/internal/openshell" ) @@ -39,11 +39,10 @@ type client struct { // New constructs an openshell.Client for the given target. // -// S1 handles only mTLS gateways: it loads the CLI-managed gateway config, points -// TLS at the CLI-managed client certificate under /mtls, and dials via -// the gateway.NewClient escape hatch (an explicit WithAuth skips the SDK's -// "mtls not supported" resolver; WithTLS supplies the client cert). Other auth -// modes return ErrConfig until S2 generalizes construction. +// It loads the CLI-managed gateway config and delegates the dial decision to +// planConnection. Only the mTLS branch is dialed today (the auth mode all our +// managed gateways use); the other branches return ErrConfig until S3 wires +// their dial paths. func New(ctx context.Context, t openshell.Target) (openshell.Client, error) { cfg, err := gateway.LoadConfig(t.Gateway) if err != nil { @@ -55,20 +54,20 @@ func New(ctx context.Context, t openshell.Target) (openshell.Client, error) { ws = defaultWorkspace } - if cfg.AuthMode != gateway.AuthModeMTLS { - return nil, fmt.Errorf("%w: auth mode %q not yet supported (S1 handles mtls only)", openshell.ErrConfig, cfg.AuthMode) + plan, err := planConnection(cfg, os.Getenv) + if err != nil { + return nil, err } - mtlsDir := filepath.Join(cfg.Dir, "mtls") - tls := &types.TLSConfig{ - CertFile: filepath.Join(mtlsDir, "tls.crt"), - KeyFile: filepath.Join(mtlsDir, "tls.key"), - CAFile: filepath.Join(mtlsDir, "ca.crt"), + // S2 wires only the mTLS branch (the auth mode all our managed gateways + // use). The remaining branches gain their dial paths in S3. + if plan.branch != branchMTLS { + return nil, fmt.Errorf("%w: auth mode %q not yet supported (mtls only until S3)", openshell.ErrConfig, plan.mode) } - raw, err := gateway.NewClient(t.Gateway, gateway.WithAuth(v1.NoAuth()), gateway.WithTLS(tls)) + raw, err := gateway.NewClient(plan.name, gateway.WithAuth(v1.NoAuth()), gateway.WithTLS(plan.tls)) if err != nil { - return nil, fmt.Errorf("%w: dial gateway %q: %v", openshell.ErrConfig, t.Gateway, err) + return nil, fmt.Errorf("%w: dial gateway %q: %v", openshell.ErrConfig, plan.name, err) } return &client{raw: raw, workspace: ws}, nil