From 012603ae9c909a57a8bf0aa6d0999e5a11103cf4 Mon Sep 17 00:00:00 2001 From: Hai Huang Date: Thu, 25 Jun 2026 12:07:21 -0400 Subject: [PATCH 1/9] feat(authbridge): add placeholder-resolve outbound plugin Resolve OpenShell-style credential placeholders (openshell:resolve:env:) in request headers to their real secret values on the wire, so the agent never holds the real credential. The placeholder rides in Authorization (Bearer) for the OpenShell anthropic provider, which is the header the forward proxy propagates to the upstream. Resolution is via a pluggable Resolver interface (the swap seam for a future gateway-gRPC source backed by OpenShell's GetSandboxProviderEnvironment). Default resolvers: inline mappings (testing), a mounted secret dir, or the process env. Fails closed on an unresolved/unknown placeholder; rejects CR/LF/NUL in resolved values (CWE-113). Purely additive: implements the existing pipeline.Plugin/Configurable contract with no shared-interface change, registered via a build-tagged cmd/authbridge-proxy/plugins_placeholderresolve.go (matching the per-plugin registration scheme so the plugin is droppable via -tags exclude_plugin_placeholderresolve). Verified by unit tests and a forward-proxy isolation run (resolve -> real value on the wire, unknown key -> 401 fail-closed, non-placeholder -> passthrough). Assisted-By: Claude (Anthropic AI) Signed-off-by: Hai Huang --- .../plugins/placeholderresolve/plugin.go | 266 ++++++++++++++++++ .../plugins/placeholderresolve/plugin_test.go | 124 ++++++++ .../plugins_placeholderresolve.go | 5 + 3 files changed, 395 insertions(+) create mode 100644 authbridge/authlib/plugins/placeholderresolve/plugin.go create mode 100644 authbridge/authlib/plugins/placeholderresolve/plugin_test.go create mode 100644 authbridge/cmd/authbridge-proxy/plugins_placeholderresolve.go diff --git a/authbridge/authlib/plugins/placeholderresolve/plugin.go b/authbridge/authlib/plugins/placeholderresolve/plugin.go new file mode 100644 index 000000000..4893e77bd --- /dev/null +++ b/authbridge/authlib/plugins/placeholderresolve/plugin.go @@ -0,0 +1,266 @@ +// Package placeholderresolve provides an outbound pipeline plugin that +// resolves OpenShell-style credential placeholders in request headers to +// their real secret values, on the wire, so the agent never holds the +// real credential. +// +// OpenShell injects an agent's credential env (e.g. ANTHROPIC_AUTH_TOKEN) +// as a placeholder string of the form "openshell:resolve:env:". The +// agent emits that placeholder verbatim (for the `claude` provider it +// rides in `Authorization: Bearer `); this plugin scans the +// configured headers, resolves each placeholder via an injected Resolver, +// and rewrites the header in place. Unresolvable placeholders fail closed +// (the request is denied rather than forwarded with the placeholder). +// +// The Resolver is the swap seam: the default implementations resolve from +// an inline map (isolation testing), a mounted secret directory, or the +// process environment. A future gateway-gRPC resolver (calling OpenShell's +// GetSandboxProviderEnvironment) plugs in here without touching the plugin. +package placeholderresolve + +import ( + "bytes" + "context" + "encoding/json" + "errors" + "fmt" + "os" + "path/filepath" + "regexp" + "strings" + + "github.com/kagenti/kagenti-extensions/authbridge/authlib/config" + "github.com/kagenti/kagenti-extensions/authbridge/authlib/pipeline" + "github.com/kagenti/kagenti-extensions/authbridge/authlib/plugins" +) + +// defaultPrefix is the OpenShell placeholder prefix. Kept identical to the +// supervisor's so agents are unchanged when AuthBridge replaces OpenShell's +// internal proxy (OpenShell secrets.rs PLACEHOLDER_PREFIX). +const defaultPrefix = "openshell:resolve:env:" + +// envKeyPattern is OpenShell's env-key grammar: a leading letter/underscore +// followed by letters, digits, or underscores. It bounds the KEY captured +// after the prefix and (because it admits no '/' or '.') also makes the +// file-source path join safe from traversal. +const envKeyPattern = `([A-Za-z_][A-Za-z0-9_]*)` + +// placeholderResolveConfig is the plugin's local config schema. Field tags +// drive both runtime decoding (json) and operator-facing schema +// introspection (description / default). +type placeholderResolveConfig struct { + // Prefix is the placeholder prefix to match. The KEY following it is + // matched against the env-key grammar. + Prefix string `json:"prefix" description:"Placeholder prefix to match before the env key." default:"openshell:resolve:env:"` + + // Headers lists the request headers to scan and rewrite. Defaults to + // Authorization (the only header the forward proxy propagates to the + // upstream wire today). + Headers []string `json:"headers" description:"Request headers to scan for placeholders." default:"Authorization"` + + // Mappings is an optional inline KEY->value map used as the resolver + // source. Intended for isolation testing; takes precedence over + // secret_dir and env when non-empty. + Mappings map[string]string `json:"mappings" description:"Inline KEY->value resolver source (isolation testing)."` + + // SecretDir, when set (and Mappings empty), resolves each KEY by + // reading / as a credential file. + SecretDir string `json:"secret_dir" description:"Directory to resolve each KEY from a file named ."` +} + +func (c *placeholderResolveConfig) applyDefaults() { + if c.Prefix == "" { + c.Prefix = defaultPrefix + } + if len(c.Headers) == 0 { + c.Headers = []string{"Authorization"} + } +} + +func (c *placeholderResolveConfig) validate() error { + if c.Prefix == "" { + return errors.New("prefix must not be empty") + } + if len(c.Headers) == 0 { + return errors.New("headers must list at least one header") + } + return nil +} + +// Resolver maps a placeholder KEY to its real secret value. ok is false +// when the key is unknown — the plugin fails closed on a false. This is the +// swap seam: the gateway-gRPC source (B2) implements this interface. +type Resolver interface { + Resolve(ctx context.Context, key string) (string, bool) +} + +// mapResolver resolves from an inline map (isolation testing). +type mapResolver map[string]string + +func (m mapResolver) Resolve(_ context.Context, key string) (string, bool) { + v, ok := m[key] + return v, ok +} + +// envResolver resolves from the process environment. An unset or empty var +// is treated as unresolved (fail closed). +type envResolver struct{} + +func (envResolver) Resolve(_ context.Context, key string) (string, bool) { + v, ok := os.LookupEnv(key) + if !ok || v == "" { + return "", false + } + return v, true +} + +// fileResolver resolves each KEY by reading /. The env-key grammar +// forbids '/' and '.' so the join cannot escape dir. +type fileResolver struct{ dir string } + +func (f fileResolver) Resolve(_ context.Context, key string) (string, bool) { + v, err := config.ReadCredentialFile(filepath.Join(f.dir, key)) + if err != nil { + return "", false + } + return v, true +} + +// PlaceholderResolve is the outbound plugin. cfg/re/resolver are immutable +// after Configure returns, so OnRequest reads them without synchronization. +type PlaceholderResolve struct { + cfg placeholderResolveConfig + re *regexp.Regexp + resolver Resolver +} + +// New constructs an unconfigured plugin. +func New() *PlaceholderResolve { return &PlaceholderResolve{} } + +func init() { + plugins.RegisterPlugin("placeholder-resolve", func() pipeline.Plugin { return New() }) +} + +func (p *PlaceholderResolve) Name() string { return "placeholder-resolve" } + +func (p *PlaceholderResolve) Capabilities() pipeline.PluginCapabilities { + return pipeline.PluginCapabilities{ + Description: "Resolve OpenShell credential placeholders in headers to real values (fail-closed).", + } +} + +// ConfigSchema surfaces field metadata to config-aware tooling. +func (p *PlaceholderResolve) ConfigSchema() []pipeline.FieldSchema { + return pipeline.SchemaOf(placeholderResolveConfig{}) +} + +func (p *PlaceholderResolve) Configure(raw json.RawMessage) error { + var c placeholderResolveConfig + if len(raw) > 0 { + dec := json.NewDecoder(bytes.NewReader(raw)) + dec.DisallowUnknownFields() + if err := dec.Decode(&c); err != nil { + return fmt.Errorf("placeholder-resolve config: %w", err) + } + } + c.applyDefaults() + if err := c.validate(); err != nil { + return fmt.Errorf("placeholder-resolve config: %w", err) + } + + re, err := regexp.Compile(regexp.QuoteMeta(c.Prefix) + envKeyPattern) + if err != nil { + return fmt.Errorf("placeholder-resolve: compile prefix pattern: %w", err) + } + + // Pick the resolver source. Mappings wins (deterministic, for tests), + // then a mounted secret dir, else the process environment. The gateway + // gRPC source (B2) will be selected here behind a new config option. + var resolver Resolver + switch { + case len(c.Mappings) > 0: + resolver = mapResolver(c.Mappings) + case c.SecretDir != "": + resolver = fileResolver{dir: c.SecretDir} + default: + resolver = envResolver{} + } + + // Commit only after all fallible construction succeeded, so a failed + // Configure leaves the plugin in its zero state. + p.cfg = c + p.re = re + p.resolver = resolver + return nil +} + +func (p *PlaceholderResolve) OnRequest(ctx context.Context, pctx *pipeline.Context) pipeline.Action { + if p.resolver == nil || p.re == nil { + return pipeline.DenyStatus(503, "upstream.unreachable", "placeholder-resolve not configured") + } + + anyResolved := false + for _, h := range p.cfg.Headers { + val := pctx.Headers.Get(h) + if val == "" { + continue + } + rewritten, found, ok := p.rewrite(ctx, val) + if !found { + continue + } + if !ok { + // A placeholder was present but could not be resolved (unknown + // key or invalid value). Fail closed — never forward the + // placeholder to the upstream. + return pctx.DenyAndRecord("placeholder_unresolved", "auth.unauthorized", "unresolvable credential placeholder") + } + pctx.Headers.Set(h, rewritten) + anyResolved = true + } + + if anyResolved { + pctx.Modify("placeholder_resolved") + } else { + pctx.Skip("no_placeholder") + } + return pipeline.Action{Type: pipeline.Continue} +} + +func (p *PlaceholderResolve) OnResponse(_ context.Context, _ *pipeline.Context) pipeline.Action { + return pipeline.Action{Type: pipeline.Continue} +} + +// rewrite replaces every placeholder in val with its resolved value. +// Returns (rewritten, found, ok): found is true when at least one +// placeholder matched; ok is false when a matched placeholder could not be +// resolved or resolved to an unsafe value (caller must fail closed). +func (p *PlaceholderResolve) rewrite(ctx context.Context, val string) (string, bool, bool) { + found := false + failed := false + out := p.re.ReplaceAllStringFunc(val, func(match string) string { + found = true + key := strings.TrimPrefix(match, p.cfg.Prefix) + v, ok := p.resolver.Resolve(ctx, key) + if !ok || !validResolved(v) { + failed = true + return match // unchanged; the request is denied regardless + } + return v + }) + if failed { + return "", true, false + } + return out, found, true +} + +// validResolved rejects resolved values carrying CR, LF, or NUL — guarding +// against header-injection (CWE-113) via a poisoned credential store. +func validResolved(s string) bool { + return !strings.ContainsAny(s, "\r\n\x00") +} + +// Compile-time interface checks. +var ( + _ pipeline.Plugin = (*PlaceholderResolve)(nil) + _ pipeline.Configurable = (*PlaceholderResolve)(nil) +) diff --git a/authbridge/authlib/plugins/placeholderresolve/plugin_test.go b/authbridge/authlib/plugins/placeholderresolve/plugin_test.go new file mode 100644 index 000000000..740e9a5b6 --- /dev/null +++ b/authbridge/authlib/plugins/placeholderresolve/plugin_test.go @@ -0,0 +1,124 @@ +package placeholderresolve + +import ( + "context" + "encoding/json" + "net/http" + "testing" + + "github.com/kagenti/kagenti-extensions/authbridge/authlib/pipeline" +) + +// configure builds and configures a plugin with the given inline mappings. +func configure(t *testing.T, mappings map[string]string) *PlaceholderResolve { + t.Helper() + raw, err := json.Marshal(map[string]any{"mappings": mappings}) + if err != nil { + t.Fatalf("marshal config: %v", err) + } + p := New() + if err := p.Configure(raw); err != nil { + t.Fatalf("Configure: %v", err) + } + return p +} + +func TestConfigureDefaults(t *testing.T) { + p := configure(t, map[string]string{"FOO": "x"}) + if p.cfg.Prefix != defaultPrefix { + t.Errorf("prefix default = %q, want %q", p.cfg.Prefix, defaultPrefix) + } + if len(p.cfg.Headers) != 1 || p.cfg.Headers[0] != "Authorization" { + t.Errorf("headers default = %v, want [Authorization]", p.cfg.Headers) + } +} + +func TestOnRequest(t *testing.T) { + mappings := map[string]string{ + "ANTHROPIC_AUTH_TOKEN": "sk-real-token", + "BAD": "evil\r\nInjected: 1", + } + + tests := []struct { + name string + authIn string + wantReject bool + wantAuth string // expected Authorization after OnRequest (when not rejected) + }{ + { + name: "resolves placeholder bearer", + authIn: "Bearer openshell:resolve:env:ANTHROPIC_AUTH_TOKEN", + wantAuth: "Bearer sk-real-token", + }, + { + name: "non-placeholder passthrough", + authIn: "Bearer already-a-real-token", + wantAuth: "Bearer already-a-real-token", + }, + { + name: "unknown key fails closed", + authIn: "Bearer openshell:resolve:env:UNKNOWN", + wantReject: true, + }, + { + name: "invalid resolved value fails closed", + authIn: "Bearer openshell:resolve:env:BAD", + wantReject: true, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + p := configure(t, mappings) + pctx := &pipeline.Context{Direction: pipeline.Outbound, Headers: http.Header{}} + pctx.Headers.Set("Authorization", tc.authIn) + + act := p.OnRequest(context.Background(), pctx) + + if tc.wantReject { + if act.Type != pipeline.Reject { + t.Fatalf("expected Reject, got %v", act.Type) + } + if act.Violation == nil || act.Violation.Code != "auth.unauthorized" { + t.Errorf("expected auth.unauthorized violation, got %+v", act.Violation) + } + // The placeholder must never be forwarded — on a reject the + // caller drops the request, so the resolved value is moot, + // but assert we did not leak a real secret either. + if got := pctx.Headers.Get("Authorization"); got == "Bearer sk-real-token" { + t.Errorf("rejected request unexpectedly carries a resolved secret") + } + return + } + + if act.Type != pipeline.Continue { + t.Fatalf("expected Continue, got %v (violation %+v)", act.Type, act.Violation) + } + if got := pctx.Headers.Get("Authorization"); got != tc.wantAuth { + t.Errorf("Authorization = %q, want %q", got, tc.wantAuth) + } + }) + } +} + +func TestRewriteMultiplePlaceholders(t *testing.T) { + p := configure(t, map[string]string{"A": "1", "B": "2"}) + got, found, ok := p.rewrite(context.Background(), + "openshell:resolve:env:A and openshell:resolve:env:B") + if !found || !ok { + t.Fatalf("found=%v ok=%v, want both true", found, ok) + } + if want := "1 and 2"; got != want { + t.Errorf("rewrite = %q, want %q", got, want) + } +} + +func TestUnconfiguredFailsClosed(t *testing.T) { + p := New() // never Configured + pctx := &pipeline.Context{Direction: pipeline.Outbound, Headers: http.Header{}} + pctx.Headers.Set("Authorization", "Bearer openshell:resolve:env:FOO") + act := p.OnRequest(context.Background(), pctx) + if act.Type != pipeline.Reject { + t.Fatalf("unconfigured plugin must reject, got %v", act.Type) + } +} diff --git a/authbridge/cmd/authbridge-proxy/plugins_placeholderresolve.go b/authbridge/cmd/authbridge-proxy/plugins_placeholderresolve.go new file mode 100644 index 000000000..719f1e323 --- /dev/null +++ b/authbridge/cmd/authbridge-proxy/plugins_placeholderresolve.go @@ -0,0 +1,5 @@ +//go:build !exclude_plugin_placeholderresolve + +package main + +import _ "github.com/kagenti/kagenti-extensions/authbridge/authlib/plugins/placeholderresolve" From d49da3ac403df21cba833f94fb246f8d179ff2e8 Mon Sep 17 00:00:00 2001 From: Hai Huang Date: Thu, 25 Jun 2026 15:00:23 -0400 Subject: [PATCH 2/9] feat(authbridge): resolve placeholders from the OpenShell gateway MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a gateway-backed Resolver source to the placeholder-resolve plugin (B2). It fetches the sandbox's resolved provider environment from the OpenShell gateway and serves placeholder lookups from an in-memory cache, so AuthBridge — running as a sidecar in the sandbox pod — injects the live credential using OpenShell's native provider mechanism (no mounted secrets). - authlib/openshell: generated gRPC stubs (buf + vendored protos) and a Client that replicates the supervisor's auth chain: read the projected SA token, IssueSandboxToken (Bearer SA token) -> gateway JWT, GetSandboxProviderEnvironment(sandbox_id) (Bearer JWT) -> resolved env. Re-mints the JWT on Unauthenticated. - authlib/plugins/placeholderresolve/gateway: a caching Resolver that primes and refreshes the env map in the background and serves Resolve from cache (lock-free); honors per-key credential expiry; fails closed until primed. - placeholderresolve: a `gateway` config block selects the gateway source (precedence over mappings/secret_dir/env); the plugin gains Init/Ready/Shutdown to drive the resolver's warm-up (mirrors token-exchange's lifecycle). Structural interfaces avoid an import cycle. Verified offline against an in-process mock gateway: client metadata flow + re-mint, and a plugin end-to-end (placeholder -> real value), all under -race. Live in-cluster auth is deferred to the sidecar-deployment phase (the gateway restricts these RPCs to the sandbox's own identity). Assisted-By: Claude (Anthropic AI) Signed-off-by: Hai Huang --- authbridge/authlib/go.mod | 2 +- authbridge/authlib/openshell/buf.gen.yaml | 21 ++ authbridge/authlib/openshell/buf.yaml | 3 + authbridge/authlib/openshell/client.go | 258 +++++++++++++++ authbridge/authlib/openshell/client_test.go | 148 +++++++++ .../genproto/openshellv1/openshell.pb.go | 313 ++++++++++++++++++ .../genproto/openshellv1/openshell_grpc.pb.go | 180 ++++++++++ .../authlib/openshell/proto/openshell.proto | 48 +++ .../placeholderresolve/gateway/resolver.go | 162 +++++++++ .../placeholderresolve/gateway_e2e_test.go | 89 +++++ .../plugins/placeholderresolve/plugin.go | 117 ++++++- 11 files changed, 1325 insertions(+), 16 deletions(-) create mode 100644 authbridge/authlib/openshell/buf.gen.yaml create mode 100644 authbridge/authlib/openshell/buf.yaml create mode 100644 authbridge/authlib/openshell/client.go create mode 100644 authbridge/authlib/openshell/client_test.go create mode 100644 authbridge/authlib/openshell/genproto/openshellv1/openshell.pb.go create mode 100644 authbridge/authlib/openshell/genproto/openshellv1/openshell_grpc.pb.go create mode 100644 authbridge/authlib/openshell/proto/openshell.proto create mode 100644 authbridge/authlib/plugins/placeholderresolve/gateway/resolver.go create mode 100644 authbridge/authlib/plugins/placeholderresolve/gateway_e2e_test.go diff --git a/authbridge/authlib/go.mod b/authbridge/authlib/go.mod index 197612414..66859a89c 100644 --- a/authbridge/authlib/go.mod +++ b/authbridge/authlib/go.mod @@ -13,6 +13,7 @@ require ( golang.org/x/sys v0.46.0 google.golang.org/genproto/googleapis/rpc v0.0.0-20260226221140-a57be14db171 google.golang.org/grpc v1.81.1 + google.golang.org/protobuf v1.36.11 gopkg.in/yaml.v3 v3.0.1 ) @@ -71,7 +72,6 @@ require ( golang.org/x/sync v0.21.0 // indirect golang.org/x/text v0.38.0 // indirect golang.org/x/time v0.12.0 // indirect - google.golang.org/protobuf v1.36.11 // indirect oras.land/oras-go/v2 v2.5.0 // indirect sigs.k8s.io/yaml v1.4.0 // indirect ) diff --git a/authbridge/authlib/openshell/buf.gen.yaml b/authbridge/authlib/openshell/buf.gen.yaml new file mode 100644 index 000000000..b08ab3c64 --- /dev/null +++ b/authbridge/authlib/openshell/buf.gen.yaml @@ -0,0 +1,21 @@ +# Generates Go gRPC stubs from the trimmed OpenShell gateway proto. Only the two +# RPCs the placeholder-resolve gateway resolver calls are vendored (see +# proto/openshell.proto), so a single go_package override is needed. The proto +# declares no go_package, so managed mode injects it; module= output places it +# under genproto/openshellv1. Remote plugins are pinned to match authlib/go.mod. +# +# Regenerate: cd authlib/openshell && buf generate +version: v2 +managed: + enabled: true + override: + - file_option: go_package + path: openshell.proto + value: github.com/kagenti/kagenti-extensions/authbridge/authlib/openshell/genproto/openshellv1 +plugins: + - remote: buf.build/protocolbuffers/go:v1.36.6 + out: genproto + opt: module=github.com/kagenti/kagenti-extensions/authbridge/authlib/openshell/genproto + - remote: buf.build/grpc/go:v1.5.1 + out: genproto + opt: module=github.com/kagenti/kagenti-extensions/authbridge/authlib/openshell/genproto diff --git a/authbridge/authlib/openshell/buf.yaml b/authbridge/authlib/openshell/buf.yaml new file mode 100644 index 000000000..ba5ddf507 --- /dev/null +++ b/authbridge/authlib/openshell/buf.yaml @@ -0,0 +1,3 @@ +version: v2 +modules: + - path: proto diff --git a/authbridge/authlib/openshell/client.go b/authbridge/authlib/openshell/client.go new file mode 100644 index 000000000..4ccf68d7c --- /dev/null +++ b/authbridge/authlib/openshell/client.go @@ -0,0 +1,258 @@ +// Package openshell is a Go client for the OpenShell gateway's credential +// surface. It replicates how the sandbox supervisor authenticates and fetches +// its resolved provider environment, so AuthBridge — running as a sidecar in +// the sandbox pod — can do the same: +// +// 1. read the pod's projected SA token (audience openshell-gateway); +// 2. dial the gateway over mTLS; +// 3. IssueSandboxToken (Bearer ) → a gateway-minted sandbox JWT; +// 4. GetSandboxProviderEnvironment(sandbox_id) (Bearer ) → the resolved +// real credential values keyed by env-var name. +// +// The gateway restricts these RPCs to the sandbox's own identity, so this +// client only succeeds when it runs in the sandbox pod (shared SA + the +// openshell.io/sandbox-id annotation). +package openshell + +import ( + "context" + "crypto/tls" + "crypto/x509" + "fmt" + "net" + "os" + "path/filepath" + "strings" + "sync" + "time" + + "google.golang.org/grpc" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/credentials" + "google.golang.org/grpc/credentials/insecure" + "google.golang.org/grpc/metadata" + "google.golang.org/grpc/status" + + "github.com/kagenti/kagenti-extensions/authbridge/authlib/config" + osv1 "github.com/kagenti/kagenti-extensions/authbridge/authlib/openshell/genproto/openshellv1" +) + +// Config locates the gateway and the credentials needed to talk to it. The +// paths mirror what the OpenShell k8s driver injects into the sandbox pod. +type Config struct { + // Endpoint is the gateway gRPC endpoint (OPENSHELL_ENDPOINT). An + // https:// scheme selects mTLS; http:// is plaintext. + Endpoint string + // MTLSCertDir holds ca.crt + tls.crt + tls.key for mTLS (the + // openshell-client-tls secret mount, e.g. /etc/openshell-tls/client). + // Required when Endpoint is https://. + MTLSCertDir string + // SATokenPath is the projected SA token file (OPENSHELL_K8S_SA_TOKEN_FILE, + // e.g. /var/run/secrets/openshell/token). + SATokenPath string + // SandboxID is this sandbox's id (OPENSHELL_SANDBOX_ID); it must equal + // the id the gateway binds into the minted JWT, or the gateway rejects + // the provider-environment fetch as cross-sandbox access. + SandboxID string +} + +func (c Config) validate() error { + if c.Endpoint == "" { + return fmt.Errorf("openshell: endpoint is required") + } + if c.SATokenPath == "" { + return fmt.Errorf("openshell: sa_token_path is required") + } + if c.SandboxID == "" { + return fmt.Errorf("openshell: sandbox_id is required") + } + if strings.HasPrefix(c.Endpoint, "https://") && c.MTLSCertDir == "" { + return fmt.Errorf("openshell: mtls_cert_dir is required for an https endpoint") + } + return nil +} + +// Environment is the resolved provider environment for a sandbox. +type Environment struct { + // Values maps env-var name (e.g. ANTHROPIC_AUTH_TOKEN) to the real + // resolved credential value. + Values map[string]string + // Revision is a fingerprint of the inputs that produced Values. + Revision uint64 + // ExpiresAtMs is the per-key expiry (epoch ms); absent/0 means no expiry. + ExpiresAtMs map[string]int64 +} + +// Client talks to the OpenShell gateway. It is safe for concurrent use. +type Client struct { + cfg Config + conn *grpc.ClientConn + rpc osv1.OpenShellClient + + mu sync.RWMutex + token string // current gateway JWT + tokenExp time.Time // zero = unknown / non-expiring +} + +// Dial connects to the gateway. It does not contact the server until the +// first RPC, so a successful Dial does not imply reachability. +func Dial(cfg Config) (*Client, error) { + if err := cfg.validate(); err != nil { + return nil, err + } + + target := cfg.Endpoint + var creds credentials.TransportCredentials + switch { + case strings.HasPrefix(cfg.Endpoint, "https://"): + target = strings.TrimPrefix(cfg.Endpoint, "https://") + tlsCfg, err := mtlsConfig(cfg.MTLSCertDir, hostOnly(target)) + if err != nil { + return nil, err + } + creds = credentials.NewTLS(tlsCfg) + case strings.HasPrefix(cfg.Endpoint, "http://"): + target = strings.TrimPrefix(cfg.Endpoint, "http://") + creds = insecure.NewCredentials() + default: + // No scheme: mTLS when a cert dir is configured, else plaintext. + if cfg.MTLSCertDir != "" { + tlsCfg, err := mtlsConfig(cfg.MTLSCertDir, hostOnly(target)) + if err != nil { + return nil, err + } + creds = credentials.NewTLS(tlsCfg) + } else { + creds = insecure.NewCredentials() + } + } + + conn, err := grpc.NewClient(target, grpc.WithTransportCredentials(creds)) + if err != nil { + return nil, fmt.Errorf("openshell: dial %q: %w", target, err) + } + return &Client{cfg: cfg, conn: conn, rpc: osv1.NewOpenShellClient(conn)}, nil +} + +// FetchEnvironment returns the sandbox's resolved provider environment, +// minting (or re-minting) the gateway JWT as needed. +func (c *Client) FetchEnvironment(ctx context.Context) (*Environment, error) { + tok, err := c.ensureToken(ctx) + if err != nil { + return nil, err + } + env, err := c.fetchWithToken(ctx, tok) + if status.Code(err) == codes.Unauthenticated { + // JWT rejected (expired or rotated under us) — re-bootstrap from the + // (possibly rotated) SA token and retry once. + if mErr := c.mint(ctx); mErr != nil { + return nil, mErr + } + tok, _ = c.getToken() + env, err = c.fetchWithToken(ctx, tok) + } + return env, err +} + +// Close releases the underlying connection. +func (c *Client) Close() error { + if c.conn != nil { + return c.conn.Close() + } + return nil +} + +func (c *Client) fetchWithToken(ctx context.Context, tok string) (*Environment, error) { + md := metadata.AppendToOutgoingContext(ctx, "authorization", "Bearer "+tok) + resp, err := c.rpc.GetSandboxProviderEnvironment(md, &osv1.GetSandboxProviderEnvironmentRequest{ + SandboxId: c.cfg.SandboxID, + }) + if err != nil { + return nil, fmt.Errorf("openshell: GetSandboxProviderEnvironment: %w", err) + } + return &Environment{ + Values: resp.GetEnvironment(), + Revision: resp.GetProviderEnvRevision(), + ExpiresAtMs: resp.GetCredentialExpiresAtMs(), + }, nil +} + +// ensureToken returns a non-expired gateway JWT, minting one if absent or +// expired. +func (c *Client) ensureToken(ctx context.Context) (string, error) { + if tok, exp := c.getToken(); tok != "" && (exp.IsZero() || time.Now().Before(exp)) { + return tok, nil + } + if err := c.mint(ctx); err != nil { + return "", err + } + tok, _ := c.getToken() + return tok, nil +} + +// mint exchanges the projected SA token for a gateway JWT via IssueSandboxToken. +func (c *Client) mint(ctx context.Context) error { + sa, err := config.ReadCredentialFile(c.cfg.SATokenPath) + if err != nil { + return fmt.Errorf("openshell: read SA token %q: %w", c.cfg.SATokenPath, err) + } + md := metadata.AppendToOutgoingContext(ctx, "authorization", "Bearer "+sa) + resp, err := c.rpc.IssueSandboxToken(md, &osv1.IssueSandboxTokenRequest{}) + if err != nil { + return fmt.Errorf("openshell: IssueSandboxToken: %w", err) + } + c.setToken(resp.GetToken(), resp.GetExpiresAtMs()) + return nil +} + +func (c *Client) setToken(tok string, expMs int64) { + c.mu.Lock() + defer c.mu.Unlock() + c.token = tok + if expMs > 0 { + c.tokenExp = time.UnixMilli(expMs) + } else { + c.tokenExp = time.Time{} + } +} + +func (c *Client) getToken() (string, time.Time) { + c.mu.RLock() + defer c.mu.RUnlock() + return c.token, c.tokenExp +} + +// mtlsConfig builds a client mTLS config from a cert dir holding ca.crt, +// tls.crt, tls.key. The CA pool extends the system roots (mirrors +// authlib/tlsbridge/upstream.go). +func mtlsConfig(dir, serverName string) (*tls.Config, error) { + cert, err := tls.LoadX509KeyPair(filepath.Join(dir, "tls.crt"), filepath.Join(dir, "tls.key")) + if err != nil { + return nil, fmt.Errorf("openshell: load client cert from %q: %w", dir, err) + } + pool, err := x509.SystemCertPool() + if err != nil || pool == nil { + pool = x509.NewCertPool() + } + caPEM, err := os.ReadFile(filepath.Join(dir, "ca.crt")) + if err != nil { + return nil, fmt.Errorf("openshell: read ca.crt from %q: %w", dir, err) + } + if !pool.AppendCertsFromPEM(caPEM) { + return nil, fmt.Errorf("openshell: ca.crt in %q is not valid PEM", dir) + } + return &tls.Config{ + Certificates: []tls.Certificate{cert}, + RootCAs: pool, + ServerName: serverName, + MinVersion: tls.VersionTLS12, + }, nil +} + +// hostOnly strips a :port suffix, leaving the host for ServerName. +func hostOnly(hostport string) string { + if h, _, err := net.SplitHostPort(hostport); err == nil { + return h + } + return hostport +} diff --git a/authbridge/authlib/openshell/client_test.go b/authbridge/authlib/openshell/client_test.go new file mode 100644 index 000000000..86404e989 --- /dev/null +++ b/authbridge/authlib/openshell/client_test.go @@ -0,0 +1,148 @@ +package openshell + +import ( + "context" + "net" + "os" + "path/filepath" + "strconv" + "strings" + "sync" + "testing" + + "google.golang.org/grpc" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/metadata" + "google.golang.org/grpc/status" + + osv1 "github.com/kagenti/kagenti-extensions/authbridge/authlib/openshell/genproto/openshellv1" +) + +// mockGateway is an in-process OpenShell gateway for client tests. It records +// the bearer credentials it observed so tests can assert the two-step auth +// (SA token -> IssueSandboxToken, then JWT -> GetSandboxProviderEnvironment). +type mockGateway struct { + osv1.UnimplementedOpenShellServer + + mu sync.Mutex + sawSAToken string + sawJWT string + sawSandboxID string + issueCalls int + fetchCalls int + failFirstFetch bool // return Unauthenticated on the first fetch +} + +func bearerOf(ctx context.Context) string { + md, _ := metadata.FromIncomingContext(ctx) + v := md.Get("authorization") + if len(v) == 0 { + return "" + } + return strings.TrimPrefix(v[0], "Bearer ") +} + +func (m *mockGateway) IssueSandboxToken(ctx context.Context, _ *osv1.IssueSandboxTokenRequest) (*osv1.IssueSandboxTokenResponse, error) { + m.mu.Lock() + defer m.mu.Unlock() + m.issueCalls++ + m.sawSAToken = bearerOf(ctx) + return &osv1.IssueSandboxTokenResponse{Token: "jwt-" + strconv.Itoa(m.issueCalls)}, nil +} + +func (m *mockGateway) GetSandboxProviderEnvironment(ctx context.Context, req *osv1.GetSandboxProviderEnvironmentRequest) (*osv1.GetSandboxProviderEnvironmentResponse, error) { + m.mu.Lock() + defer m.mu.Unlock() + m.fetchCalls++ + m.sawJWT = bearerOf(ctx) + m.sawSandboxID = req.GetSandboxId() + if m.failFirstFetch && m.fetchCalls == 1 { + return nil, status.Error(codes.Unauthenticated, "jwt expired") + } + return &osv1.GetSandboxProviderEnvironmentResponse{ + Environment: map[string]string{"ANTHROPIC_AUTH_TOKEN": "sk-real"}, + }, nil +} + +func startMockGateway(t *testing.T, m *mockGateway) string { + t.Helper() + lis, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("listen: %v", err) + } + srv := grpc.NewServer() + osv1.RegisterOpenShellServer(srv, m) + go func() { _ = srv.Serve(lis) }() + t.Cleanup(srv.Stop) + return lis.Addr().String() +} + +func writeSAToken(t *testing.T, value string) string { + t.Helper() + p := filepath.Join(t.TempDir(), "token") + if err := os.WriteFile(p, []byte(value), 0o600); err != nil { + t.Fatalf("write SA token: %v", err) + } + return p +} + +func TestClientFetchEnvironment(t *testing.T) { + m := &mockGateway{} + addr := startMockGateway(t, m) + c, err := Dial(Config{ + Endpoint: "http://" + addr, + SATokenPath: writeSAToken(t, "sa-token-abc"), + SandboxID: "sb-123", + }) + if err != nil { + t.Fatalf("Dial: %v", err) + } + defer func() { _ = c.Close() }() + + env, err := c.FetchEnvironment(context.Background()) + if err != nil { + t.Fatalf("FetchEnvironment: %v", err) + } + if got := env.Values["ANTHROPIC_AUTH_TOKEN"]; got != "sk-real" { + t.Errorf("env[ANTHROPIC_AUTH_TOKEN] = %q, want sk-real", got) + } + + m.mu.Lock() + defer m.mu.Unlock() + if m.sawSAToken != "sa-token-abc" { + t.Errorf("IssueSandboxToken saw bearer %q, want the SA token", m.sawSAToken) + } + if m.sawJWT != "jwt-1" { + t.Errorf("GetSandboxProviderEnvironment saw bearer %q, want the minted jwt-1", m.sawJWT) + } + if m.sawSandboxID != "sb-123" { + t.Errorf("saw sandbox_id %q, want sb-123", m.sawSandboxID) + } +} + +func TestClientReMintOnUnauthenticated(t *testing.T) { + m := &mockGateway{failFirstFetch: true} + addr := startMockGateway(t, m) + c, err := Dial(Config{ + Endpoint: "http://" + addr, + SATokenPath: writeSAToken(t, "sa-token-abc"), + SandboxID: "sb-123", + }) + if err != nil { + t.Fatalf("Dial: %v", err) + } + defer func() { _ = c.Close() }() + + env, err := c.FetchEnvironment(context.Background()) + if err != nil { + t.Fatalf("FetchEnvironment: %v", err) + } + if env.Values["ANTHROPIC_AUTH_TOKEN"] != "sk-real" { + t.Error("expected sk-real after re-mint") + } + m.mu.Lock() + defer m.mu.Unlock() + if m.issueCalls != 2 { + t.Errorf("issueCalls = %d, want 2 (initial mint + re-mint after Unauthenticated)", m.issueCalls) + } +} diff --git a/authbridge/authlib/openshell/genproto/openshellv1/openshell.pb.go b/authbridge/authlib/openshell/genproto/openshellv1/openshell.pb.go new file mode 100644 index 000000000..738cfd866 --- /dev/null +++ b/authbridge/authlib/openshell/genproto/openshellv1/openshell.pb.go @@ -0,0 +1,313 @@ +// SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +// Code generated by protoc-gen-go. DO NOT EDIT. +// versions: +// protoc-gen-go v1.36.6 +// protoc (unknown) +// source: openshell.proto + +package openshellv1 + +import ( + protoreflect "google.golang.org/protobuf/reflect/protoreflect" + protoimpl "google.golang.org/protobuf/runtime/protoimpl" + reflect "reflect" + sync "sync" + unsafe "unsafe" +) + +const ( + // Verify that this generated code is sufficiently up-to-date. + _ = protoimpl.EnforceVersion(20 - protoimpl.MinVersion) + // Verify that runtime/protoimpl is sufficiently up-to-date. + _ = protoimpl.EnforceVersion(protoimpl.MaxVersion - 20) +) + +// IssueSandboxToken request. The caller is identified by its SA-token bearer +// credential, so the request body carries no fields. +type IssueSandboxTokenRequest struct { + state protoimpl.MessageState `protogen:"open.v1"` + unknownFields protoimpl.UnknownFields + sizeCache protoimpl.SizeCache +} + +func (x *IssueSandboxTokenRequest) Reset() { + *x = IssueSandboxTokenRequest{} + mi := &file_openshell_proto_msgTypes[0] + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + ms.StoreMessageInfo(mi) +} + +func (x *IssueSandboxTokenRequest) String() string { + return protoimpl.X.MessageStringOf(x) +} + +func (*IssueSandboxTokenRequest) ProtoMessage() {} + +func (x *IssueSandboxTokenRequest) ProtoReflect() protoreflect.Message { + mi := &file_openshell_proto_msgTypes[0] + if x != nil { + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + if ms.LoadMessageInfo() == nil { + ms.StoreMessageInfo(mi) + } + return ms + } + return mi.MessageOf(x) +} + +// Deprecated: Use IssueSandboxTokenRequest.ProtoReflect.Descriptor instead. +func (*IssueSandboxTokenRequest) Descriptor() ([]byte, []int) { + return file_openshell_proto_rawDescGZIP(), []int{0} +} + +// IssueSandboxToken response. +type IssueSandboxTokenResponse struct { + state protoimpl.MessageState `protogen:"open.v1"` + // Gateway-minted JWT bound to the calling sandbox's UUID. + Token string `protobuf:"bytes,1,opt,name=token,proto3" json:"token,omitempty"` + // Absolute expiry of the issued token, milliseconds since the epoch. 0 means + // the token is non-expiring. + ExpiresAtMs int64 `protobuf:"varint,2,opt,name=expires_at_ms,json=expiresAtMs,proto3" json:"expires_at_ms,omitempty"` + unknownFields protoimpl.UnknownFields + sizeCache protoimpl.SizeCache +} + +func (x *IssueSandboxTokenResponse) Reset() { + *x = IssueSandboxTokenResponse{} + mi := &file_openshell_proto_msgTypes[1] + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + ms.StoreMessageInfo(mi) +} + +func (x *IssueSandboxTokenResponse) String() string { + return protoimpl.X.MessageStringOf(x) +} + +func (*IssueSandboxTokenResponse) ProtoMessage() {} + +func (x *IssueSandboxTokenResponse) ProtoReflect() protoreflect.Message { + mi := &file_openshell_proto_msgTypes[1] + if x != nil { + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + if ms.LoadMessageInfo() == nil { + ms.StoreMessageInfo(mi) + } + return ms + } + return mi.MessageOf(x) +} + +// Deprecated: Use IssueSandboxTokenResponse.ProtoReflect.Descriptor instead. +func (*IssueSandboxTokenResponse) Descriptor() ([]byte, []int) { + return file_openshell_proto_rawDescGZIP(), []int{1} +} + +func (x *IssueSandboxTokenResponse) GetToken() string { + if x != nil { + return x.Token + } + return "" +} + +func (x *IssueSandboxTokenResponse) GetExpiresAtMs() int64 { + if x != nil { + return x.ExpiresAtMs + } + return 0 +} + +type GetSandboxProviderEnvironmentRequest struct { + state protoimpl.MessageState `protogen:"open.v1"` + // The sandbox ID. + SandboxId string `protobuf:"bytes,1,opt,name=sandbox_id,json=sandboxId,proto3" json:"sandbox_id,omitempty"` + unknownFields protoimpl.UnknownFields + sizeCache protoimpl.SizeCache +} + +func (x *GetSandboxProviderEnvironmentRequest) Reset() { + *x = GetSandboxProviderEnvironmentRequest{} + mi := &file_openshell_proto_msgTypes[2] + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + ms.StoreMessageInfo(mi) +} + +func (x *GetSandboxProviderEnvironmentRequest) String() string { + return protoimpl.X.MessageStringOf(x) +} + +func (*GetSandboxProviderEnvironmentRequest) ProtoMessage() {} + +func (x *GetSandboxProviderEnvironmentRequest) ProtoReflect() protoreflect.Message { + mi := &file_openshell_proto_msgTypes[2] + if x != nil { + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + if ms.LoadMessageInfo() == nil { + ms.StoreMessageInfo(mi) + } + return ms + } + return mi.MessageOf(x) +} + +// Deprecated: Use GetSandboxProviderEnvironmentRequest.ProtoReflect.Descriptor instead. +func (*GetSandboxProviderEnvironmentRequest) Descriptor() ([]byte, []int) { + return file_openshell_proto_rawDescGZIP(), []int{2} +} + +func (x *GetSandboxProviderEnvironmentRequest) GetSandboxId() string { + if x != nil { + return x.SandboxId + } + return "" +} + +type GetSandboxProviderEnvironmentResponse struct { + state protoimpl.MessageState `protogen:"open.v1"` + // Provider credential environment variables. + Environment map[string]string `protobuf:"bytes,1,rep,name=environment,proto3" json:"environment,omitempty" protobuf_key:"bytes,1,opt,name=key" protobuf_val:"bytes,2,opt,name=value"` + // Fingerprint for the provider credential inputs that produced environment. + ProviderEnvRevision uint64 `protobuf:"varint,2,opt,name=provider_env_revision,json=providerEnvRevision,proto3" json:"provider_env_revision,omitempty"` + // Expiration timestamps for returned environment variables. + CredentialExpiresAtMs map[string]int64 `protobuf:"bytes,3,rep,name=credential_expires_at_ms,json=credentialExpiresAtMs,proto3" json:"credential_expires_at_ms,omitempty" protobuf_key:"bytes,1,opt,name=key" protobuf_val:"varint,2,opt,name=value"` + unknownFields protoimpl.UnknownFields + sizeCache protoimpl.SizeCache +} + +func (x *GetSandboxProviderEnvironmentResponse) Reset() { + *x = GetSandboxProviderEnvironmentResponse{} + mi := &file_openshell_proto_msgTypes[3] + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + ms.StoreMessageInfo(mi) +} + +func (x *GetSandboxProviderEnvironmentResponse) String() string { + return protoimpl.X.MessageStringOf(x) +} + +func (*GetSandboxProviderEnvironmentResponse) ProtoMessage() {} + +func (x *GetSandboxProviderEnvironmentResponse) ProtoReflect() protoreflect.Message { + mi := &file_openshell_proto_msgTypes[3] + if x != nil { + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + if ms.LoadMessageInfo() == nil { + ms.StoreMessageInfo(mi) + } + return ms + } + return mi.MessageOf(x) +} + +// Deprecated: Use GetSandboxProviderEnvironmentResponse.ProtoReflect.Descriptor instead. +func (*GetSandboxProviderEnvironmentResponse) Descriptor() ([]byte, []int) { + return file_openshell_proto_rawDescGZIP(), []int{3} +} + +func (x *GetSandboxProviderEnvironmentResponse) GetEnvironment() map[string]string { + if x != nil { + return x.Environment + } + return nil +} + +func (x *GetSandboxProviderEnvironmentResponse) GetProviderEnvRevision() uint64 { + if x != nil { + return x.ProviderEnvRevision + } + return 0 +} + +func (x *GetSandboxProviderEnvironmentResponse) GetCredentialExpiresAtMs() map[string]int64 { + if x != nil { + return x.CredentialExpiresAtMs + } + return nil +} + +var File_openshell_proto protoreflect.FileDescriptor + +const file_openshell_proto_rawDesc = "" + + "\n" + + "\x0fopenshell.proto\x12\fopenshell.v1\"\x1a\n" + + "\x18IssueSandboxTokenRequest\"U\n" + + "\x19IssueSandboxTokenResponse\x12\x14\n" + + "\x05token\x18\x01 \x01(\tR\x05token\x12\"\n" + + "\rexpires_at_ms\x18\x02 \x01(\x03R\vexpiresAtMs\"E\n" + + "$GetSandboxProviderEnvironmentRequest\x12\x1d\n" + + "\n" + + "sandbox_id\x18\x01 \x01(\tR\tsandboxId\"\xd7\x03\n" + + "%GetSandboxProviderEnvironmentResponse\x12f\n" + + "\venvironment\x18\x01 \x03(\v2D.openshell.v1.GetSandboxProviderEnvironmentResponse.EnvironmentEntryR\venvironment\x122\n" + + "\x15provider_env_revision\x18\x02 \x01(\x04R\x13providerEnvRevision\x12\x87\x01\n" + + "\x18credential_expires_at_ms\x18\x03 \x03(\v2N.openshell.v1.GetSandboxProviderEnvironmentResponse.CredentialExpiresAtMsEntryR\x15credentialExpiresAtMs\x1a>\n" + + "\x10EnvironmentEntry\x12\x10\n" + + "\x03key\x18\x01 \x01(\tR\x03key\x12\x14\n" + + "\x05value\x18\x02 \x01(\tR\x05value:\x028\x01\x1aH\n" + + "\x1aCredentialExpiresAtMsEntry\x12\x10\n" + + "\x03key\x18\x01 \x01(\tR\x03key\x12\x14\n" + + "\x05value\x18\x02 \x01(\x03R\x05value:\x028\x012\xfc\x01\n" + + "\tOpenShell\x12\x88\x01\n" + + "\x1dGetSandboxProviderEnvironment\x122.openshell.v1.GetSandboxProviderEnvironmentRequest\x1a3.openshell.v1.GetSandboxProviderEnvironmentResponse\x12d\n" + + "\x11IssueSandboxToken\x12&.openshell.v1.IssueSandboxTokenRequest\x1a'.openshell.v1.IssueSandboxTokenResponseB\xcc\x01\n" + + "\x10com.openshell.v1B\x0eOpenshellProtoP\x01ZWgithub.com/kagenti/kagenti-extensions/authbridge/authlib/openshell/genproto/openshellv1\xa2\x02\x03OXX\xaa\x02\fOpenshell.V1\xca\x02\fOpenshell\\V1\xe2\x02\x18Openshell\\V1\\GPBMetadata\xea\x02\rOpenshell::V1b\x06proto3" + +var ( + file_openshell_proto_rawDescOnce sync.Once + file_openshell_proto_rawDescData []byte +) + +func file_openshell_proto_rawDescGZIP() []byte { + file_openshell_proto_rawDescOnce.Do(func() { + file_openshell_proto_rawDescData = protoimpl.X.CompressGZIP(unsafe.Slice(unsafe.StringData(file_openshell_proto_rawDesc), len(file_openshell_proto_rawDesc))) + }) + return file_openshell_proto_rawDescData +} + +var file_openshell_proto_msgTypes = make([]protoimpl.MessageInfo, 6) +var file_openshell_proto_goTypes = []any{ + (*IssueSandboxTokenRequest)(nil), // 0: openshell.v1.IssueSandboxTokenRequest + (*IssueSandboxTokenResponse)(nil), // 1: openshell.v1.IssueSandboxTokenResponse + (*GetSandboxProviderEnvironmentRequest)(nil), // 2: openshell.v1.GetSandboxProviderEnvironmentRequest + (*GetSandboxProviderEnvironmentResponse)(nil), // 3: openshell.v1.GetSandboxProviderEnvironmentResponse + nil, // 4: openshell.v1.GetSandboxProviderEnvironmentResponse.EnvironmentEntry + nil, // 5: openshell.v1.GetSandboxProviderEnvironmentResponse.CredentialExpiresAtMsEntry +} +var file_openshell_proto_depIdxs = []int32{ + 4, // 0: openshell.v1.GetSandboxProviderEnvironmentResponse.environment:type_name -> openshell.v1.GetSandboxProviderEnvironmentResponse.EnvironmentEntry + 5, // 1: openshell.v1.GetSandboxProviderEnvironmentResponse.credential_expires_at_ms:type_name -> openshell.v1.GetSandboxProviderEnvironmentResponse.CredentialExpiresAtMsEntry + 2, // 2: openshell.v1.OpenShell.GetSandboxProviderEnvironment:input_type -> openshell.v1.GetSandboxProviderEnvironmentRequest + 0, // 3: openshell.v1.OpenShell.IssueSandboxToken:input_type -> openshell.v1.IssueSandboxTokenRequest + 3, // 4: openshell.v1.OpenShell.GetSandboxProviderEnvironment:output_type -> openshell.v1.GetSandboxProviderEnvironmentResponse + 1, // 5: openshell.v1.OpenShell.IssueSandboxToken:output_type -> openshell.v1.IssueSandboxTokenResponse + 4, // [4:6] is the sub-list for method output_type + 2, // [2:4] is the sub-list for method input_type + 2, // [2:2] is the sub-list for extension type_name + 2, // [2:2] is the sub-list for extension extendee + 0, // [0:2] is the sub-list for field type_name +} + +func init() { file_openshell_proto_init() } +func file_openshell_proto_init() { + if File_openshell_proto != nil { + return + } + type x struct{} + out := protoimpl.TypeBuilder{ + File: protoimpl.DescBuilder{ + GoPackagePath: reflect.TypeOf(x{}).PkgPath(), + RawDescriptor: unsafe.Slice(unsafe.StringData(file_openshell_proto_rawDesc), len(file_openshell_proto_rawDesc)), + NumEnums: 0, + NumMessages: 6, + NumExtensions: 0, + NumServices: 1, + }, + GoTypes: file_openshell_proto_goTypes, + DependencyIndexes: file_openshell_proto_depIdxs, + MessageInfos: file_openshell_proto_msgTypes, + }.Build() + File_openshell_proto = out.File + file_openshell_proto_goTypes = nil + file_openshell_proto_depIdxs = nil +} diff --git a/authbridge/authlib/openshell/genproto/openshellv1/openshell_grpc.pb.go b/authbridge/authlib/openshell/genproto/openshellv1/openshell_grpc.pb.go new file mode 100644 index 000000000..23ad9cc49 --- /dev/null +++ b/authbridge/authlib/openshell/genproto/openshellv1/openshell_grpc.pb.go @@ -0,0 +1,180 @@ +// SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +// Code generated by protoc-gen-go-grpc. DO NOT EDIT. +// versions: +// - protoc-gen-go-grpc v1.5.1 +// - protoc (unknown) +// source: openshell.proto + +package openshellv1 + +import ( + context "context" + grpc "google.golang.org/grpc" + codes "google.golang.org/grpc/codes" + status "google.golang.org/grpc/status" +) + +// This is a compile-time assertion to ensure that this generated file +// is compatible with the grpc package it is being compiled against. +// Requires gRPC-Go v1.64.0 or later. +const _ = grpc.SupportPackageIsVersion9 + +const ( + OpenShell_GetSandboxProviderEnvironment_FullMethodName = "/openshell.v1.OpenShell/GetSandboxProviderEnvironment" + OpenShell_IssueSandboxToken_FullMethodName = "/openshell.v1.OpenShell/IssueSandboxToken" +) + +// OpenShellClient is the client API for OpenShell service. +// +// For semantics around ctx use and closing/ending streaming RPCs, please refer to https://pkg.go.dev/google.golang.org/grpc/?tab=doc#ClientConn.NewStream. +// +// Minimal subset of the OpenShell gateway API: only the two RPCs the AuthBridge +// placeholder-resolve gateway resolver calls, plus their messages. The full +// OpenShell API is intentionally NOT vendored here. The package, service, RPC, +// and message names and field numbers match the gateway's openshell.v1.OpenShell +// service exactly, so the wire contract is unchanged. +type OpenShellClient interface { + // Get provider environment for a sandbox (called by the sandbox supervisor, and + // by an AuthBridge sidecar, at startup). Restricted to the sandbox's own identity. + GetSandboxProviderEnvironment(ctx context.Context, in *GetSandboxProviderEnvironmentRequest, opts ...grpc.CallOption) (*GetSandboxProviderEnvironmentResponse, error) + // Mint a gateway JWT bound to the calling sandbox's identity (Bearer SA token). + IssueSandboxToken(ctx context.Context, in *IssueSandboxTokenRequest, opts ...grpc.CallOption) (*IssueSandboxTokenResponse, error) +} + +type openShellClient struct { + cc grpc.ClientConnInterface +} + +func NewOpenShellClient(cc grpc.ClientConnInterface) OpenShellClient { + return &openShellClient{cc} +} + +func (c *openShellClient) GetSandboxProviderEnvironment(ctx context.Context, in *GetSandboxProviderEnvironmentRequest, opts ...grpc.CallOption) (*GetSandboxProviderEnvironmentResponse, error) { + cOpts := append([]grpc.CallOption{grpc.StaticMethod()}, opts...) + out := new(GetSandboxProviderEnvironmentResponse) + err := c.cc.Invoke(ctx, OpenShell_GetSandboxProviderEnvironment_FullMethodName, in, out, cOpts...) + if err != nil { + return nil, err + } + return out, nil +} + +func (c *openShellClient) IssueSandboxToken(ctx context.Context, in *IssueSandboxTokenRequest, opts ...grpc.CallOption) (*IssueSandboxTokenResponse, error) { + cOpts := append([]grpc.CallOption{grpc.StaticMethod()}, opts...) + out := new(IssueSandboxTokenResponse) + err := c.cc.Invoke(ctx, OpenShell_IssueSandboxToken_FullMethodName, in, out, cOpts...) + if err != nil { + return nil, err + } + return out, nil +} + +// OpenShellServer is the server API for OpenShell service. +// All implementations must embed UnimplementedOpenShellServer +// for forward compatibility. +// +// Minimal subset of the OpenShell gateway API: only the two RPCs the AuthBridge +// placeholder-resolve gateway resolver calls, plus their messages. The full +// OpenShell API is intentionally NOT vendored here. The package, service, RPC, +// and message names and field numbers match the gateway's openshell.v1.OpenShell +// service exactly, so the wire contract is unchanged. +type OpenShellServer interface { + // Get provider environment for a sandbox (called by the sandbox supervisor, and + // by an AuthBridge sidecar, at startup). Restricted to the sandbox's own identity. + GetSandboxProviderEnvironment(context.Context, *GetSandboxProviderEnvironmentRequest) (*GetSandboxProviderEnvironmentResponse, error) + // Mint a gateway JWT bound to the calling sandbox's identity (Bearer SA token). + IssueSandboxToken(context.Context, *IssueSandboxTokenRequest) (*IssueSandboxTokenResponse, error) + mustEmbedUnimplementedOpenShellServer() +} + +// UnimplementedOpenShellServer must be embedded to have +// forward compatible implementations. +// +// NOTE: this should be embedded by value instead of pointer to avoid a nil +// pointer dereference when methods are called. +type UnimplementedOpenShellServer struct{} + +func (UnimplementedOpenShellServer) GetSandboxProviderEnvironment(context.Context, *GetSandboxProviderEnvironmentRequest) (*GetSandboxProviderEnvironmentResponse, error) { + return nil, status.Errorf(codes.Unimplemented, "method GetSandboxProviderEnvironment not implemented") +} +func (UnimplementedOpenShellServer) IssueSandboxToken(context.Context, *IssueSandboxTokenRequest) (*IssueSandboxTokenResponse, error) { + return nil, status.Errorf(codes.Unimplemented, "method IssueSandboxToken not implemented") +} +func (UnimplementedOpenShellServer) mustEmbedUnimplementedOpenShellServer() {} +func (UnimplementedOpenShellServer) testEmbeddedByValue() {} + +// UnsafeOpenShellServer may be embedded to opt out of forward compatibility for this service. +// Use of this interface is not recommended, as added methods to OpenShellServer will +// result in compilation errors. +type UnsafeOpenShellServer interface { + mustEmbedUnimplementedOpenShellServer() +} + +func RegisterOpenShellServer(s grpc.ServiceRegistrar, srv OpenShellServer) { + // If the following call pancis, it indicates UnimplementedOpenShellServer was + // embedded by pointer and is nil. This will cause panics if an + // unimplemented method is ever invoked, so we test this at initialization + // time to prevent it from happening at runtime later due to I/O. + if t, ok := srv.(interface{ testEmbeddedByValue() }); ok { + t.testEmbeddedByValue() + } + s.RegisterService(&OpenShell_ServiceDesc, srv) +} + +func _OpenShell_GetSandboxProviderEnvironment_Handler(srv interface{}, ctx context.Context, dec func(interface{}) error, interceptor grpc.UnaryServerInterceptor) (interface{}, error) { + in := new(GetSandboxProviderEnvironmentRequest) + if err := dec(in); err != nil { + return nil, err + } + if interceptor == nil { + return srv.(OpenShellServer).GetSandboxProviderEnvironment(ctx, in) + } + info := &grpc.UnaryServerInfo{ + Server: srv, + FullMethod: OpenShell_GetSandboxProviderEnvironment_FullMethodName, + } + handler := func(ctx context.Context, req interface{}) (interface{}, error) { + return srv.(OpenShellServer).GetSandboxProviderEnvironment(ctx, req.(*GetSandboxProviderEnvironmentRequest)) + } + return interceptor(ctx, in, info, handler) +} + +func _OpenShell_IssueSandboxToken_Handler(srv interface{}, ctx context.Context, dec func(interface{}) error, interceptor grpc.UnaryServerInterceptor) (interface{}, error) { + in := new(IssueSandboxTokenRequest) + if err := dec(in); err != nil { + return nil, err + } + if interceptor == nil { + return srv.(OpenShellServer).IssueSandboxToken(ctx, in) + } + info := &grpc.UnaryServerInfo{ + Server: srv, + FullMethod: OpenShell_IssueSandboxToken_FullMethodName, + } + handler := func(ctx context.Context, req interface{}) (interface{}, error) { + return srv.(OpenShellServer).IssueSandboxToken(ctx, req.(*IssueSandboxTokenRequest)) + } + return interceptor(ctx, in, info, handler) +} + +// OpenShell_ServiceDesc is the grpc.ServiceDesc for OpenShell service. +// It's only intended for direct use with grpc.RegisterService, +// and not to be introspected or modified (even as a copy) +var OpenShell_ServiceDesc = grpc.ServiceDesc{ + ServiceName: "openshell.v1.OpenShell", + HandlerType: (*OpenShellServer)(nil), + Methods: []grpc.MethodDesc{ + { + MethodName: "GetSandboxProviderEnvironment", + Handler: _OpenShell_GetSandboxProviderEnvironment_Handler, + }, + { + MethodName: "IssueSandboxToken", + Handler: _OpenShell_IssueSandboxToken_Handler, + }, + }, + Streams: []grpc.StreamDesc{}, + Metadata: "openshell.proto", +} diff --git a/authbridge/authlib/openshell/proto/openshell.proto b/authbridge/authlib/openshell/proto/openshell.proto new file mode 100644 index 000000000..68800f4d0 --- /dev/null +++ b/authbridge/authlib/openshell/proto/openshell.proto @@ -0,0 +1,48 @@ +// SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +syntax = "proto3"; + +package openshell.v1; + +// Minimal subset of the OpenShell gateway API: only the two RPCs the AuthBridge +// placeholder-resolve gateway resolver calls, plus their messages. The full +// OpenShell API is intentionally NOT vendored here. The package, service, RPC, +// and message names and field numbers match the gateway's openshell.v1.OpenShell +// service exactly, so the wire contract is unchanged. +service OpenShell { + // Get provider environment for a sandbox (called by the sandbox supervisor, and + // by an AuthBridge sidecar, at startup). Restricted to the sandbox's own identity. + rpc GetSandboxProviderEnvironment(GetSandboxProviderEnvironmentRequest) + returns (GetSandboxProviderEnvironmentResponse); + + // Mint a gateway JWT bound to the calling sandbox's identity (Bearer SA token). + rpc IssueSandboxToken(IssueSandboxTokenRequest) returns (IssueSandboxTokenResponse); +} + +// IssueSandboxToken request. The caller is identified by its SA-token bearer +// credential, so the request body carries no fields. +message IssueSandboxTokenRequest {} + +// IssueSandboxToken response. +message IssueSandboxTokenResponse { + // Gateway-minted JWT bound to the calling sandbox's UUID. + string token = 1; + // Absolute expiry of the issued token, milliseconds since the epoch. 0 means + // the token is non-expiring. + int64 expires_at_ms = 2; +} + +message GetSandboxProviderEnvironmentRequest { + // The sandbox ID. + string sandbox_id = 1; +} + +message GetSandboxProviderEnvironmentResponse { + // Provider credential environment variables. + map environment = 1; + // Fingerprint for the provider credential inputs that produced environment. + uint64 provider_env_revision = 2; + // Expiration timestamps for returned environment variables. + map credential_expires_at_ms = 3; +} diff --git a/authbridge/authlib/plugins/placeholderresolve/gateway/resolver.go b/authbridge/authlib/plugins/placeholderresolve/gateway/resolver.go new file mode 100644 index 000000000..2a34ebefb --- /dev/null +++ b/authbridge/authlib/plugins/placeholderresolve/gateway/resolver.go @@ -0,0 +1,162 @@ +// Package gateway provides a placeholderresolve.Resolver backed by the +// OpenShell gateway: it fetches the sandbox's resolved provider environment +// once (and on a refresh schedule) and serves placeholder lookups from an +// in-memory cache, so per-request resolution does no network I/O. +// +// It satisfies the plugin's Resolver and lifecycle interfaces structurally — +// this package does not import placeholderresolve, avoiding an import cycle. +package gateway + +import ( + "context" + "log/slog" + "sync/atomic" + "time" + + "github.com/kagenti/kagenti-extensions/authbridge/authlib/openshell" +) + +const ( + // defaultRefreshInterval bounds how often the cache is refreshed when no + // shorter per-credential expiry applies. + defaultRefreshInterval = 5 * time.Minute + // retryInterval is how often Start/refresh retries before the first + // successful fetch (the gateway may not be reachable at pod boot). + retryInterval = 3 * time.Second + // minRefreshInterval floors the expiry-derived refresh delay. + minRefreshInterval = 30 * time.Second +) + +// Config locates the gateway and the sandbox identity material. +type Config struct { + Endpoint string + MTLSCertDir string + SATokenPath string + SandboxID string +} + +// Resolver caches a sandbox's resolved provider environment from the gateway. +type Resolver struct { + client *openshell.Client + refreshInterval time.Duration + + env atomic.Pointer[openshell.Environment] + ready atomic.Bool + bgCancel atomic.Pointer[context.CancelFunc] +} + +// New dials the gateway and returns an unstarted Resolver. Call Start to prime +// the cache and begin background refresh. +func New(cfg Config) (*Resolver, error) { + client, err := openshell.Dial(openshell.Config{ + Endpoint: cfg.Endpoint, + MTLSCertDir: cfg.MTLSCertDir, + SATokenPath: cfg.SATokenPath, + SandboxID: cfg.SandboxID, + }) + if err != nil { + return nil, err + } + return &Resolver{client: client, refreshInterval: defaultRefreshInterval}, nil +} + +// Resolve returns the real value for an env-var key from the cached +// environment. It fails closed (ok=false) when the cache is not yet primed, +// the key is absent, or the key's credential has expired. +func (r *Resolver) Resolve(_ context.Context, key string) (string, bool) { + env := r.env.Load() + if env == nil { + return "", false + } + v, ok := env.Values[key] + if !ok { + return "", false + } + if exp, has := env.ExpiresAtMs[key]; has && exp > 0 && time.Now().UnixMilli() >= exp { + return "", false + } + return v, true +} + +// Start launches the refresh loop on a process-lifetime context (so it +// outlives the pipeline's Init budget) and returns immediately. The loop +// fetches right away; Ready() flips true once the first fetch succeeds. +func (r *Resolver) Start(_ context.Context) error { + if r.bgCancel.Load() != nil { + return nil // already started + } + bgCtx, cancel := context.WithCancel(context.Background()) + r.bgCancel.Store(&cancel) + go r.refreshLoop(bgCtx) + return nil +} + +// Ready reports whether the cache has been primed at least once. +func (r *Resolver) Ready() bool { return r.ready.Load() } + +// Stop cancels the refresh loop and closes the gateway connection. +func (r *Resolver) Stop(_ context.Context) error { + if cancel := r.bgCancel.Swap(nil); cancel != nil { + (*cancel)() + } + return r.client.Close() +} + +func (r *Resolver) fetch(ctx context.Context) error { + env, err := r.client.FetchEnvironment(ctx) + if err != nil { + return err + } + r.env.Store(env) + r.ready.Store(true) + return nil +} + +func (r *Resolver) refreshLoop(ctx context.Context) { + for { + if err := r.fetch(ctx); err != nil { + slog.Warn("gateway resolver: provider-environment fetch failed", "error", err) + } + timer := time.NewTimer(r.nextDelay()) + select { + case <-ctx.Done(): + timer.Stop() + return + case <-timer.C: + } + } +} + +// nextDelay schedules the next refresh: a short retry until the cache is +// primed, then 80% of the soonest per-credential expiry (clamped), or the +// default interval when nothing expires. +func (r *Resolver) nextDelay() time.Duration { + if !r.ready.Load() { + return retryInterval + } + env := r.env.Load() + if env == nil || len(env.ExpiresAtMs) == 0 { + return r.refreshInterval + } + now := time.Now().UnixMilli() + soonest := int64(-1) + for _, exp := range env.ExpiresAtMs { + if exp <= 0 { + continue + } + if soonest < 0 || exp < soonest { + soonest = exp + } + } + if soonest < 0 { + return r.refreshInterval + } + d := time.Duration(soonest-now) * time.Millisecond * 8 / 10 + if d < minRefreshInterval { + d = minRefreshInterval + } + if d > r.refreshInterval { + d = r.refreshInterval + } + return d +} diff --git a/authbridge/authlib/plugins/placeholderresolve/gateway_e2e_test.go b/authbridge/authlib/plugins/placeholderresolve/gateway_e2e_test.go new file mode 100644 index 000000000..18b2499fd --- /dev/null +++ b/authbridge/authlib/plugins/placeholderresolve/gateway_e2e_test.go @@ -0,0 +1,89 @@ +package placeholderresolve + +import ( + "context" + "encoding/json" + "net" + "net/http" + "os" + "path/filepath" + "testing" + "time" + + "google.golang.org/grpc" + + osv1 "github.com/kagenti/kagenti-extensions/authbridge/authlib/openshell/genproto/openshellv1" + "github.com/kagenti/kagenti-extensions/authbridge/authlib/pipeline" +) + +// e2eMockGateway is a minimal in-process OpenShell gateway: it mints a fixed +// JWT and returns one resolved credential. +type e2eMockGateway struct { + osv1.UnimplementedOpenShellServer +} + +func (e2eMockGateway) IssueSandboxToken(context.Context, *osv1.IssueSandboxTokenRequest) (*osv1.IssueSandboxTokenResponse, error) { + return &osv1.IssueSandboxTokenResponse{Token: "jwt-1"}, nil +} + +func (e2eMockGateway) GetSandboxProviderEnvironment(context.Context, *osv1.GetSandboxProviderEnvironmentRequest) (*osv1.GetSandboxProviderEnvironmentResponse, error) { + return &osv1.GetSandboxProviderEnvironmentResponse{ + Environment: map[string]string{"ANTHROPIC_AUTH_TOKEN": "sk-real"}, + }, nil +} + +// TestOnRequestGatewayResolver drives the whole gateway path end-to-end: +// Configure(gateway block) -> Init (background warm-up) -> Ready -> OnRequest +// resolves the placeholder against the gateway's provider environment. +func TestOnRequestGatewayResolver(t *testing.T) { + lis, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("listen: %v", err) + } + srv := grpc.NewServer() + osv1.RegisterOpenShellServer(srv, e2eMockGateway{}) + go func() { _ = srv.Serve(lis) }() + t.Cleanup(srv.Stop) + + saPath := filepath.Join(t.TempDir(), "token") + if err := os.WriteFile(saPath, []byte("sa-token"), 0o600); err != nil { + t.Fatalf("write SA token: %v", err) + } + + raw, _ := json.Marshal(map[string]any{ + "gateway": map[string]any{ + "endpoint": "http://" + lis.Addr().String(), + "sandbox_id": "sb-123", + "sa_token_path": saPath, + }, + }) + + p := New() + if err := p.Configure(raw); err != nil { + t.Fatalf("Configure: %v", err) + } + if err := p.Init(context.Background()); err != nil { + t.Fatalf("Init: %v", err) + } + t.Cleanup(func() { _ = p.Shutdown(context.Background()) }) + + // The gateway resolver primes its cache asynchronously. + deadline := time.Now().Add(3 * time.Second) + for !p.Ready() { + if time.Now().After(deadline) { + t.Fatal("plugin never became Ready (gateway resolver did not prime)") + } + time.Sleep(20 * time.Millisecond) + } + + pctx := &pipeline.Context{Direction: pipeline.Outbound, Headers: http.Header{}} + pctx.Headers.Set("Authorization", "Bearer openshell:resolve:env:ANTHROPIC_AUTH_TOKEN") + + act := p.OnRequest(context.Background(), pctx) + if act.Type != pipeline.Continue { + t.Fatalf("OnRequest = %v (violation %+v), want Continue", act.Type, act.Violation) + } + if got := pctx.Headers.Get("Authorization"); got != "Bearer sk-real" { + t.Errorf("Authorization = %q, want Bearer sk-real", got) + } +} diff --git a/authbridge/authlib/plugins/placeholderresolve/plugin.go b/authbridge/authlib/plugins/placeholderresolve/plugin.go index 4893e77bd..97c498430 100644 --- a/authbridge/authlib/plugins/placeholderresolve/plugin.go +++ b/authbridge/authlib/plugins/placeholderresolve/plugin.go @@ -11,10 +11,11 @@ // and rewrites the header in place. Unresolvable placeholders fail closed // (the request is denied rather than forwarded with the placeholder). // -// The Resolver is the swap seam: the default implementations resolve from -// an inline map (isolation testing), a mounted secret directory, or the -// process environment. A future gateway-gRPC resolver (calling OpenShell's -// GetSandboxProviderEnvironment) plugs in here without touching the plugin. +// The Resolver is the swap seam. The primary source is the OpenShell gateway +// (the `gateway` config block — fetches the sandbox's resolved provider +// environment via GetSandboxProviderEnvironment, requires running as a +// sidecar in the sandbox pod). For testing it can resolve from an inline +// map, a mounted secret directory, or the process environment. package placeholderresolve import ( @@ -31,6 +32,7 @@ import ( "github.com/kagenti/kagenti-extensions/authbridge/authlib/config" "github.com/kagenti/kagenti-extensions/authbridge/authlib/pipeline" "github.com/kagenti/kagenti-extensions/authbridge/authlib/plugins" + "github.com/kagenti/kagenti-extensions/authbridge/authlib/plugins/placeholderresolve/gateway" ) // defaultPrefix is the OpenShell placeholder prefix. Kept identical to the @@ -44,6 +46,13 @@ const defaultPrefix = "openshell:resolve:env:" // file-source path join safe from traversal. const envKeyPattern = `([A-Za-z_][A-Za-z0-9_]*)` +// Default OpenShell sidecar paths for the gateway source (match the k8s +// driver's pod-spec conventions). +const ( + defaultMTLSCertDir = "/etc/openshell-tls/client" + defaultSATokenPath = "/var/run/secrets/openshell/token" +) + // placeholderResolveConfig is the plugin's local config schema. Field tags // drive both runtime decoding (json) and operator-facing schema // introspection (description / default). @@ -57,16 +66,29 @@ type placeholderResolveConfig struct { // upstream wire today). Headers []string `json:"headers" description:"Request headers to scan for placeholders." default:"Authorization"` + // Gateway, when set, resolves each KEY from the OpenShell gateway's + // resolved provider environment (the native-provider source). Takes + // precedence over mappings/secret_dir/env. Requires running as a sidecar + // in the sandbox pod (shared SA + sandbox-id annotation). + Gateway *gatewayConfig `json:"gateway" description:"Resolve credentials from the OpenShell gateway (sandbox sidecar)."` + // Mappings is an optional inline KEY->value map used as the resolver - // source. Intended for isolation testing; takes precedence over - // secret_dir and env when non-empty. + // source. Intended for isolation testing. Mappings map[string]string `json:"mappings" description:"Inline KEY->value resolver source (isolation testing)."` - // SecretDir, when set (and Mappings empty), resolves each KEY by - // reading / as a credential file. + // SecretDir, when set, resolves each KEY by reading / as + // a credential file. SecretDir string `json:"secret_dir" description:"Directory to resolve each KEY from a file named ."` } +// gatewayConfig configures the OpenShell-gateway resolver source. +type gatewayConfig struct { + Endpoint string `json:"endpoint" required:"true" description:"OpenShell gateway gRPC endpoint, e.g. https://openshell..svc:8080."` + MTLSCertDir string `json:"mtls_cert_dir" description:"Dir holding ca.crt/tls.crt/tls.key for mTLS to the gateway." default:"/etc/openshell-tls/client"` + SATokenPath string `json:"sa_token_path" description:"Projected SA token file (audience openshell-gateway)." default:"/var/run/secrets/openshell/token"` + SandboxID string `json:"sandbox_id" required:"true" description:"This sandbox's id (OPENSHELL_SANDBOX_ID); must match the gateway-minted JWT."` +} + func (c *placeholderResolveConfig) applyDefaults() { if c.Prefix == "" { c.Prefix = defaultPrefix @@ -74,6 +96,14 @@ func (c *placeholderResolveConfig) applyDefaults() { if len(c.Headers) == 0 { c.Headers = []string{"Authorization"} } + if c.Gateway != nil { + if c.Gateway.MTLSCertDir == "" { + c.Gateway.MTLSCertDir = defaultMTLSCertDir + } + if c.Gateway.SATokenPath == "" { + c.Gateway.SATokenPath = defaultSATokenPath + } + } } func (c *placeholderResolveConfig) validate() error { @@ -83,16 +113,32 @@ func (c *placeholderResolveConfig) validate() error { if len(c.Headers) == 0 { return errors.New("headers must list at least one header") } + if c.Gateway != nil { + if c.Gateway.Endpoint == "" { + return errors.New("gateway.endpoint is required") + } + if c.Gateway.SandboxID == "" { + return errors.New("gateway.sandbox_id is required") + } + } return nil } // Resolver maps a placeholder KEY to its real secret value. ok is false -// when the key is unknown — the plugin fails closed on a false. This is the -// swap seam: the gateway-gRPC source (B2) implements this interface. +// when the key is unknown — the plugin fails closed on a false. type Resolver interface { Resolve(ctx context.Context, key string) (string, bool) } +// lifecycleResolver is implemented by resolvers needing background warm-up +// (the gateway source). The map/file/env resolvers do not implement it and +// are treated as always-ready. +type lifecycleResolver interface { + Start(ctx context.Context) error + Ready() bool + Stop(ctx context.Context) error +} + // mapResolver resolves from an inline map (isolation testing). type mapResolver map[string]string @@ -148,9 +194,10 @@ func (p *PlaceholderResolve) Capabilities() pipeline.PluginCapabilities { } } -// ConfigSchema surfaces field metadata to config-aware tooling. +// ConfigSchema surfaces field metadata to config-aware tooling. A non-nil +// Gateway is passed so the nested block's fields are reflected. func (p *PlaceholderResolve) ConfigSchema() []pipeline.FieldSchema { - return pipeline.SchemaOf(placeholderResolveConfig{}) + return pipeline.SchemaOf(placeholderResolveConfig{Gateway: &gatewayConfig{}}) } func (p *PlaceholderResolve) Configure(raw json.RawMessage) error { @@ -172,11 +219,21 @@ func (p *PlaceholderResolve) Configure(raw json.RawMessage) error { return fmt.Errorf("placeholder-resolve: compile prefix pattern: %w", err) } - // Pick the resolver source. Mappings wins (deterministic, for tests), - // then a mounted secret dir, else the process environment. The gateway - // gRPC source (B2) will be selected here behind a new config option. + // Pick the resolver source. The gateway (native OpenShell provider) wins; + // then inline mappings, a mounted secret dir, else the process env. var resolver Resolver switch { + case c.Gateway != nil: + gw, gerr := gateway.New(gateway.Config{ + Endpoint: c.Gateway.Endpoint, + MTLSCertDir: c.Gateway.MTLSCertDir, + SATokenPath: c.Gateway.SATokenPath, + SandboxID: c.Gateway.SandboxID, + }) + if gerr != nil { + return fmt.Errorf("placeholder-resolve: gateway resolver: %w", gerr) + } + resolver = gw case len(c.Mappings) > 0: resolver = mapResolver(c.Mappings) case c.SecretDir != "": @@ -193,6 +250,33 @@ func (p *PlaceholderResolve) Configure(raw json.RawMessage) error { return nil } +// Init starts a lifecycle-capable resolver's background warm-up (the gateway +// source). It returns promptly; readiness is reported via Ready so the +// pipeline gates traffic until the credential cache is primed. +func (p *PlaceholderResolve) Init(ctx context.Context) error { + if lr, ok := p.resolver.(lifecycleResolver); ok { + return lr.Start(ctx) + } + return nil +} + +// Ready reports resolver readiness — always true for the static +// (map/file/env) resolvers; gated on the primed cache for the gateway source. +func (p *PlaceholderResolve) Ready() bool { + if lr, ok := p.resolver.(lifecycleResolver); ok { + return lr.Ready() + } + return true +} + +// Shutdown stops a lifecycle-capable resolver's background work. +func (p *PlaceholderResolve) Shutdown(ctx context.Context) error { + if lr, ok := p.resolver.(lifecycleResolver); ok { + return lr.Stop(ctx) + } + return nil +} + func (p *PlaceholderResolve) OnRequest(ctx context.Context, pctx *pipeline.Context) pipeline.Action { if p.resolver == nil || p.re == nil { return pipeline.DenyStatus(503, "upstream.unreachable", "placeholder-resolve not configured") @@ -263,4 +347,7 @@ func validResolved(s string) bool { var ( _ pipeline.Plugin = (*PlaceholderResolve)(nil) _ pipeline.Configurable = (*PlaceholderResolve)(nil) + _ pipeline.Initializer = (*PlaceholderResolve)(nil) + _ pipeline.Readier = (*PlaceholderResolve)(nil) + _ pipeline.Shutdowner = (*PlaceholderResolve)(nil) ) From dd2277d8e91d13cb632b2b78701d6b2513c8fbbb Mon Sep 17 00:00:00 2001 From: Hai Huang Date: Thu, 25 Jun 2026 23:11:17 -0400 Subject: [PATCH 3/9] fix(authbridge): harden placeholder-resolve gateway client (review) Address PR #626 review feedback on the OpenShell gateway resolver. - Fail closed on plaintext gRPC: http:// (or a bare host with no mtls_cert_dir) to a non-loopback gateway is refused, since the SA token and minted JWT would otherwise travel as cleartext metadata. An explicit `insecure: true` opt-in (logged with a warning) overrides; loopback is always allowed. (openshell.Config.Insecure, plumbed through the plugin's gateway config block.) - Bound every gateway RPC with a 10s timeout so a connected-but- unresponsive gateway cannot block the resolver's background refresh goroutine indefinitely (stale cache -> fail-closed denials). - Keep authbridge-lite minimal: exclude placeholder-resolve from the lite build (it pulls in gRPC + the openshell client) via the lite build-args in build.yaml / ci.yaml / CLAUDE.md. - Document the plugin's config in docs/plugin-reference.md. - Tests: plaintext fail-closed / insecure / loopback dial paths, mtlsConfig + hostOnly + isLoopbackHost, and the resolver's expiry-derived nextDelay. Assisted-By: Claude (Anthropic AI) Signed-off-by: Hai Huang --- .github/workflows/build.yaml | 2 +- .github/workflows/ci.yaml | 2 +- authbridge/CLAUDE.md | 2 +- authbridge/authlib/openshell/client.go | 45 +++++- authbridge/authlib/openshell/client_test.go | 147 ++++++++++++++++++ .../placeholderresolve/gateway/resolver.go | 3 + .../gateway/resolver_test.go | 46 ++++++ .../plugins/placeholderresolve/plugin.go | 2 + authbridge/docs/plugin-reference.md | 39 +++++ 9 files changed, 283 insertions(+), 5 deletions(-) create mode 100644 authbridge/authlib/plugins/placeholderresolve/gateway/resolver_test.go diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml index 0ff8fd97f..05c152af8 100644 --- a/.github/workflows/build.yaml +++ b/.github/workflows/build.yaml @@ -54,7 +54,7 @@ jobs: context: ./authbridge dockerfile: cmd/authbridge-proxy/Dockerfile build_args: | - GO_BUILD_TAGS=exclude_plugin_a2aparser,exclude_plugin_ibac,exclude_plugin_inferenceparser,exclude_plugin_mcpparser,exclude_plugin_opa,exclude_plugin_sparc,exclude_plugin_tokenbroker + GO_BUILD_TAGS=exclude_plugin_a2aparser,exclude_plugin_ibac,exclude_plugin_inferenceparser,exclude_plugin_mcpparser,exclude_plugin_opa,exclude_plugin_placeholderresolve,exclude_plugin_sparc,exclude_plugin_tokenbroker # SPARC reflection service — the backend the `sparc` plugin calls. # Deployed once per cluster via authbridge/sparc-service/deploy. diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 87bad7b2c..df7d4c84e 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -108,7 +108,7 @@ jobs: if: matrix.binary == 'authbridge-proxy' run: | TAGS="exclude_plugin_a2aparser,exclude_plugin_ibac,exclude_plugin_inferenceparser" - TAGS="$TAGS,exclude_plugin_mcpparser,exclude_plugin_opa,exclude_plugin_sparc,exclude_plugin_tokenbroker" + TAGS="$TAGS,exclude_plugin_mcpparser,exclude_plugin_opa,exclude_plugin_placeholderresolve,exclude_plugin_sparc,exclude_plugin_tokenbroker" go build -v -tags "$TAGS" ./... go test -v -race -cover -tags "$TAGS" ./... diff --git a/authbridge/CLAUDE.md b/authbridge/CLAUDE.md index 5574ebba1..5f85bb1cf 100644 --- a/authbridge/CLAUDE.md +++ b/authbridge/CLAUDE.md @@ -362,7 +362,7 @@ podman build -f cmd/authbridge-proxy/Dockerfile -t authbridge:latest . # p podman build -f cmd/authbridge-envoy/Dockerfile -t authbridge-envoy:latest . # envoy-sidecar # authbridge-lite: the proxy Dockerfile built with exclude_plugin_* tags (auth-only) podman build -f cmd/authbridge-proxy/Dockerfile \ - --build-arg GO_BUILD_TAGS="exclude_plugin_a2aparser,exclude_plugin_ibac,exclude_plugin_inferenceparser,exclude_plugin_mcpparser,exclude_plugin_opa,exclude_plugin_sparc,exclude_plugin_tokenbroker" \ + --build-arg GO_BUILD_TAGS="exclude_plugin_a2aparser,exclude_plugin_ibac,exclude_plugin_inferenceparser,exclude_plugin_mcpparser,exclude_plugin_opa,exclude_plugin_placeholderresolve,exclude_plugin_sparc,exclude_plugin_tokenbroker" \ -t authbridge-lite:latest . kind load docker-image authbridge:latest --name kagenti kind load docker-image authbridge-envoy:latest --name kagenti diff --git a/authbridge/authlib/openshell/client.go b/authbridge/authlib/openshell/client.go index 4ccf68d7c..bd7a99594 100644 --- a/authbridge/authlib/openshell/client.go +++ b/authbridge/authlib/openshell/client.go @@ -19,6 +19,7 @@ import ( "crypto/tls" "crypto/x509" "fmt" + "log/slog" "net" "os" "path/filepath" @@ -37,6 +38,10 @@ import ( osv1 "github.com/kagenti/kagenti-extensions/authbridge/authlib/openshell/genproto/openshellv1" ) +// rpcTimeout bounds each gateway RPC so a connected-but-unresponsive gateway +// cannot block a caller (e.g. the resolver's background refresh loop) forever. +const rpcTimeout = 10 * time.Second + // Config locates the gateway and the credentials needed to talk to it. The // paths mirror what the OpenShell k8s driver injects into the sandbox pod. type Config struct { @@ -54,6 +59,12 @@ type Config struct { // the id the gateway binds into the minted JWT, or the gateway rejects // the provider-environment fetch as cross-sandbox access. SandboxID string + // Insecure permits a plaintext (non-TLS) connection to a non-loopback + // gateway. Plaintext sends the SA token and minted JWT as cleartext gRPC + // metadata, so it is refused by default for non-loopback targets; this + // flag is an explicit opt-in (logged with a warning). Loopback targets are + // always allowed plaintext. + Insecure bool } func (c Config) validate() error { @@ -103,6 +114,7 @@ func Dial(cfg Config) (*Client, error) { target := cfg.Endpoint var creds credentials.TransportCredentials + plaintext := false switch { case strings.HasPrefix(cfg.Endpoint, "https://"): target = strings.TrimPrefix(cfg.Endpoint, "https://") @@ -113,7 +125,7 @@ func Dial(cfg Config) (*Client, error) { creds = credentials.NewTLS(tlsCfg) case strings.HasPrefix(cfg.Endpoint, "http://"): target = strings.TrimPrefix(cfg.Endpoint, "http://") - creds = insecure.NewCredentials() + plaintext = true default: // No scheme: mTLS when a cert dir is configured, else plaintext. if cfg.MTLSCertDir != "" { @@ -123,8 +135,21 @@ func Dial(cfg Config) (*Client, error) { } creds = credentials.NewTLS(tlsCfg) } else { - creds = insecure.NewCredentials() + plaintext = true + } + } + + if plaintext { + // Fail closed: a plaintext transport leaks the SA token and the minted + // gateway JWT as cleartext metadata. Only allow it for loopback, or + // behind the explicit Insecure opt-in (with a warning). + if !cfg.Insecure && !isLoopbackHost(target) { + return nil, fmt.Errorf("openshell: refusing plaintext gRPC to non-loopback %q — the SA token and gateway JWT would travel in cleartext; use an https:// endpoint with mtls_cert_dir, or set insecure: true to override", target) } + if !isLoopbackHost(target) { + slog.Warn("openshell: insecure plaintext gRPC to the gateway — SA token and minted JWT travel unencrypted", "endpoint", cfg.Endpoint) + } + creds = insecure.NewCredentials() } conn, err := grpc.NewClient(target, grpc.WithTransportCredentials(creds)) @@ -163,6 +188,8 @@ func (c *Client) Close() error { } func (c *Client) fetchWithToken(ctx context.Context, tok string) (*Environment, error) { + ctx, cancel := context.WithTimeout(ctx, rpcTimeout) + defer cancel() md := metadata.AppendToOutgoingContext(ctx, "authorization", "Bearer "+tok) resp, err := c.rpc.GetSandboxProviderEnvironment(md, &osv1.GetSandboxProviderEnvironmentRequest{ SandboxId: c.cfg.SandboxID, @@ -196,6 +223,8 @@ func (c *Client) mint(ctx context.Context) error { if err != nil { return fmt.Errorf("openshell: read SA token %q: %w", c.cfg.SATokenPath, err) } + ctx, cancel := context.WithTimeout(ctx, rpcTimeout) + defer cancel() md := metadata.AppendToOutgoingContext(ctx, "authorization", "Bearer "+sa) resp, err := c.rpc.IssueSandboxToken(md, &osv1.IssueSandboxTokenRequest{}) if err != nil { @@ -256,3 +285,15 @@ func hostOnly(hostport string) string { } return hostport } + +// isLoopbackHost reports whether the host part of hostport is localhost or a +// loopback IP — the only targets for which plaintext gRPC is allowed without +// the explicit Insecure opt-in. +func isLoopbackHost(hostport string) bool { + h := hostOnly(hostport) + if h == "localhost" { + return true + } + ip := net.ParseIP(h) + return ip != nil && ip.IsLoopback() +} diff --git a/authbridge/authlib/openshell/client_test.go b/authbridge/authlib/openshell/client_test.go index 86404e989..41dce532d 100644 --- a/authbridge/authlib/openshell/client_test.go +++ b/authbridge/authlib/openshell/client_test.go @@ -2,6 +2,13 @@ package openshell import ( "context" + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "crypto/x509" + "crypto/x509/pkix" + "encoding/pem" + "math/big" "net" "os" "path/filepath" @@ -9,6 +16,7 @@ import ( "strings" "sync" "testing" + "time" "google.golang.org/grpc" "google.golang.org/grpc/codes" @@ -146,3 +154,142 @@ func TestClientReMintOnUnauthenticated(t *testing.T) { t.Errorf("issueCalls = %d, want 2 (initial mint + re-mint after Unauthenticated)", m.issueCalls) } } + +func TestDialRefusesPlaintextNonLoopback(t *testing.T) { + _, err := Dial(Config{ + Endpoint: "http://gw.example.com:8080", + SATokenPath: writeSAToken(t, "sa"), + SandboxID: "sb", + }) + if err == nil { + t.Fatal("expected fail-closed error for non-loopback plaintext") + } + if !strings.Contains(err.Error(), "refusing plaintext") { + t.Errorf("error = %v, want a refusing-plaintext error", err) + } +} + +func TestDialInsecureOptIn(t *testing.T) { + c, err := Dial(Config{ + Endpoint: "http://gw.example.com:8080", + SATokenPath: writeSAToken(t, "sa"), + SandboxID: "sb", + Insecure: true, + }) + if err != nil { + t.Fatalf("Insecure opt-in should dial plaintext: %v", err) + } + _ = c.Close() +} + +func TestDialLoopbackPlaintextAllowed(t *testing.T) { + for _, ep := range []string{"http://127.0.0.1:8080", "http://localhost:8080", "http://[::1]:8080"} { + c, err := Dial(Config{Endpoint: ep, SATokenPath: writeSAToken(t, "sa"), SandboxID: "sb"}) + if err != nil { + t.Errorf("loopback %q should dial without Insecure: %v", ep, err) + continue + } + _ = c.Close() + } +} + +func TestDialHTTPSUsesMTLS(t *testing.T) { + dir := writeMTLSCertDir(t) + c, err := Dial(Config{ + Endpoint: "https://gw.example.com:8080", + MTLSCertDir: dir, + SATokenPath: writeSAToken(t, "sa"), + SandboxID: "sb", + }) + if err != nil { + t.Fatalf("https dial with cert dir: %v", err) + } + _ = c.Close() +} + +func TestDialHTTPSRequiresCertDir(t *testing.T) { + _, err := Dial(Config{ + Endpoint: "https://gw.example.com:8080", + SATokenPath: writeSAToken(t, "sa"), + SandboxID: "sb", + }) + if err == nil { + t.Fatal("https without mtls_cert_dir should fail validation") + } +} + +func TestMTLSConfig(t *testing.T) { + dir := writeMTLSCertDir(t) + cfg, err := mtlsConfig(dir, "gw.example.com") + if err != nil { + t.Fatalf("mtlsConfig: %v", err) + } + if cfg.ServerName != "gw.example.com" { + t.Errorf("ServerName = %q, want gw.example.com", cfg.ServerName) + } + if len(cfg.Certificates) != 1 { + t.Errorf("client certs = %d, want 1", len(cfg.Certificates)) + } +} + +func TestHostOnly(t *testing.T) { + for in, want := range map[string]string{ + "host:443": "host", + "host": "host", + "127.0.0.1:8080": "127.0.0.1", + "[::1]:8080": "::1", + } { + if got := hostOnly(in); got != want { + t.Errorf("hostOnly(%q) = %q, want %q", in, got, want) + } + } +} + +func TestIsLoopbackHost(t *testing.T) { + for _, h := range []string{"127.0.0.1:8080", "localhost:443", "[::1]:80", "127.0.0.1"} { + if !isLoopbackHost(h) { + t.Errorf("isLoopbackHost(%q) = false, want true", h) + } + } + for _, h := range []string{"gw.example.com:8080", "10.0.0.5:443", "8.8.8.8"} { + if isLoopbackHost(h) { + t.Errorf("isLoopbackHost(%q) = true, want false", h) + } + } +} + +// writeMTLSCertDir generates a self-signed CA cert + key and writes it as +// ca.crt / tls.crt / tls.key in a temp dir, for exercising the mTLS branch. +func writeMTLSCertDir(t *testing.T) string { + t.Helper() + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + t.Fatalf("gen key: %v", err) + } + tmpl := &x509.Certificate{ + SerialNumber: big.NewInt(1), + Subject: pkix.Name{CommonName: "test-ca"}, + NotBefore: time.Now().Add(-time.Hour), + NotAfter: time.Now().Add(time.Hour), + IsCA: true, + KeyUsage: x509.KeyUsageCertSign | x509.KeyUsageDigitalSignature, + BasicConstraintsValid: true, + } + der, err := x509.CreateCertificate(rand.Reader, tmpl, tmpl, &key.PublicKey, key) + if err != nil { + t.Fatalf("create cert: %v", err) + } + certPEM := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: der}) + keyDER, err := x509.MarshalPKCS8PrivateKey(key) + if err != nil { + t.Fatalf("marshal key: %v", err) + } + keyPEM := pem.EncodeToMemory(&pem.Block{Type: "PRIVATE KEY", Bytes: keyDER}) + dir := t.TempDir() + for name, data := range map[string][]byte{"ca.crt": certPEM, "tls.crt": certPEM, "tls.key": keyPEM} { + if err := os.WriteFile(filepath.Join(dir, name), data, 0o600); err != nil { + t.Fatalf("write %s: %v", name, err) + } + } + return dir +} diff --git a/authbridge/authlib/plugins/placeholderresolve/gateway/resolver.go b/authbridge/authlib/plugins/placeholderresolve/gateway/resolver.go index 2a34ebefb..29b200c50 100644 --- a/authbridge/authlib/plugins/placeholderresolve/gateway/resolver.go +++ b/authbridge/authlib/plugins/placeholderresolve/gateway/resolver.go @@ -33,6 +33,8 @@ type Config struct { MTLSCertDir string SATokenPath string SandboxID string + // Insecure permits plaintext gRPC to a non-loopback gateway (opt-in). + Insecure bool } // Resolver caches a sandbox's resolved provider environment from the gateway. @@ -53,6 +55,7 @@ func New(cfg Config) (*Resolver, error) { MTLSCertDir: cfg.MTLSCertDir, SATokenPath: cfg.SATokenPath, SandboxID: cfg.SandboxID, + Insecure: cfg.Insecure, }) if err != nil { return nil, err diff --git a/authbridge/authlib/plugins/placeholderresolve/gateway/resolver_test.go b/authbridge/authlib/plugins/placeholderresolve/gateway/resolver_test.go new file mode 100644 index 000000000..9435501ac --- /dev/null +++ b/authbridge/authlib/plugins/placeholderresolve/gateway/resolver_test.go @@ -0,0 +1,46 @@ +// SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +package gateway + +import ( + "testing" + "time" + + "github.com/kagenti/kagenti-extensions/authbridge/authlib/openshell" +) + +// TestNextDelay exercises the refresh schedule: a short retry until primed, +// then a per-credential-expiry-derived delay clamped to [min, refreshInterval]. +func TestNextDelay(t *testing.T) { + r := &Resolver{refreshInterval: defaultRefreshInterval} + + // Not primed yet -> retry quickly. + if d := r.nextDelay(); d != retryInterval { + t.Errorf("unready nextDelay = %v, want %v", d, retryInterval) + } + + r.ready.Store(true) + + // Primed, no expiry -> default refresh interval. + r.env.Store(&openshell.Environment{Values: map[string]string{"K": "v"}}) + if d := r.nextDelay(); d != r.refreshInterval { + t.Errorf("no-expiry nextDelay = %v, want %v", d, r.refreshInterval) + } + + // Soonest expiry very near -> clamped up to minRefreshInterval. + r.env.Store(&openshell.Environment{ + ExpiresAtMs: map[string]int64{"K": time.Now().Add(10 * time.Second).UnixMilli()}, + }) + if d := r.nextDelay(); d != minRefreshInterval { + t.Errorf("near-expiry nextDelay = %v, want %v (clamped to min)", d, minRefreshInterval) + } + + // Soonest expiry far out -> clamped down to refreshInterval. + r.env.Store(&openshell.Environment{ + ExpiresAtMs: map[string]int64{"K": time.Now().Add(time.Hour).UnixMilli()}, + }) + if d := r.nextDelay(); d != r.refreshInterval { + t.Errorf("far-expiry nextDelay = %v, want %v (clamped to refresh)", d, r.refreshInterval) + } +} diff --git a/authbridge/authlib/plugins/placeholderresolve/plugin.go b/authbridge/authlib/plugins/placeholderresolve/plugin.go index 97c498430..e5b2f61b5 100644 --- a/authbridge/authlib/plugins/placeholderresolve/plugin.go +++ b/authbridge/authlib/plugins/placeholderresolve/plugin.go @@ -87,6 +87,7 @@ type gatewayConfig struct { MTLSCertDir string `json:"mtls_cert_dir" description:"Dir holding ca.crt/tls.crt/tls.key for mTLS to the gateway." default:"/etc/openshell-tls/client"` SATokenPath string `json:"sa_token_path" description:"Projected SA token file (audience openshell-gateway)." default:"/var/run/secrets/openshell/token"` SandboxID string `json:"sandbox_id" required:"true" description:"This sandbox's id (OPENSHELL_SANDBOX_ID); must match the gateway-minted JWT."` + Insecure bool `json:"insecure" description:"Permit plaintext gRPC to a non-loopback gateway (opt-in). Sends the SA token + JWT in cleartext; refused by default for non-loopback endpoints." default:"false"` } func (c *placeholderResolveConfig) applyDefaults() { @@ -229,6 +230,7 @@ func (p *PlaceholderResolve) Configure(raw json.RawMessage) error { MTLSCertDir: c.Gateway.MTLSCertDir, SATokenPath: c.Gateway.SATokenPath, SandboxID: c.Gateway.SandboxID, + Insecure: c.Gateway.Insecure, }) if gerr != nil { return fmt.Errorf("placeholder-resolve: gateway resolver: %w", gerr) diff --git a/authbridge/docs/plugin-reference.md b/authbridge/docs/plugin-reference.md index b5f52b227..d4a7cdd20 100644 --- a/authbridge/docs/plugin-reference.md +++ b/authbridge/docs/plugin-reference.md @@ -103,6 +103,45 @@ Mint requires `pctx.Shared` to be wired by the listener; if it is nil when `placeholder_mode` is on, the plugin fails fast at init (a deploy error) rather than silently forwarding the real token. +### `placeholder-resolve`: credential injection config + +Resolves OpenShell credential placeholders (`openshell:resolve:env:`) in the +configured request headers to their real values on the wire, fail-closed. Outbound +plugin; opt-in (inert unless listed in `pipeline.outbound`). + +```yaml +- name: placeholder-resolve + config: + prefix: "openshell:resolve:env:" # default; the KEY follows this prefix + headers: ["Authorization"] # default; headers to scan + rewrite + # Resolver source — precedence: gateway > mappings > secret_dir > process env. + gateway: # native OpenShell-provider source (sidecar) + endpoint: "https://openshell..svc:8080" # required + sandbox_id: "" # required; must match the minted JWT + mtls_cert_dir: "/etc/openshell-tls/client" # ca.crt/tls.crt/tls.key; required for https + sa_token_path: "/var/run/secrets/openshell/token" + insecure: false # opt-in plaintext (non-loopback refused by default) + mappings: {} # inline KEY->value (testing) + secret_dir: "" # resolve KEY from / (mounted secret) +``` + +| Field | Default | Notes | +|-------|---------|-------| +| `prefix` | `openshell:resolve:env:` | Placeholder prefix; the KEY after it matches `[A-Za-z_][A-Za-z0-9_]*`. | +| `headers` | `[Authorization]` | `Authorization` is the only header the forward proxy propagates to the upstream wire today. | +| `gateway.endpoint` | — (required if `gateway` set) | Gateway gRPC endpoint; `https://` selects mTLS, `http://` plaintext. | +| `gateway.sandbox_id` | — (required if `gateway` set) | Must equal the id bound into the gateway-minted JWT. | +| `gateway.mtls_cert_dir` | `/etc/openshell-tls/client` | `ca.crt`/`tls.crt`/`tls.key`; required for an `https://` endpoint. | +| `gateway.sa_token_path` | `/var/run/secrets/openshell/token` | Projected SA token (audience `openshell-gateway`). | +| `gateway.insecure` | `false` | Permit plaintext gRPC to a **non-loopback** gateway. Plaintext sends the SA token + JWT in cleartext, so it is **refused by default** for non-loopback endpoints (loopback is always allowed). | +| `mappings` / `secret_dir` | — | Static resolver sources (inline map for testing / mounted secret directory). | + +An unresolved placeholder, or a resolved value containing CR/LF/NUL, fails the +request closed (`401 auth.unauthorized`) — the placeholder is never forwarded. The +`gateway` source requires running as a sidecar in the sandbox pod (shared SA + the +`openshell.io/sandbox-id` annotation) and warms its cache in the background, so the +pipeline gates traffic on `Ready()` until the first fetch succeeds. + ## `on_error` policy > **Naming caveat.** Despite the name, `on_error` controls how the From 0778e25e57cc89f1145ac9df6c26f86c71b5fa0c Mon Sep 17 00:00:00 2001 From: Hai Huang Date: Fri, 26 Jun 2026 09:09:30 -0400 Subject: [PATCH 4/9] refactor(authbridge): extract credinject; harden + simplify placeholder-resolve MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address review feedback on the placeholder-resolve plugin (PR #626). Secured: - Remove the `env` resolver (the source AND the silent default). It let the agent name any KEY and exfiltrate the proxy's own environment onto the upstream Authorization header. An unconfigured source now fails closed. - credinject.FileResolver path-contains the key (rejects separators / "..") instead of trusting the env-key grammar, so it is safe for arbitrary keys. Simplified: - Require an explicit `source` enum (gateway | secret_dir), replacing the implicit by-field-presence selection and its undocumented precedence. - Drop the `headers` field; hardcode Authorization (the only header any listener reconciles to the upstream wire — others were silently dropped). - Drop the inline `mappings` source from the production config (tests inject a resolver directly). Reusable: - New authlib/credinject package: Resolver / LifecycleResolver interfaces, FileResolver / MapResolver / DenyResolver, and SafeHeaderValue / SafeSetHeader (the CWE-113 header guard, previously unique to this plugin). placeholder- resolve consumes it; the OpenShell gateway resolver satisfies the interfaces structurally. A future host-keyed (e.g. DAM) credential injector can import credinject rather than reimplement these. Optimized: - Replace the per-request regexp with a prefix scan + a strings.Contains early-out; removes the only regexp dependency in the plugin. Tests and plugin-reference.md updated. Build, lite build (placeholder-resolve excluded), go test -race, go vet, and golangci-lint all green. Assisted-By: Claude (Anthropic AI) Signed-off-by: Hai Huang --- authbridge/authlib/credinject/credinject.go | 95 ++++++ .../authlib/credinject/credinject_test.go | 83 +++++ .../placeholderresolve/gateway_e2e_test.go | 1 + .../plugins/placeholderresolve/plugin.go | 306 ++++++++---------- .../plugins/placeholderresolve/plugin_test.go | 138 +++++--- authbridge/docs/plugin-reference.md | 28 +- 6 files changed, 426 insertions(+), 225 deletions(-) create mode 100644 authbridge/authlib/credinject/credinject.go create mode 100644 authbridge/authlib/credinject/credinject_test.go diff --git a/authbridge/authlib/credinject/credinject.go b/authbridge/authlib/credinject/credinject.go new file mode 100644 index 000000000..4c14284e9 --- /dev/null +++ b/authbridge/authlib/credinject/credinject.go @@ -0,0 +1,95 @@ +// SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +// Package credinject provides reusable primitives for resolving a credential +// and safely injecting it into an HTTP header — the shared core behind +// credential-injecting plugins (e.g. placeholder-resolve, and a future +// host-keyed injector). It is transport-agnostic and imports no plugin, so +// plugins depend on it rather than the reverse. +package credinject + +import ( + "context" + "net/http" + "path/filepath" + "strings" + + "github.com/kagenti/kagenti-extensions/authbridge/authlib/config" +) + +// Resolver maps a lookup key to a real credential value. ok is false when the +// key is unknown or unavailable — callers MUST fail closed on a false (never +// forward the unresolved key). +type Resolver interface { + Resolve(ctx context.Context, key string) (value string, ok bool) +} + +// LifecycleResolver is implemented by resolvers that need background warm-up +// (e.g. a remote/cached source). Static resolvers omit it and are treated as +// always ready. +type LifecycleResolver interface { + Start(ctx context.Context) error + Ready() bool + Stop(ctx context.Context) error +} + +// DenyResolver resolves nothing. It is the fail-closed default for an +// unconfigured source, so a missing source never silently resolves from an +// ambient source (e.g. the process environment). +type DenyResolver struct{} + +// Resolve always fails closed. +func (DenyResolver) Resolve(context.Context, string) (string, bool) { return "", false } + +// MapResolver resolves from an inline map. For tests/dev only — it holds +// cleartext credentials in process memory and is not a production source. +type MapResolver map[string]string + +// Resolve returns the mapped value for key, if present. +func (m MapResolver) Resolve(_ context.Context, key string) (string, bool) { + v, ok := m[key] + return v, ok +} + +// FileResolver resolves each key by reading / as a credential file +// (whitespace-trimmed via config.ReadCredentialFile). The key is path- +// contained: any key that is not a direct child filename of Dir (i.e. contains +// a separator or "..") is rejected, so the resolver is safe for arbitrary, +// caller-supplied keys such as a destination host. +type FileResolver struct{ Dir string } + +// Resolve reads /, rejecting keys that would escape Dir. +func (f FileResolver) Resolve(_ context.Context, key string) (string, bool) { + if key == "" { + return "", false + } + p := filepath.Join(f.Dir, key) + // The joined path must be a direct child of Dir — this rejects separators, + // absolute keys, and "../" traversal regardless of the key's grammar. + if filepath.Dir(p) != filepath.Clean(f.Dir) { + return "", false + } + v, err := config.ReadCredentialFile(p) + if err != nil { + return "", false + } + return v, true +} + +// SafeHeaderValue reports whether v is safe to place in an HTTP header value. +// It rejects CR, LF, and NUL to prevent header/response splitting (CWE-113) +// via a poisoned credential store. +func SafeHeaderValue(v string) bool { + return !strings.ContainsAny(v, "\r\n\x00") +} + +// SafeSetHeader sets h[name]=value only when value is header-safe. It returns +// false and leaves h unmodified when value is unsafe, so the caller can fail +// closed rather than forward a poisoned value. +func SafeSetHeader(h http.Header, name, value string) bool { + if !SafeHeaderValue(value) { + return false + } + h.Set(name, value) + return true +} diff --git a/authbridge/authlib/credinject/credinject_test.go b/authbridge/authlib/credinject/credinject_test.go new file mode 100644 index 000000000..6ded3e48b --- /dev/null +++ b/authbridge/authlib/credinject/credinject_test.go @@ -0,0 +1,83 @@ +// SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +package credinject + +import ( + "context" + "net/http" + "os" + "path/filepath" + "testing" +) + +func TestDenyResolverFailsClosed(t *testing.T) { + if v, ok := (DenyResolver{}).Resolve(context.Background(), "ANYTHING"); ok || v != "" { + t.Errorf("DenyResolver.Resolve = (%q, %v), want (\"\", false)", v, ok) + } +} + +func TestMapResolver(t *testing.T) { + m := MapResolver{"K": "v"} + if v, ok := m.Resolve(context.Background(), "K"); !ok || v != "v" { + t.Errorf("hit = (%q,%v), want (v,true)", v, ok) + } + if _, ok := m.Resolve(context.Background(), "MISS"); ok { + t.Error("miss should be ok=false") + } +} + +func TestFileResolverReadsDirectChild(t *testing.T) { + dir := t.TempDir() + if err := os.WriteFile(filepath.Join(dir, "TOKEN"), []byte(" sk-real\n"), 0o600); err != nil { + t.Fatal(err) + } + v, ok := FileResolver{Dir: dir}.Resolve(context.Background(), "TOKEN") + if !ok || v != "sk-real" { // ReadCredentialFile trims surrounding whitespace + t.Errorf("Resolve = (%q,%v), want (sk-real,true)", v, ok) + } +} + +func TestFileResolverRejectsTraversalAndSeparators(t *testing.T) { + dir := t.TempDir() + // A secret one level up that must never be reachable. + if err := os.WriteFile(filepath.Join(filepath.Dir(dir), "SECRET"), []byte("leak"), 0o600); err != nil { + t.Fatal(err) + } + fr := FileResolver{Dir: dir} + for _, key := range []string{"../SECRET", "sub/TOKEN", "/etc/passwd", "", "a/../../SECRET"} { + if v, ok := fr.Resolve(context.Background(), key); ok { + t.Errorf("Resolve(%q) = (%q,true), want ok=false (containment)", key, v) + } + } +} + +func TestSafeHeaderValue(t *testing.T) { + for _, ok := range []string{"sk-abc123", "Bearer x.y.z", ""} { + if !SafeHeaderValue(ok) { + t.Errorf("SafeHeaderValue(%q) = false, want true", ok) + } + } + for _, bad := range []string{"a\rb", "a\nb", "a\x00b", "tok\r\nInjected: 1"} { + if SafeHeaderValue(bad) { + t.Errorf("SafeHeaderValue(%q) = true, want false", bad) + } + } +} + +func TestSafeSetHeader(t *testing.T) { + h := http.Header{} + if !SafeSetHeader(h, "Authorization", "Bearer ok") { + t.Fatal("safe value should set") + } + if got := h.Get("Authorization"); got != "Bearer ok" { + t.Errorf("Authorization = %q", got) + } + h2 := http.Header{} + if SafeSetHeader(h2, "Authorization", "Bearer bad\r\nX: y") { + t.Fatal("unsafe value should not set") + } + if h2.Get("Authorization") != "" { + t.Error("header must be unmodified on unsafe value") + } +} diff --git a/authbridge/authlib/plugins/placeholderresolve/gateway_e2e_test.go b/authbridge/authlib/plugins/placeholderresolve/gateway_e2e_test.go index 18b2499fd..484fb2788 100644 --- a/authbridge/authlib/plugins/placeholderresolve/gateway_e2e_test.go +++ b/authbridge/authlib/plugins/placeholderresolve/gateway_e2e_test.go @@ -51,6 +51,7 @@ func TestOnRequestGatewayResolver(t *testing.T) { } raw, _ := json.Marshal(map[string]any{ + "source": "gateway", "gateway": map[string]any{ "endpoint": "http://" + lis.Addr().String(), "sandbox_id": "sb-123", diff --git a/authbridge/authlib/plugins/placeholderresolve/plugin.go b/authbridge/authlib/plugins/placeholderresolve/plugin.go index e5b2f61b5..63469838b 100644 --- a/authbridge/authlib/plugins/placeholderresolve/plugin.go +++ b/authbridge/authlib/plugins/placeholderresolve/plugin.go @@ -1,21 +1,24 @@ // Package placeholderresolve provides an outbound pipeline plugin that -// resolves OpenShell-style credential placeholders in request headers to -// their real secret values, on the wire, so the agent never holds the -// real credential. +// resolves OpenShell-style credential placeholders in the Authorization +// header to their real secret values, on the wire, so the agent never holds +// the real credential. // // OpenShell injects an agent's credential env (e.g. ANTHROPIC_AUTH_TOKEN) // as a placeholder string of the form "openshell:resolve:env:". The -// agent emits that placeholder verbatim (for the `claude` provider it -// rides in `Authorization: Bearer `); this plugin scans the -// configured headers, resolves each placeholder via an injected Resolver, -// and rewrites the header in place. Unresolvable placeholders fail closed -// (the request is denied rather than forwarded with the placeholder). +// agent emits that placeholder verbatim (for the `claude` provider it rides +// in `Authorization: Bearer ` — the only header any listener +// reconciles to the upstream wire). This plugin scans the Authorization +// header, resolves each placeholder via the configured source, and rewrites +// the header in place. Unresolvable placeholders fail closed (the request is +// denied rather than forwarded with the placeholder). // -// The Resolver is the swap seam. The primary source is the OpenShell gateway -// (the `gateway` config block — fetches the sandbox's resolved provider -// environment via GetSandboxProviderEnvironment, requires running as a -// sidecar in the sandbox pod). For testing it can resolve from an inline -// map, a mounted secret directory, or the process environment. +// The credential source is selected by the required `source` field: +// - "gateway": the OpenShell gateway (fetches the sandbox's resolved +// provider environment; requires running as a sidecar in the sandbox pod). +// - "secret_dir": a mounted directory, one file per KEY. +// +// The resolution + safe header-injection primitives live in authlib/credinject +// so a future host-keyed injector can reuse them. package placeholderresolve import ( @@ -24,28 +27,19 @@ import ( "encoding/json" "errors" "fmt" - "os" - "path/filepath" - "regexp" "strings" - "github.com/kagenti/kagenti-extensions/authbridge/authlib/config" + "github.com/kagenti/kagenti-extensions/authbridge/authlib/credinject" "github.com/kagenti/kagenti-extensions/authbridge/authlib/pipeline" "github.com/kagenti/kagenti-extensions/authbridge/authlib/plugins" "github.com/kagenti/kagenti-extensions/authbridge/authlib/plugins/placeholderresolve/gateway" ) -// defaultPrefix is the OpenShell placeholder prefix. Kept identical to the +// defaultPrefix is the OpenShell placeholder prefix — kept identical to the // supervisor's so agents are unchanged when AuthBridge replaces OpenShell's // internal proxy (OpenShell secrets.rs PLACEHOLDER_PREFIX). const defaultPrefix = "openshell:resolve:env:" -// envKeyPattern is OpenShell's env-key grammar: a leading letter/underscore -// followed by letters, digits, or underscores. It bounds the KEY captured -// after the prefix and (because it admits no '/' or '.') also makes the -// file-source path join safe from traversal. -const envKeyPattern = `([A-Za-z_][A-Za-z0-9_]*)` - // Default OpenShell sidecar paths for the gateway source (match the k8s // driver's pod-spec conventions). const ( @@ -53,32 +47,29 @@ const ( defaultSATokenPath = "/var/run/secrets/openshell/token" ) -// placeholderResolveConfig is the plugin's local config schema. Field tags -// drive both runtime decoding (json) and operator-facing schema -// introspection (description / default). +// Credential source discriminators for the `source` config field. +const ( + sourceGateway = "gateway" + sourceSecretDir = "secret_dir" +) + +// placeholderResolveConfig is the plugin's config schema. Field tags drive both +// runtime decoding (json) and operator-facing schema introspection. type placeholderResolveConfig struct { - // Prefix is the placeholder prefix to match. The KEY following it is - // matched against the env-key grammar. + // Prefix is the placeholder prefix to match; the KEY following it is + // matched against the OpenShell env-key grammar. Prefix string `json:"prefix" description:"Placeholder prefix to match before the env key." default:"openshell:resolve:env:"` - // Headers lists the request headers to scan and rewrite. Defaults to - // Authorization (the only header the forward proxy propagates to the - // upstream wire today). - Headers []string `json:"headers" description:"Request headers to scan for placeholders." default:"Authorization"` - - // Gateway, when set, resolves each KEY from the OpenShell gateway's - // resolved provider environment (the native-provider source). Takes - // precedence over mappings/secret_dir/env. Requires running as a sidecar - // in the sandbox pod (shared SA + sandbox-id annotation). - Gateway *gatewayConfig `json:"gateway" description:"Resolve credentials from the OpenShell gateway (sandbox sidecar)."` + // Source selects where credentials are resolved from. Required; there is + // no implicit fallback — an unconfigured source fails closed. + Source string `json:"source" required:"true" description:"Credential source: 'gateway' (OpenShell sandbox sidecar) or 'secret_dir' (mounted files)."` - // Mappings is an optional inline KEY->value map used as the resolver - // source. Intended for isolation testing. - Mappings map[string]string `json:"mappings" description:"Inline KEY->value resolver source (isolation testing)."` + // Gateway configures the OpenShell-gateway source (source: gateway). + Gateway *gatewayConfig `json:"gateway" description:"OpenShell gateway source config (used when source=gateway)."` - // SecretDir, when set, resolves each KEY by reading / as - // a credential file. - SecretDir string `json:"secret_dir" description:"Directory to resolve each KEY from a file named ."` + // SecretDir is the directory for the file source (source: secret_dir): + // each KEY is read from /. + SecretDir string `json:"secret_dir" description:"Directory to resolve each KEY from a file named (used when source=secret_dir)."` } // gatewayConfig configures the OpenShell-gateway resolver source. @@ -94,10 +85,7 @@ func (c *placeholderResolveConfig) applyDefaults() { if c.Prefix == "" { c.Prefix = defaultPrefix } - if len(c.Headers) == 0 { - c.Headers = []string{"Authorization"} - } - if c.Gateway != nil { + if c.Source == sourceGateway && c.Gateway != nil { if c.Gateway.MTLSCertDir == "" { c.Gateway.MTLSCertDir = defaultMTLSCertDir } @@ -111,73 +99,34 @@ func (c *placeholderResolveConfig) validate() error { if c.Prefix == "" { return errors.New("prefix must not be empty") } - if len(c.Headers) == 0 { - return errors.New("headers must list at least one header") - } - if c.Gateway != nil { + switch c.Source { + case sourceGateway: + if c.Gateway == nil { + return errors.New("source 'gateway' requires a gateway block") + } if c.Gateway.Endpoint == "" { return errors.New("gateway.endpoint is required") } if c.Gateway.SandboxID == "" { return errors.New("gateway.sandbox_id is required") } + case sourceSecretDir: + if c.SecretDir == "" { + return errors.New("source 'secret_dir' requires secret_dir") + } + case "": + return errors.New("source is required (one of: gateway, secret_dir)") + default: + return fmt.Errorf("unknown source %q (want one of: gateway, secret_dir)", c.Source) } return nil } -// Resolver maps a placeholder KEY to its real secret value. ok is false -// when the key is unknown — the plugin fails closed on a false. -type Resolver interface { - Resolve(ctx context.Context, key string) (string, bool) -} - -// lifecycleResolver is implemented by resolvers needing background warm-up -// (the gateway source). The map/file/env resolvers do not implement it and -// are treated as always-ready. -type lifecycleResolver interface { - Start(ctx context.Context) error - Ready() bool - Stop(ctx context.Context) error -} - -// mapResolver resolves from an inline map (isolation testing). -type mapResolver map[string]string - -func (m mapResolver) Resolve(_ context.Context, key string) (string, bool) { - v, ok := m[key] - return v, ok -} - -// envResolver resolves from the process environment. An unset or empty var -// is treated as unresolved (fail closed). -type envResolver struct{} - -func (envResolver) Resolve(_ context.Context, key string) (string, bool) { - v, ok := os.LookupEnv(key) - if !ok || v == "" { - return "", false - } - return v, true -} - -// fileResolver resolves each KEY by reading /. The env-key grammar -// forbids '/' and '.' so the join cannot escape dir. -type fileResolver struct{ dir string } - -func (f fileResolver) Resolve(_ context.Context, key string) (string, bool) { - v, err := config.ReadCredentialFile(filepath.Join(f.dir, key)) - if err != nil { - return "", false - } - return v, true -} - -// PlaceholderResolve is the outbound plugin. cfg/re/resolver are immutable -// after Configure returns, so OnRequest reads them without synchronization. +// PlaceholderResolve is the outbound plugin. cfg/resolver are immutable after +// Configure returns, so OnRequest reads them without synchronization. type PlaceholderResolve struct { cfg placeholderResolveConfig - re *regexp.Regexp - resolver Resolver + resolver credinject.Resolver } // New constructs an unconfigured plugin. @@ -191,7 +140,7 @@ func (p *PlaceholderResolve) Name() string { return "placeholder-resolve" } func (p *PlaceholderResolve) Capabilities() pipeline.PluginCapabilities { return pipeline.PluginCapabilities{ - Description: "Resolve OpenShell credential placeholders in headers to real values (fail-closed).", + Description: "Resolve OpenShell credential placeholders in the Authorization header to real values (fail-closed).", } } @@ -215,16 +164,9 @@ func (p *PlaceholderResolve) Configure(raw json.RawMessage) error { return fmt.Errorf("placeholder-resolve config: %w", err) } - re, err := regexp.Compile(regexp.QuoteMeta(c.Prefix) + envKeyPattern) - if err != nil { - return fmt.Errorf("placeholder-resolve: compile prefix pattern: %w", err) - } - - // Pick the resolver source. The gateway (native OpenShell provider) wins; - // then inline mappings, a mounted secret dir, else the process env. - var resolver Resolver - switch { - case c.Gateway != nil: + var resolver credinject.Resolver + switch c.Source { + case sourceGateway: gw, gerr := gateway.New(gateway.Config{ Endpoint: c.Gateway.Endpoint, MTLSCertDir: c.Gateway.MTLSCertDir, @@ -236,36 +178,30 @@ func (p *PlaceholderResolve) Configure(raw json.RawMessage) error { return fmt.Errorf("placeholder-resolve: gateway resolver: %w", gerr) } resolver = gw - case len(c.Mappings) > 0: - resolver = mapResolver(c.Mappings) - case c.SecretDir != "": - resolver = fileResolver{dir: c.SecretDir} - default: - resolver = envResolver{} + case sourceSecretDir: + resolver = credinject.FileResolver{Dir: c.SecretDir} } // Commit only after all fallible construction succeeded, so a failed - // Configure leaves the plugin in its zero state. + // Configure leaves the plugin in its zero (deny) state. p.cfg = c - p.re = re p.resolver = resolver return nil } // Init starts a lifecycle-capable resolver's background warm-up (the gateway -// source). It returns promptly; readiness is reported via Ready so the -// pipeline gates traffic until the credential cache is primed. +// source). Static resolvers are always ready. func (p *PlaceholderResolve) Init(ctx context.Context) error { - if lr, ok := p.resolver.(lifecycleResolver); ok { + if lr, ok := p.resolver.(credinject.LifecycleResolver); ok { return lr.Start(ctx) } return nil } -// Ready reports resolver readiness — always true for the static -// (map/file/env) resolvers; gated on the primed cache for the gateway source. +// Ready reports resolver readiness — always true for the static (file) +// resolver; gated on the primed cache for the gateway source. func (p *PlaceholderResolve) Ready() bool { - if lr, ok := p.resolver.(lifecycleResolver); ok { + if lr, ok := p.resolver.(credinject.LifecycleResolver); ok { return lr.Ready() } return true @@ -273,42 +209,41 @@ func (p *PlaceholderResolve) Ready() bool { // Shutdown stops a lifecycle-capable resolver's background work. func (p *PlaceholderResolve) Shutdown(ctx context.Context) error { - if lr, ok := p.resolver.(lifecycleResolver); ok { + if lr, ok := p.resolver.(credinject.LifecycleResolver); ok { return lr.Stop(ctx) } return nil } +// authHeader is the only header any listener reconciles to the upstream wire, +// so it is the only header this plugin scans and rewrites. +const authHeader = "Authorization" + func (p *PlaceholderResolve) OnRequest(ctx context.Context, pctx *pipeline.Context) pipeline.Action { - if p.resolver == nil || p.re == nil { + if p.resolver == nil { return pipeline.DenyStatus(503, "upstream.unreachable", "placeholder-resolve not configured") } - anyResolved := false - for _, h := range p.cfg.Headers { - val := pctx.Headers.Get(h) - if val == "" { - continue - } - rewritten, found, ok := p.rewrite(ctx, val) - if !found { - continue - } - if !ok { - // A placeholder was present but could not be resolved (unknown - // key or invalid value). Fail closed — never forward the - // placeholder to the upstream. - return pctx.DenyAndRecord("placeholder_unresolved", "auth.unauthorized", "unresolvable credential placeholder") - } - pctx.Headers.Set(h, rewritten) - anyResolved = true + val := pctx.Headers.Get(authHeader) + if val == "" { + pctx.Skip("no_authorization") + return pipeline.Action{Type: pipeline.Continue} } - if anyResolved { - pctx.Modify("placeholder_resolved") - } else { + rewritten, found, ok := p.rewrite(ctx, val) + if !found { pctx.Skip("no_placeholder") + return pipeline.Action{Type: pipeline.Continue} + } + if !ok { + // A placeholder was present but could not be resolved (unknown key or + // unsafe value). Fail closed — never forward the placeholder upstream. + return pctx.DenyAndRecord("placeholder_unresolved", "auth.unauthorized", "unresolvable credential placeholder") + } + if !credinject.SafeSetHeader(pctx.Headers, authHeader, rewritten) { + return pctx.DenyAndRecord("placeholder_unsafe", "auth.unauthorized", "unsafe resolved credential value") } + pctx.Modify("placeholder_resolved") return pipeline.Action{Type: pipeline.Continue} } @@ -316,33 +251,60 @@ func (p *PlaceholderResolve) OnResponse(_ context.Context, _ *pipeline.Context) return pipeline.Action{Type: pipeline.Continue} } -// rewrite replaces every placeholder in val with its resolved value. -// Returns (rewritten, found, ok): found is true when at least one -// placeholder matched; ok is false when a matched placeholder could not be -// resolved or resolved to an unsafe value (caller must fail closed). +// rewrite replaces every placeholder (prefix + an OpenShell env key matching +// [A-Za-z_][A-Za-z0-9_]*) in val with its resolved value. Returns +// (rewritten, found, ok): found is true when at least one placeholder matched; +// ok is false when a matched placeholder could not be resolved or resolved to +// an unsafe value (the caller must fail closed). A prefix not followed by a +// valid key is left literal. func (p *PlaceholderResolve) rewrite(ctx context.Context, val string) (string, bool, bool) { + if !strings.Contains(val, p.cfg.Prefix) { + return val, false, true + } found := false - failed := false - out := p.re.ReplaceAllStringFunc(val, func(match string) string { + var b strings.Builder + i := 0 + for { + rel := strings.Index(val[i:], p.cfg.Prefix) + if rel < 0 { + b.WriteString(val[i:]) + break + } + start := i + rel + b.WriteString(val[i:start]) // text before the prefix + keyStart := start + len(p.cfg.Prefix) + keyEnd := keyStart + for keyEnd < len(val) && isEnvKeyChar(val[keyEnd], keyEnd == keyStart) { + keyEnd++ + } + if keyEnd == keyStart { + // Prefix not followed by a valid env key — leave it literal. + b.WriteString(p.cfg.Prefix) + i = keyStart + continue + } found = true - key := strings.TrimPrefix(match, p.cfg.Prefix) - v, ok := p.resolver.Resolve(ctx, key) - if !ok || !validResolved(v) { - failed = true - return match // unchanged; the request is denied regardless + v, okv := p.resolver.Resolve(ctx, val[keyStart:keyEnd]) + if !okv || !credinject.SafeHeaderValue(v) { + return "", true, false // fail closed; the request is denied } - return v - }) - if failed { - return "", true, false + b.WriteString(v) + i = keyEnd } - return out, found, true + return b.String(), found, true } -// validResolved rejects resolved values carrying CR, LF, or NUL — guarding -// against header-injection (CWE-113) via a poisoned credential store. -func validResolved(s string) bool { - return !strings.ContainsAny(s, "\r\n\x00") +// isEnvKeyChar reports whether c is allowed in an OpenShell env key. The first +// character must be a letter or underscore; later characters may also be digits. +func isEnvKeyChar(c byte, first bool) bool { + switch { + case c == '_', c >= 'A' && c <= 'Z', c >= 'a' && c <= 'z': + return true + case !first && c >= '0' && c <= '9': + return true + default: + return false + } } // Compile-time interface checks. diff --git a/authbridge/authlib/plugins/placeholderresolve/plugin_test.go b/authbridge/authlib/plugins/placeholderresolve/plugin_test.go index 740e9a5b6..b1f49aa16 100644 --- a/authbridge/authlib/plugins/placeholderresolve/plugin_test.go +++ b/authbridge/authlib/plugins/placeholderresolve/plugin_test.go @@ -4,35 +4,84 @@ import ( "context" "encoding/json" "net/http" + "os" + "path/filepath" "testing" + "github.com/kagenti/kagenti-extensions/authbridge/authlib/credinject" "github.com/kagenti/kagenti-extensions/authbridge/authlib/pipeline" ) -// configure builds and configures a plugin with the given inline mappings. -func configure(t *testing.T, mappings map[string]string) *PlaceholderResolve { +// withMap returns a plugin whose resolver is an inline map, to exercise +// OnRequest/rewrite without a real credential source (the production sources +// are gateway/secret_dir; the map is a test seam). +func withMap(t *testing.T, mappings map[string]string) *PlaceholderResolve { t.Helper() - raw, err := json.Marshal(map[string]any{"mappings": mappings}) + p := New() + p.cfg.Prefix = defaultPrefix + p.resolver = credinject.MapResolver(mappings) + return p +} + +func configureJSON(t *testing.T, cfg map[string]any) (*PlaceholderResolve, error) { + t.Helper() + raw, err := json.Marshal(cfg) if err != nil { t.Fatalf("marshal config: %v", err) } p := New() - if err := p.Configure(raw); err != nil { - t.Fatalf("Configure: %v", err) - } - return p + return p, p.Configure(raw) } -func TestConfigureDefaults(t *testing.T) { - p := configure(t, map[string]string{"FOO": "x"}) +func TestConfigureSecretDirDefaults(t *testing.T) { + p, err := configureJSON(t, map[string]any{"source": "secret_dir", "secret_dir": t.TempDir()}) + if err != nil { + t.Fatalf("Configure: %v", err) + } if p.cfg.Prefix != defaultPrefix { t.Errorf("prefix default = %q, want %q", p.cfg.Prefix, defaultPrefix) } - if len(p.cfg.Headers) != 1 || p.cfg.Headers[0] != "Authorization" { - t.Errorf("headers default = %v, want [Authorization]", p.cfg.Headers) +} + +func TestConfigureRequiresSource(t *testing.T) { + if _, err := configureJSON(t, map[string]any{"secret_dir": "/tmp"}); err == nil { + t.Fatal("expected error: source is required") + } +} + +func TestConfigureRejectsUnknownSource(t *testing.T) { + // `env` was removed as a source (it let the agent read the proxy's env); + // an unknown source must be rejected, not silently honored. + if _, err := configureJSON(t, map[string]any{"source": "env"}); err == nil { + t.Fatal("expected error for unknown source 'env'") } } +func TestConfigureGatewayRequiresBlock(t *testing.T) { + if _, err := configureJSON(t, map[string]any{"source": "gateway"}); err == nil { + t.Fatal("expected error: gateway source requires a gateway block") + } +} + +func TestConfigureGatewayDefaults(t *testing.T) { + // Loopback http endpoint so Dial succeeds without certs; we only assert + // that the gateway sub-defaults are applied. + p, err := configureJSON(t, map[string]any{ + "source": "gateway", + "gateway": map[string]any{ + "endpoint": "http://127.0.0.1:8080", + "sandbox_id": "sb-1", + }, + }) + if err != nil { + t.Fatalf("Configure: %v", err) + } + if p.cfg.Gateway.MTLSCertDir != defaultMTLSCertDir || p.cfg.Gateway.SATokenPath != defaultSATokenPath { + t.Errorf("gateway defaults not applied: %+v", p.cfg.Gateway) + } + _ = p.Shutdown(context.Background()) +} + func TestOnRequest(t *testing.T) { mappings := map[string]string{ "ANTHROPIC_AUTH_TOKEN": "sk-real-token", @@ -43,33 +92,17 @@ func TestOnRequest(t *testing.T) { name string authIn string wantReject bool - wantAuth string // expected Authorization after OnRequest (when not rejected) + wantAuth string }{ - { - name: "resolves placeholder bearer", - authIn: "Bearer openshell:resolve:env:ANTHROPIC_AUTH_TOKEN", - wantAuth: "Bearer sk-real-token", - }, - { - name: "non-placeholder passthrough", - authIn: "Bearer already-a-real-token", - wantAuth: "Bearer already-a-real-token", - }, - { - name: "unknown key fails closed", - authIn: "Bearer openshell:resolve:env:UNKNOWN", - wantReject: true, - }, - { - name: "invalid resolved value fails closed", - authIn: "Bearer openshell:resolve:env:BAD", - wantReject: true, - }, + {"resolves placeholder bearer", "Bearer openshell:resolve:env:ANTHROPIC_AUTH_TOKEN", false, "Bearer sk-real-token"}, + {"non-placeholder passthrough", "Bearer already-a-real-token", false, "Bearer already-a-real-token"}, + {"unknown key fails closed", "Bearer openshell:resolve:env:UNKNOWN", true, ""}, + {"invalid resolved value fails closed", "Bearer openshell:resolve:env:BAD", true, ""}, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - p := configure(t, mappings) + p := withMap(t, mappings) pctx := &pipeline.Context{Direction: pipeline.Outbound, Headers: http.Header{}} pctx.Headers.Set("Authorization", tc.authIn) @@ -82,9 +115,6 @@ func TestOnRequest(t *testing.T) { if act.Violation == nil || act.Violation.Code != "auth.unauthorized" { t.Errorf("expected auth.unauthorized violation, got %+v", act.Violation) } - // The placeholder must never be forwarded — on a reject the - // caller drops the request, so the resolved value is moot, - // but assert we did not leak a real secret either. if got := pctx.Headers.Get("Authorization"); got == "Bearer sk-real-token" { t.Errorf("rejected request unexpectedly carries a resolved secret") } @@ -102,7 +132,7 @@ func TestOnRequest(t *testing.T) { } func TestRewriteMultiplePlaceholders(t *testing.T) { - p := configure(t, map[string]string{"A": "1", "B": "2"}) + p := withMap(t, map[string]string{"A": "1", "B": "2"}) got, found, ok := p.rewrite(context.Background(), "openshell:resolve:env:A and openshell:resolve:env:B") if !found || !ok { @@ -113,12 +143,42 @@ func TestRewriteMultiplePlaceholders(t *testing.T) { } } +func TestRewritePrefixWithoutValidKey(t *testing.T) { + // A prefix not followed by a valid env key is left literal (no match). + p := withMap(t, map[string]string{}) + got, found, ok := p.rewrite(context.Background(), "openshell:resolve:env:123") + if found || !ok { + t.Fatalf("found=%v ok=%v, want found=false ok=true", found, ok) + } + if got != "openshell:resolve:env:123" { + t.Errorf("rewrite = %q, want the input unchanged", got) + } +} + +func TestSecretDirResolvesEndToEnd(t *testing.T) { + dir := t.TempDir() + if err := os.WriteFile(filepath.Join(dir, "ANTHROPIC_AUTH_TOKEN"), []byte("sk-real\n"), 0o600); err != nil { + t.Fatal(err) + } + p, err := configureJSON(t, map[string]any{"source": "secret_dir", "secret_dir": dir}) + if err != nil { + t.Fatalf("Configure: %v", err) + } + pctx := &pipeline.Context{Direction: pipeline.Outbound, Headers: http.Header{}} + pctx.Headers.Set("Authorization", "Bearer openshell:resolve:env:ANTHROPIC_AUTH_TOKEN") + if act := p.OnRequest(context.Background(), pctx); act.Type != pipeline.Continue { + t.Fatalf("expected Continue, got %v", act.Type) + } + if got := pctx.Headers.Get("Authorization"); got != "Bearer sk-real" { + t.Errorf("Authorization = %q, want Bearer sk-real", got) + } +} + func TestUnconfiguredFailsClosed(t *testing.T) { p := New() // never Configured pctx := &pipeline.Context{Direction: pipeline.Outbound, Headers: http.Header{}} pctx.Headers.Set("Authorization", "Bearer openshell:resolve:env:FOO") - act := p.OnRequest(context.Background(), pctx) - if act.Type != pipeline.Reject { + if act := p.OnRequest(context.Background(), pctx); act.Type != pipeline.Reject { t.Fatalf("unconfigured plugin must reject, got %v", act.Type) } } diff --git a/authbridge/docs/plugin-reference.md b/authbridge/docs/plugin-reference.md index d4a7cdd20..24b2ac615 100644 --- a/authbridge/docs/plugin-reference.md +++ b/authbridge/docs/plugin-reference.md @@ -106,41 +106,41 @@ error) rather than silently forwarding the real token. ### `placeholder-resolve`: credential injection config Resolves OpenShell credential placeholders (`openshell:resolve:env:`) in the -configured request headers to their real values on the wire, fail-closed. Outbound -plugin; opt-in (inert unless listed in `pipeline.outbound`). +**`Authorization`** header to their real values on the wire, fail-closed. Outbound +plugin; opt-in (inert unless listed in `pipeline.outbound`). Only `Authorization` is +rewritten — it is the only header any listener reconciles to the upstream wire. ```yaml - name: placeholder-resolve config: prefix: "openshell:resolve:env:" # default; the KEY follows this prefix - headers: ["Authorization"] # default; headers to scan + rewrite - # Resolver source — precedence: gateway > mappings > secret_dir > process env. - gateway: # native OpenShell-provider source (sidecar) + source: gateway # REQUIRED: "gateway" or "secret_dir" (no default — fail-closed) + gateway: # used when source: gateway (OpenShell sandbox sidecar) endpoint: "https://openshell..svc:8080" # required sandbox_id: "" # required; must match the minted JWT mtls_cert_dir: "/etc/openshell-tls/client" # ca.crt/tls.crt/tls.key; required for https sa_token_path: "/var/run/secrets/openshell/token" - insecure: false # opt-in plaintext (non-loopback refused by default) - mappings: {} # inline KEY->value (testing) - secret_dir: "" # resolve KEY from / (mounted secret) + insecure: false # opt-in plaintext (non-loopback refused by default) + # secret_dir: "/etc/authbridge-cred" # used when source: secret_dir (file per KEY) ``` | Field | Default | Notes | |-------|---------|-------| | `prefix` | `openshell:resolve:env:` | Placeholder prefix; the KEY after it matches `[A-Za-z_][A-Za-z0-9_]*`. | -| `headers` | `[Authorization]` | `Authorization` is the only header the forward proxy propagates to the upstream wire today. | -| `gateway.endpoint` | — (required if `gateway` set) | Gateway gRPC endpoint; `https://` selects mTLS, `http://` plaintext. | -| `gateway.sandbox_id` | — (required if `gateway` set) | Must equal the id bound into the gateway-minted JWT. | +| `source` | — (**required**) | `gateway` or `secret_dir`. There is **no implicit fallback** — an unset/unknown source fails `Configure`, so the plugin never silently resolves from an ambient source. | +| `gateway.endpoint` | — (required for `gateway`) | Gateway gRPC endpoint; `https://` selects mTLS, `http://` plaintext (non-loopback refused unless `insecure`). | +| `gateway.sandbox_id` | — (required for `gateway`) | Must equal the id bound into the gateway-minted JWT. | | `gateway.mtls_cert_dir` | `/etc/openshell-tls/client` | `ca.crt`/`tls.crt`/`tls.key`; required for an `https://` endpoint. | | `gateway.sa_token_path` | `/var/run/secrets/openshell/token` | Projected SA token (audience `openshell-gateway`). | -| `gateway.insecure` | `false` | Permit plaintext gRPC to a **non-loopback** gateway. Plaintext sends the SA token + JWT in cleartext, so it is **refused by default** for non-loopback endpoints (loopback is always allowed). | -| `mappings` / `secret_dir` | — | Static resolver sources (inline map for testing / mounted secret directory). | +| `gateway.insecure` | `false` | Permit plaintext gRPC to a **non-loopback** gateway. Plaintext sends the SA token + JWT in cleartext, so it is **refused by default** for non-loopback endpoints (loopback always allowed). | +| `secret_dir` | — (required for `secret_dir`) | Directory; each KEY is read from a path-contained `/` file. | An unresolved placeholder, or a resolved value containing CR/LF/NUL, fails the request closed (`401 auth.unauthorized`) — the placeholder is never forwarded. The `gateway` source requires running as a sidecar in the sandbox pod (shared SA + the `openshell.io/sandbox-id` annotation) and warms its cache in the background, so the -pipeline gates traffic on `Ready()` until the first fetch succeeds. +pipeline gates traffic on `Ready()` until the first fetch succeeds. (The shared +resolution + safe-header-injection primitives live in `authlib/credinject`.) ## `on_error` policy From c1acaa5f82fe2a9b5eb611ef28b4b4dc06c6b662 Mon Sep 17 00:00:00 2001 From: Hai Huang Date: Fri, 26 Jun 2026 16:21:57 -0400 Subject: [PATCH 5/9] fix(authbridge): gateway resolver strips v_ placeholder prefix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit OpenShell injects provider-env placeholders as openshell:resolve:env:v_ (secrets.rs: format!("{PREFIX}v{revision}_{key}")), but GetSandboxProviderEnvironment returns the environment keyed by the bare . The gateway resolver did a byte-for-byte map lookup with no revision handling, so every revision-keyed placeholder missed and the plugin failed closed (401) — making source: gateway unusable in practice (only source: secret_dir worked, by naming the mounted file with the revision prefix). Resolve now tries the literal key first (covers revision==0 / bare placeholders and any real var literally shaped like a prefix), then strips a single leading "v_" and retries the bare name. Expiry is checked against the resolved (bare) key. Fail-closed semantics are preserved. Add TestResolveStripsRevisionPrefix: revision-keyed → bare value, bare still works, absent/non-numeric/empty-revision fail closed, literal precedence, expiry on the stripped key, cold cache. Assisted-By: Claude (Anthropic AI) Signed-off-by: Hai Huang --- .../placeholderresolve/gateway/resolver.go | 42 ++++++++++-- .../gateway/resolver_test.go | 67 +++++++++++++++++++ 2 files changed, 105 insertions(+), 4 deletions(-) diff --git a/authbridge/authlib/plugins/placeholderresolve/gateway/resolver.go b/authbridge/authlib/plugins/placeholderresolve/gateway/resolver.go index 29b200c50..2cfe0bcb3 100644 --- a/authbridge/authlib/plugins/placeholderresolve/gateway/resolver.go +++ b/authbridge/authlib/plugins/placeholderresolve/gateway/resolver.go @@ -66,19 +66,53 @@ func New(cfg Config) (*Resolver, error) { // Resolve returns the real value for an env-var key from the cached // environment. It fails closed (ok=false) when the cache is not yet primed, // the key is absent, or the key's credential has expired. +// +// OpenShell prepends a "v_" revision prefix to provider-env placeholders +// (secrets.rs: format!("{PREFIX}v{revision}_{key}")), but the gateway returns +// the environment keyed by the bare name. So if the literal key misses, strip a +// single revision prefix and retry the bare name. The literal lookup runs first, +// so a real env var named like "v2_FOO" still resolves to itself. func (r *Resolver) Resolve(_ context.Context, key string) (string, bool) { env := r.env.Load() if env == nil { return "", false } - v, ok := env.Values[key] - if !ok { + lookup := func(k string) (string, bool) { + v, ok := env.Values[k] + if !ok { + return "", false + } + if exp, has := env.ExpiresAtMs[k]; has && exp > 0 && time.Now().UnixMilli() >= exp { + return "", false + } + return v, true + } + if v, ok := lookup(key); ok { + return v, true + } + if bare, stripped := stripRevisionPrefix(key); stripped { + return lookup(bare) + } + return "", false +} + +// stripRevisionPrefix removes a single leading "v_" revision prefix +// (the form OpenShell prepends to provider-env placeholders). It returns the +// bare key and true only when the prefix is well-formed — a "v", at least one +// decimal digit, an underscore, and a non-empty remainder — otherwise "" and +// false. +func stripRevisionPrefix(key string) (string, bool) { + if len(key) < 3 || key[0] != 'v' { return "", false } - if exp, has := env.ExpiresAtMs[key]; has && exp > 0 && time.Now().UnixMilli() >= exp { + i := 1 + for i < len(key) && key[i] >= '0' && key[i] <= '9' { + i++ + } + if i == 1 || i >= len(key) || key[i] != '_' || i+1 >= len(key) { return "", false } - return v, true + return key[i+1:], true } // Start launches the refresh loop on a process-lifetime context (so it diff --git a/authbridge/authlib/plugins/placeholderresolve/gateway/resolver_test.go b/authbridge/authlib/plugins/placeholderresolve/gateway/resolver_test.go index 9435501ac..8407b2941 100644 --- a/authbridge/authlib/plugins/placeholderresolve/gateway/resolver_test.go +++ b/authbridge/authlib/plugins/placeholderresolve/gateway/resolver_test.go @@ -4,6 +4,7 @@ package gateway import ( + "context" "testing" "time" @@ -44,3 +45,69 @@ func TestNextDelay(t *testing.T) { t.Errorf("far-expiry nextDelay = %v, want %v (clamped to refresh)", d, r.refreshInterval) } } + +// TestResolveStripsRevisionPrefix covers the bug fix: OpenShell injects +// "v_" placeholders but the gateway returns the env keyed by the bare +// , so the resolver must strip a single "v_" prefix on a miss. +func TestResolveStripsRevisionPrefix(t *testing.T) { + ctx := context.Background() + + // Primed cache keyed by the bare name (as the gateway returns it). + r := &Resolver{} + r.env.Store(&openshell.Environment{ + Values: map[string]string{"ANTHROPIC_AUTH_TOKEN": "sk-real"}, + }) + + cases := []struct { + name string + key string + want string + wantOK bool + }{ + {"revision-keyed resolves to bare value", "v7_ANTHROPIC_AUTH_TOKEN", "sk-real", true}, + {"bare key still resolves", "ANTHROPIC_AUTH_TOKEN", "sk-real", true}, + {"revision-keyed but absent fails closed", "v7_MISSING", "", false}, + {"non-numeric revision is not stripped", "vX_ANTHROPIC_AUTH_TOKEN", "", false}, + {"empty revision is not stripped", "v_ANTHROPIC_AUTH_TOKEN", "", false}, + {"empty key after prefix fails closed", "v7_", "", false}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got, ok := r.Resolve(ctx, tc.key) + if got != tc.want || ok != tc.wantOK { + t.Errorf("Resolve(%q) = (%q, %v), want (%q, %v)", tc.key, got, ok, tc.want, tc.wantOK) + } + }) + } + + // A literal key matching the revision shape wins over the bare fallback. + t.Run("literal key takes precedence over strip", func(t *testing.T) { + lit := &Resolver{} + lit.env.Store(&openshell.Environment{ + Values: map[string]string{"v2_FOO": "literal", "FOO": "bare"}, + }) + if got, ok := lit.Resolve(ctx, "v2_FOO"); got != "literal" || !ok { + t.Errorf("Resolve(v2_FOO) = (%q, %v), want (literal, true)", got, ok) + } + }) + + // Expiry is checked against the bare key on the stripped path. + t.Run("expired credential on stripped key fails closed", func(t *testing.T) { + exp := &Resolver{} + exp.env.Store(&openshell.Environment{ + Values: map[string]string{"TOK": "x"}, + ExpiresAtMs: map[string]int64{"TOK": time.Now().Add(-time.Minute).UnixMilli()}, + }) + if got, ok := exp.Resolve(ctx, "v3_TOK"); ok { + t.Errorf("Resolve(v3_TOK) = (%q, %v), want fail-closed", got, ok) + } + }) + + // Cold cache fails closed regardless of key shape. + t.Run("cold cache fails closed", func(t *testing.T) { + cold := &Resolver{} + if got, ok := cold.Resolve(ctx, "v1_ANTHROPIC_AUTH_TOKEN"); ok { + t.Errorf("Resolve on cold cache = (%q, %v), want fail-closed", got, ok) + } + }) +} From 0b7ed76f9a083eb8597646bb733e669784193ca0 Mon Sep 17 00:00:00 2001 From: Hai Huang Date: Fri, 26 Jun 2026 17:50:18 -0400 Subject: [PATCH 6/9] feat(authbridge): gateway resolver takes independent mtls_cert/mtls_key/mtls_ca paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit OpenShell splits the gateway TLS material — client cert/key in one secret (OPENSHELL_TLS_CERT/KEY) and the CA in another (OPENSHELL_TLS_CA) — but the gateway client loaded ca.crt/tls.crt/tls.key from a single mtls_cert_dir, forcing consumers to recombine the split material into one dir. Replace mtls_cert_dir with three independent file paths (mtls_cert/mtls_key/mtls_ca) mirroring OpenShell's three env vars, so a sidecar/webhook can point straight at the driver-injected /tls/client + /tls/ca mounts (PR unmerged — no back-compat shim). - openshell.Config: MTLSCert/MTLSKey/MTLSCA; mtlsConfig(cert,key,ca,serverName) - gateway.Config + plugin gatewayConfig: mtls_cert/mtls_key/mtls_ca (dropped the mtls_cert_dir default) - tests + plugin-reference.md updated Assisted-By: Claude (Anthropic AI) Signed-off-by: Hai Huang --- authbridge/authlib/openshell/client.go | 44 ++++++++++--------- authbridge/authlib/openshell/client_test.go | 10 +++-- .../placeholderresolve/gateway/resolver.go | 8 +++- .../plugins/placeholderresolve/plugin.go | 12 ++--- .../plugins/placeholderresolve/plugin_test.go | 4 +- authbridge/docs/plugin-reference.md | 6 ++- 6 files changed, 48 insertions(+), 36 deletions(-) diff --git a/authbridge/authlib/openshell/client.go b/authbridge/authlib/openshell/client.go index bd7a99594..30b0ef785 100644 --- a/authbridge/authlib/openshell/client.go +++ b/authbridge/authlib/openshell/client.go @@ -22,7 +22,6 @@ import ( "log/slog" "net" "os" - "path/filepath" "strings" "sync" "time" @@ -48,10 +47,14 @@ type Config struct { // Endpoint is the gateway gRPC endpoint (OPENSHELL_ENDPOINT). An // https:// scheme selects mTLS; http:// is plaintext. Endpoint string - // MTLSCertDir holds ca.crt + tls.crt + tls.key for mTLS (the - // openshell-client-tls secret mount, e.g. /etc/openshell-tls/client). - // Required when Endpoint is https://. - MTLSCertDir string + // MTLSCert, MTLSKey, MTLSCA are the client cert, key, and CA file paths for + // mTLS — independent paths mirroring OpenShell's OPENSHELL_TLS_CERT / + // OPENSHELL_TLS_KEY / OPENSHELL_TLS_CA (the driver splits the CA into a + // separate secret/dir from the client keypair). All three are required when + // Endpoint is https://. + MTLSCert string + MTLSKey string + MTLSCA string // SATokenPath is the projected SA token file (OPENSHELL_K8S_SA_TOKEN_FILE, // e.g. /var/run/secrets/openshell/token). SATokenPath string @@ -77,8 +80,8 @@ func (c Config) validate() error { if c.SandboxID == "" { return fmt.Errorf("openshell: sandbox_id is required") } - if strings.HasPrefix(c.Endpoint, "https://") && c.MTLSCertDir == "" { - return fmt.Errorf("openshell: mtls_cert_dir is required for an https endpoint") + if strings.HasPrefix(c.Endpoint, "https://") && (c.MTLSCert == "" || c.MTLSKey == "" || c.MTLSCA == "") { + return fmt.Errorf("openshell: mtls_cert, mtls_key, and mtls_ca are required for an https endpoint") } return nil } @@ -118,7 +121,7 @@ func Dial(cfg Config) (*Client, error) { switch { case strings.HasPrefix(cfg.Endpoint, "https://"): target = strings.TrimPrefix(cfg.Endpoint, "https://") - tlsCfg, err := mtlsConfig(cfg.MTLSCertDir, hostOnly(target)) + tlsCfg, err := mtlsConfig(cfg.MTLSCert, cfg.MTLSKey, cfg.MTLSCA, hostOnly(target)) if err != nil { return nil, err } @@ -127,9 +130,9 @@ func Dial(cfg Config) (*Client, error) { target = strings.TrimPrefix(cfg.Endpoint, "http://") plaintext = true default: - // No scheme: mTLS when a cert dir is configured, else plaintext. - if cfg.MTLSCertDir != "" { - tlsCfg, err := mtlsConfig(cfg.MTLSCertDir, hostOnly(target)) + // No scheme: mTLS when client cert material is configured, else plaintext. + if cfg.MTLSCert != "" { + tlsCfg, err := mtlsConfig(cfg.MTLSCert, cfg.MTLSKey, cfg.MTLSCA, hostOnly(target)) if err != nil { return nil, err } @@ -251,24 +254,25 @@ func (c *Client) getToken() (string, time.Time) { return c.token, c.tokenExp } -// mtlsConfig builds a client mTLS config from a cert dir holding ca.crt, -// tls.crt, tls.key. The CA pool extends the system roots (mirrors -// authlib/tlsbridge/upstream.go). -func mtlsConfig(dir, serverName string) (*tls.Config, error) { - cert, err := tls.LoadX509KeyPair(filepath.Join(dir, "tls.crt"), filepath.Join(dir, "tls.key")) +// mtlsConfig builds a client mTLS config from independent cert, key, and CA +// file paths (mirroring OpenShell's OPENSHELL_TLS_CERT/KEY/CA — the driver puts +// the CA in a different secret/dir than the client keypair). The CA pool extends +// the system roots (mirrors authlib/tlsbridge/upstream.go). +func mtlsConfig(certPath, keyPath, caPath, serverName string) (*tls.Config, error) { + cert, err := tls.LoadX509KeyPair(certPath, keyPath) if err != nil { - return nil, fmt.Errorf("openshell: load client cert from %q: %w", dir, err) + return nil, fmt.Errorf("openshell: load client cert (%q, %q): %w", certPath, keyPath, err) } pool, err := x509.SystemCertPool() if err != nil || pool == nil { pool = x509.NewCertPool() } - caPEM, err := os.ReadFile(filepath.Join(dir, "ca.crt")) + caPEM, err := os.ReadFile(caPath) if err != nil { - return nil, fmt.Errorf("openshell: read ca.crt from %q: %w", dir, err) + return nil, fmt.Errorf("openshell: read ca %q: %w", caPath, err) } if !pool.AppendCertsFromPEM(caPEM) { - return nil, fmt.Errorf("openshell: ca.crt in %q is not valid PEM", dir) + return nil, fmt.Errorf("openshell: ca %q is not valid PEM", caPath) } return &tls.Config{ Certificates: []tls.Certificate{cert}, diff --git a/authbridge/authlib/openshell/client_test.go b/authbridge/authlib/openshell/client_test.go index 41dce532d..4a477bbd6 100644 --- a/authbridge/authlib/openshell/client_test.go +++ b/authbridge/authlib/openshell/client_test.go @@ -197,7 +197,9 @@ func TestDialHTTPSUsesMTLS(t *testing.T) { dir := writeMTLSCertDir(t) c, err := Dial(Config{ Endpoint: "https://gw.example.com:8080", - MTLSCertDir: dir, + MTLSCert: filepath.Join(dir, "tls.crt"), + MTLSKey: filepath.Join(dir, "tls.key"), + MTLSCA: filepath.Join(dir, "ca.crt"), SATokenPath: writeSAToken(t, "sa"), SandboxID: "sb", }) @@ -207,20 +209,20 @@ func TestDialHTTPSUsesMTLS(t *testing.T) { _ = c.Close() } -func TestDialHTTPSRequiresCertDir(t *testing.T) { +func TestDialHTTPSRequiresMTLS(t *testing.T) { _, err := Dial(Config{ Endpoint: "https://gw.example.com:8080", SATokenPath: writeSAToken(t, "sa"), SandboxID: "sb", }) if err == nil { - t.Fatal("https without mtls_cert_dir should fail validation") + t.Fatal("https without mtls_cert/key/ca should fail validation") } } func TestMTLSConfig(t *testing.T) { dir := writeMTLSCertDir(t) - cfg, err := mtlsConfig(dir, "gw.example.com") + cfg, err := mtlsConfig(filepath.Join(dir, "tls.crt"), filepath.Join(dir, "tls.key"), filepath.Join(dir, "ca.crt"), "gw.example.com") if err != nil { t.Fatalf("mtlsConfig: %v", err) } diff --git a/authbridge/authlib/plugins/placeholderresolve/gateway/resolver.go b/authbridge/authlib/plugins/placeholderresolve/gateway/resolver.go index 2cfe0bcb3..92878b339 100644 --- a/authbridge/authlib/plugins/placeholderresolve/gateway/resolver.go +++ b/authbridge/authlib/plugins/placeholderresolve/gateway/resolver.go @@ -30,7 +30,9 @@ const ( // Config locates the gateway and the sandbox identity material. type Config struct { Endpoint string - MTLSCertDir string + MTLSCert string + MTLSKey string + MTLSCA string SATokenPath string SandboxID string // Insecure permits plaintext gRPC to a non-loopback gateway (opt-in). @@ -52,7 +54,9 @@ type Resolver struct { func New(cfg Config) (*Resolver, error) { client, err := openshell.Dial(openshell.Config{ Endpoint: cfg.Endpoint, - MTLSCertDir: cfg.MTLSCertDir, + MTLSCert: cfg.MTLSCert, + MTLSKey: cfg.MTLSKey, + MTLSCA: cfg.MTLSCA, SATokenPath: cfg.SATokenPath, SandboxID: cfg.SandboxID, Insecure: cfg.Insecure, diff --git a/authbridge/authlib/plugins/placeholderresolve/plugin.go b/authbridge/authlib/plugins/placeholderresolve/plugin.go index 63469838b..c73c3594f 100644 --- a/authbridge/authlib/plugins/placeholderresolve/plugin.go +++ b/authbridge/authlib/plugins/placeholderresolve/plugin.go @@ -43,7 +43,6 @@ const defaultPrefix = "openshell:resolve:env:" // Default OpenShell sidecar paths for the gateway source (match the k8s // driver's pod-spec conventions). const ( - defaultMTLSCertDir = "/etc/openshell-tls/client" defaultSATokenPath = "/var/run/secrets/openshell/token" ) @@ -75,7 +74,9 @@ type placeholderResolveConfig struct { // gatewayConfig configures the OpenShell-gateway resolver source. type gatewayConfig struct { Endpoint string `json:"endpoint" required:"true" description:"OpenShell gateway gRPC endpoint, e.g. https://openshell..svc:8080."` - MTLSCertDir string `json:"mtls_cert_dir" description:"Dir holding ca.crt/tls.crt/tls.key for mTLS to the gateway." default:"/etc/openshell-tls/client"` + MTLSCert string `json:"mtls_cert" description:"Client cert file for mTLS to the gateway (OPENSHELL_TLS_CERT). Required for https endpoints."` + MTLSKey string `json:"mtls_key" description:"Client key file for mTLS to the gateway (OPENSHELL_TLS_KEY). Required for https endpoints."` + MTLSCA string `json:"mtls_ca" description:"CA file verifying the gateway server cert (OPENSHELL_TLS_CA). Required for https endpoints."` SATokenPath string `json:"sa_token_path" description:"Projected SA token file (audience openshell-gateway)." default:"/var/run/secrets/openshell/token"` SandboxID string `json:"sandbox_id" required:"true" description:"This sandbox's id (OPENSHELL_SANDBOX_ID); must match the gateway-minted JWT."` Insecure bool `json:"insecure" description:"Permit plaintext gRPC to a non-loopback gateway (opt-in). Sends the SA token + JWT in cleartext; refused by default for non-loopback endpoints." default:"false"` @@ -86,9 +87,6 @@ func (c *placeholderResolveConfig) applyDefaults() { c.Prefix = defaultPrefix } if c.Source == sourceGateway && c.Gateway != nil { - if c.Gateway.MTLSCertDir == "" { - c.Gateway.MTLSCertDir = defaultMTLSCertDir - } if c.Gateway.SATokenPath == "" { c.Gateway.SATokenPath = defaultSATokenPath } @@ -169,7 +167,9 @@ func (p *PlaceholderResolve) Configure(raw json.RawMessage) error { case sourceGateway: gw, gerr := gateway.New(gateway.Config{ Endpoint: c.Gateway.Endpoint, - MTLSCertDir: c.Gateway.MTLSCertDir, + MTLSCert: c.Gateway.MTLSCert, + MTLSKey: c.Gateway.MTLSKey, + MTLSCA: c.Gateway.MTLSCA, SATokenPath: c.Gateway.SATokenPath, SandboxID: c.Gateway.SandboxID, Insecure: c.Gateway.Insecure, diff --git a/authbridge/authlib/plugins/placeholderresolve/plugin_test.go b/authbridge/authlib/plugins/placeholderresolve/plugin_test.go index b1f49aa16..42e783585 100644 --- a/authbridge/authlib/plugins/placeholderresolve/plugin_test.go +++ b/authbridge/authlib/plugins/placeholderresolve/plugin_test.go @@ -76,8 +76,8 @@ func TestConfigureGatewayDefaults(t *testing.T) { if err != nil { t.Fatalf("Configure: %v", err) } - if p.cfg.Gateway.MTLSCertDir != defaultMTLSCertDir || p.cfg.Gateway.SATokenPath != defaultSATokenPath { - t.Errorf("gateway defaults not applied: %+v", p.cfg.Gateway) + if p.cfg.Gateway.SATokenPath != defaultSATokenPath { + t.Errorf("gateway sa_token_path default not applied: %+v", p.cfg.Gateway) } _ = p.Shutdown(context.Background()) } diff --git a/authbridge/docs/plugin-reference.md b/authbridge/docs/plugin-reference.md index 24b2ac615..8e11490f3 100644 --- a/authbridge/docs/plugin-reference.md +++ b/authbridge/docs/plugin-reference.md @@ -118,7 +118,9 @@ rewritten — it is the only header any listener reconciles to the upstream wire gateway: # used when source: gateway (OpenShell sandbox sidecar) endpoint: "https://openshell..svc:8080" # required sandbox_id: "" # required; must match the minted JWT - mtls_cert_dir: "/etc/openshell-tls/client" # ca.crt/tls.crt/tls.key; required for https + mtls_cert: "/tls/client/tls.crt" # client cert (OPENSHELL_TLS_CERT); required for https + mtls_key: "/tls/client/tls.key" # client key (OPENSHELL_TLS_KEY); required for https + mtls_ca: "/tls/ca/ca.crt" # gateway CA (OPENSHELL_TLS_CA); required for https sa_token_path: "/var/run/secrets/openshell/token" insecure: false # opt-in plaintext (non-loopback refused by default) # secret_dir: "/etc/authbridge-cred" # used when source: secret_dir (file per KEY) @@ -130,7 +132,7 @@ rewritten — it is the only header any listener reconciles to the upstream wire | `source` | — (**required**) | `gateway` or `secret_dir`. There is **no implicit fallback** — an unset/unknown source fails `Configure`, so the plugin never silently resolves from an ambient source. | | `gateway.endpoint` | — (required for `gateway`) | Gateway gRPC endpoint; `https://` selects mTLS, `http://` plaintext (non-loopback refused unless `insecure`). | | `gateway.sandbox_id` | — (required for `gateway`) | Must equal the id bound into the gateway-minted JWT. | -| `gateway.mtls_cert_dir` | `/etc/openshell-tls/client` | `ca.crt`/`tls.crt`/`tls.key`; required for an `https://` endpoint. | +| `gateway.mtls_cert` / `mtls_key` / `mtls_ca` | — (required for `https://`) | Client cert, client key, and gateway CA — independent file paths, mirroring OpenShell's `OPENSHELL_TLS_CERT`/`KEY`/`CA` (the driver splits the CA from the client keypair, so they need not share a dir). | | `gateway.sa_token_path` | `/var/run/secrets/openshell/token` | Projected SA token (audience `openshell-gateway`). | | `gateway.insecure` | `false` | Permit plaintext gRPC to a **non-loopback** gateway. Plaintext sends the SA token + JWT in cleartext, so it is **refused by default** for non-loopback endpoints (loopback always allowed). | | `secret_dir` | — (required for `secret_dir`) | Directory; each KEY is read from a path-contained `/` file. | From 9e59ae483795226f29f6b400bdff21580ac6eb92 Mon Sep 17 00:00:00 2001 From: Hai Huang Date: Fri, 26 Jun 2026 18:40:52 -0400 Subject: [PATCH 7/9] fix(abctl): suppress inactive events per exchange, not per event MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The s (hideInactive) toggle evaluated each event against its own phase's invocations — request-phase for a request, response-phase for a response. A passthrough response carries no invocations of its own, so a skip-only request could be hidden while its invocation-less response stayed, orphaning the response (and symmetrically an active request could keep a hidden response). Gate suppression on the whole exchange: a request/response pair is hidden only when BOTH halves are inactive, using the partner pairing already computed in rebuildEventsTable. Unpaired rows fall back to per-event. Add TestRebuildEventsTable_HideInactivePerExchange. Assisted-By: Claude (Anthropic AI) Signed-off-by: Hai Huang --- authbridge/cmd/abctl/tui/events_pane.go | 17 ++++++++-- authbridge/cmd/abctl/tui/events_pane_test.go | 34 ++++++++++++++++++++ 2 files changed, 49 insertions(+), 2 deletions(-) diff --git a/authbridge/cmd/abctl/tui/events_pane.go b/authbridge/cmd/abctl/tui/events_pane.go index 574fff9d5..dba47284c 100644 --- a/authbridge/cmd/abctl/tui/events_pane.go +++ b/authbridge/cmd/abctl/tui/events_pane.go @@ -110,10 +110,23 @@ func (m *model) rebuildEventsTable() { // network messages". Turning it on focuses the timeline on plugin // activity (deny/modify/observe/allow). Both the filter and the // headline consider the folded tunnel's invocations too. + // + // Suppress per EXCHANGE, not per event: a request and its paired + // response are hidden only when BOTH sides are inactive. Evaluating each + // event against its own phase's invocations alone would hide, say, a + // skip-only request while keeping its (invocation-less) response — + // orphaning the response. Unpaired rows (a lone CONNECT tunnel-open, or + // an unmatched request/response) fall back to per-event. invs := er.invocations() if m.hideInactive && eventInactive(invs) { - m.hiddenInactive++ - continue + partnerInactive := true + if j, ok := partner[i]; ok { + partnerInactive = eventInactive(eventRows[j].invocations()) + } + if partnerInactive { + m.hiddenInactive++ + continue + } } action, plugin := eventAction(invs) diff --git a/authbridge/cmd/abctl/tui/events_pane_test.go b/authbridge/cmd/abctl/tui/events_pane_test.go index 02855b35a..c40e5f1a4 100644 --- a/authbridge/cmd/abctl/tui/events_pane_test.go +++ b/authbridge/cmd/abctl/tui/events_pane_test.go @@ -492,6 +492,40 @@ func TestRebuildEventsTable_HideInactive(t *testing.T) { } } +// TestRebuildEventsTable_HideInactivePerExchange checks that hideInactive +// suppresses a request/response pair only when BOTH halves are inactive — an +// active half keeps its (invocation-less) partner visible rather than orphaning +// it. Exchange A: active request + passthrough response → both shown. Exchange +// B: skip-only request + invocation-less response → both hidden. +func TestRebuildEventsTable_HideInactivePerExchange(t *testing.T) { + events := []pipeline.SessionEvent{ + {Direction: pipeline.Outbound, Phase: pipeline.SessionRequest, Host: "llm", + Invocations: &pipeline.Invocations{Outbound: []pipeline.Invocation{ + {Plugin: "placeholder-resolve", Action: pipeline.ActionModify}, + }}}, + {Direction: pipeline.Outbound, Phase: pipeline.SessionResponse, Host: "llm", StatusCode: 200}, + {Direction: pipeline.Outbound, Phase: pipeline.SessionRequest, Host: "pass", + Invocations: &pipeline.Invocations{Outbound: []pipeline.Invocation{ + {Plugin: "placeholder-resolve", Action: pipeline.ActionSkip}, + }}}, + {Direction: pipeline.Outbound, Phase: pipeline.SessionResponse, Host: "pass", StatusCode: 200}, + } + m := &model{selectedSess: "s", events: map[string][]pipeline.SessionEvent{"s": events}} + m.eventsTbl = newEventsTable() + m.hideInactive = true + m.rebuildEventsTable() + + if len(m.visibleRows) != 2 || m.hiddenInactive != 2 { + t.Fatalf("per-exchange hide: rows=%d hidden=%d, want 2/2 (active pair shown, passthrough pair hidden)", + len(m.visibleRows), m.hiddenInactive) + } + for _, er := range m.visibleRows { + if hostOnly(er.event.Host) != "llm" { + t.Errorf("visible row host = %q, want llm (both halves of the active exchange, no orphan)", er.event.Host) + } + } +} + // TestComputeEventPairIDs pairs each response row with its preceding request // row by direction + host + method, sharing one # across the exchange and // minting fresh integers for unpaired rows. From b2f35026376fb16b54090c06383fc6c33e1cd7f7 Mon Sep 17 00:00:00 2001 From: Hai Huang Date: Fri, 26 Jun 2026 19:30:07 -0400 Subject: [PATCH 8/9] fix(abctl): keep the events cursor stable while streaming (follow + identity) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The auto-follow re-set the cursor on every streamed event with index-based heuristics: it INFERRED follow-mode from 'cursor on the last row' (wasAtEnd) and restored the cursor by ROW INDEX. A rebuild that shifts indices — a CONNECT folding into its inner request, hide-inactive filtering, or a buffer trim — landing between key presses could move the cursor opposite to the arrow key (the 'down sometimes scrolls up' report). Track follow mode explicitly (set on session entry / End / a Down reaching the last row; cleared on Up / Home / scrolling away) and, when not following, restore the cursor by EVENT IDENTITY (timestamp+direction+phase) instead of row index, so folding/filtering/trimming can't scroll the view out from under the user. Tests: FollowPinsToNewest, PreservesCursorByIdentity. Assisted-By: Claude (Anthropic AI) Signed-off-by: Hai Huang --- authbridge/cmd/abctl/tui/app.go | 9 ++++ authbridge/cmd/abctl/tui/events_pane.go | 49 +++++++++++++++++--- authbridge/cmd/abctl/tui/events_pane_test.go | 49 ++++++++++++++++++++ authbridge/cmd/abctl/tui/keys.go | 3 ++ 4 files changed, 104 insertions(+), 6 deletions(-) diff --git a/authbridge/cmd/abctl/tui/app.go b/authbridge/cmd/abctl/tui/app.go index 67904b982..82d4977d9 100644 --- a/authbridge/cmd/abctl/tui/app.go +++ b/authbridge/cmd/abctl/tui/app.go @@ -231,6 +231,14 @@ type model struct { // every rebuild. visibleRows []eventRow + // follow, when true, pins the events cursor to the newest message as the + // stream advances. Set when the user is at the tail (entering a session, + // End/G, or a Down that reaches the last row); cleared on any upward or + // Home/g navigation. When false, rebuildEventsTable keeps the cursor on the + // event it was on by IDENTITY, not by row index — so folding/filtering + // can't scroll the view out from under the user. + follow bool + // pipeline is the fetched plugin composition. nil until the initial // GetPipeline response arrives; the pipeline pane shows "(loading…)" // until then. @@ -301,6 +309,7 @@ func New(ctx context.Context, c *apiclient.Client) tea.Model { cancel: cancel, events: make(map[string][]pipeline.SessionEvent), pane: paneSessions, + follow: true, // start pinned to the latest until the user scrolls up sessionsTbl: newSessionsTable(), eventsTbl: newEventsTable(), pipelineTbl: newPipelineTable(), diff --git a/authbridge/cmd/abctl/tui/events_pane.go b/authbridge/cmd/abctl/tui/events_pane.go index dba47284c..f49b5c3be 100644 --- a/authbridge/cmd/abctl/tui/events_pane.go +++ b/authbridge/cmd/abctl/tui/events_pane.go @@ -81,8 +81,17 @@ func (m *model) rebuildEventsTable() { m.eventsTbl.SetHeight(h) } + // Anchor the cursor to the EVENT it's on, captured before the rebuild + // (m.visibleRows is still the previous build, in sync with the table's + // current cursor). It's restored below by re-finding that event — its row + // index can shift when a CONNECT tunnel folds into its inner request or + // hide-inactive filters — so a streamed rebuild never scrolls the view out + // from under the user. follow mode pins to the newest event instead. prevRow := m.eventsTbl.Cursor() - wasAtEnd := prevRow >= len(m.eventsTbl.Rows())-1 + var anchor *pipeline.SessionEvent + if prevRow >= 0 && prevRow < len(m.visibleRows) { + anchor = m.visibleRows[prevRow].event + } // One display row per network message. CONNECT tunnel-opens are folded // into the decrypted inner request that immediately follows them (TLS @@ -160,15 +169,43 @@ func (m *model) rebuildEventsTable() { } m.eventsTbl.SetRows(rows) - // Auto-follow: if user was at the bottom, stay at the bottom. Otherwise - // preserve position so reading isn't disturbed by new events. - if wasAtEnd && len(rows) > 0 { + // Restore the cursor: follow the newest event in follow mode; otherwise + // re-find the anchored event by identity (timestamp + direction + phase) so a + // shifted row index doesn't jump the view. If the anchored event is gone + // (trimmed, folded away, or filtered out), clamp the old index best-effort. + switch { + case m.follow && len(rows) > 0: m.eventsTbl.SetCursor(len(rows) - 1) - } else if prevRow < len(rows) { - m.eventsTbl.SetCursor(prevRow) + case anchor != nil: + same := func(e *pipeline.SessionEvent) bool { + return e != nil && e.At.Equal(anchor.At) && + e.Direction == anchor.Direction && e.Phase == anchor.Phase + } + idx := -1 + for i := range m.visibleRows { + if same(m.visibleRows[i].event) || same(m.visibleRows[i].tunnel) { + idx = i + break + } + } + if idx >= 0 { + m.eventsTbl.SetCursor(idx) + } else if prevRow >= 0 && prevRow < len(rows) { + m.eventsTbl.SetCursor(prevRow) + } } } +// syncEventsFollow recomputes follow mode from the cursor: follow the newest +// event only when the cursor is at (or past) the last row. Called after manual +// navigation in the events pane so follow reflects where the user landed — a +// Down that reaches the bottom re-enables following; any other position is a +// deliberate scroll-away and pins the view there. +func (m *model) syncEventsFollow() { + n := len(m.eventsTbl.Rows()) + m.follow = n == 0 || m.eventsTbl.Cursor() >= n-1 +} + // selectedEvent returns the event at the cursor row, or nil. The cursor // points into m.visibleRows (one entry per rendered message), and each row // carries a reference to its source event. diff --git a/authbridge/cmd/abctl/tui/events_pane_test.go b/authbridge/cmd/abctl/tui/events_pane_test.go index c40e5f1a4..112fdc7f7 100644 --- a/authbridge/cmd/abctl/tui/events_pane_test.go +++ b/authbridge/cmd/abctl/tui/events_pane_test.go @@ -526,6 +526,55 @@ func TestRebuildEventsTable_HideInactivePerExchange(t *testing.T) { } } +// TestRebuildEventsTable_FollowPinsToNewest: in follow mode the cursor tracks +// the newest event as the stream grows. +func TestRebuildEventsTable_FollowPinsToNewest(t *testing.T) { + t0 := time.Now() + mk := func(i int) pipeline.SessionEvent { + return pipeline.SessionEvent{At: t0.Add(time.Duration(i) * time.Second), + Direction: pipeline.Outbound, Phase: pipeline.SessionRequest, Host: "h"} + } + m := &model{selectedSess: "s", events: map[string][]pipeline.SessionEvent{"s": {mk(0), mk(1)}}} + m.eventsTbl = newEventsTable() + m.follow = true + m.rebuildEventsTable() + + m.events["s"] = append(m.events["s"], mk(2)) // new event streams in + m.rebuildEventsTable() + + if got, want := m.eventsTbl.Cursor(), len(m.visibleRows)-1; got != want { + t.Errorf("follow: cursor=%d, want newest row %d", got, want) + } +} + +// TestRebuildEventsTable_PreservesCursorByIdentity: when NOT following, a rebuild +// that shifts row indices (here simulated by an event landing ahead of the +// anchored one — the effect of a CONNECT fold, a hide-inactive change, or a +// buffer trim) keeps the cursor on the SAME event, not the same index. +func TestRebuildEventsTable_PreservesCursorByIdentity(t *testing.T) { + t0 := time.Now() + x := pipeline.SessionEvent{At: t0, Direction: pipeline.Outbound, Phase: pipeline.SessionRequest, Host: "x"} + y := pipeline.SessionEvent{At: t0.Add(time.Second), Direction: pipeline.Outbound, Phase: pipeline.SessionRequest, Host: "y"} + z := pipeline.SessionEvent{At: t0.Add(2 * time.Second), Direction: pipeline.Outbound, Phase: pipeline.SessionRequest, Host: "z"} + + m := &model{selectedSess: "s", events: map[string][]pipeline.SessionEvent{"s": {x, y}}} + m.eventsTbl = newEventsTable() + m.follow = false + m.rebuildEventsTable() + m.eventsTbl.SetCursor(0) // park on x (row 0) + if got := m.visibleRows[m.eventsTbl.Cursor()].event.Host; got != "x" { + t.Fatalf("setup: cursor on %q, want x", got) + } + + // x's row index shifts from 0 to 1. + m.events["s"] = []pipeline.SessionEvent{z, x, y} + m.rebuildEventsTable() + + if got := m.visibleRows[m.eventsTbl.Cursor()].event.Host; got != "x" { + t.Errorf("after index shift: cursor on %q, want x (followed by identity, not stuck at row 0=z)", got) + } +} + // TestComputeEventPairIDs pairs each response row with its preceding request // row by direction + host + method, sharing one # across the exchange and // minting fresh integers for unpaired rows. diff --git a/authbridge/cmd/abctl/tui/keys.go b/authbridge/cmd/abctl/tui/keys.go index 8898ef3f3..0c62dca27 100644 --- a/authbridge/cmd/abctl/tui/keys.go +++ b/authbridge/cmd/abctl/tui/keys.go @@ -202,6 +202,7 @@ func (m *model) handleKey(msg tea.KeyMsg) tea.Cmd { } m.selectedSess = id m.pane = paneEvents + m.follow = true // entering a session: show the latest + follow new events m.rebuildEventsTable() // Snapshot in case the stream hasn't yet delivered history. return m.snapshotCmd(id) @@ -315,6 +316,7 @@ func (m *model) handleKey(msg tea.KeyMsg) tea.Cmd { case paneEvents: var cmd tea.Cmd m.eventsTbl, cmd = m.eventsTbl.Update(msg) + m.syncEventsFollow() // follow only if the nav left the cursor at the last row return cmd case paneDetail, panePluginDetail: var cmd tea.Cmd @@ -367,6 +369,7 @@ func (m *model) goTop() { m.sessionsTbl.SetCursor(0) case paneEvents: m.eventsTbl.SetCursor(0) + m.follow = false // jumped to the top: stop following the tail case panePipeline: m.pipelineTbl.SetCursor(0) case paneDetail, panePluginDetail: From 509cb2f7b64d25c9fd976d3a7f72b49d68182d9b Mon Sep 17 00:00:00 2001 From: Hai Huang Date: Sat, 27 Jun 2026 14:27:16 -0400 Subject: [PATCH 9/9] =?UTF-8?q?fix(authbridge):=20address=20review=20?= =?UTF-8?q?=E2=80=94=20follow=20on=20G/End,=20atomic=20resolver=20Start,?= =?UTF-8?q?=20enforce=20doc=20note?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - abctl: goBottom (G/End) now sets m.follow=true, so jumping to the tail after a scroll-up re-pins to new events (app.go documented End/G as follow-entering but goBottom didn't set it). - gateway resolver: Start uses CompareAndSwap instead of load-then-store, so the double-start guard is correct by construction (not just via single-threaded Init); the loser cancels its unused context. - plugin-reference.md: note that 'the placeholder is never forwarded' holds under the default enforce policy; on_error: observe shadows the deny and forwards the literal placeholder (no real credential leaks, but fail-closed no longer holds). Also reviewed, no change: gateway server-cert trust = system roots + configured CA (matches the tlsbridge/upstream.go convention); and the build.yaml 'full plugin set, includes parsers' comment is correct (the default authbridge image builds with empty GO_BUILD_TAGS — parsers + placeholder-resolve included; the excludes are on the separate authbridge-lite image). Assisted-By: Claude (Anthropic AI) Signed-off-by: Hai Huang --- .../plugins/placeholderresolve/gateway/resolver.go | 10 ++++++---- authbridge/cmd/abctl/tui/keys.go | 1 + authbridge/docs/plugin-reference.md | 6 +++++- 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/authbridge/authlib/plugins/placeholderresolve/gateway/resolver.go b/authbridge/authlib/plugins/placeholderresolve/gateway/resolver.go index 92878b339..c94330041 100644 --- a/authbridge/authlib/plugins/placeholderresolve/gateway/resolver.go +++ b/authbridge/authlib/plugins/placeholderresolve/gateway/resolver.go @@ -123,11 +123,13 @@ func stripRevisionPrefix(key string) (string, bool) { // outlives the pipeline's Init budget) and returns immediately. The loop // fetches right away; Ready() flips true once the first fetch succeeds. func (r *Resolver) Start(_ context.Context) error { - if r.bgCancel.Load() != nil { - return nil // already started - } bgCtx, cancel := context.WithCancel(context.Background()) - r.bgCancel.Store(&cancel) + // Atomic guard against a double Start (correct by construction even though the + // pipeline calls Init once): only the goroutine that wins the CAS owns the loop. + if !r.bgCancel.CompareAndSwap(nil, &cancel) { + cancel() // already started — discard the unused context + return nil + } go r.refreshLoop(bgCtx) return nil } diff --git a/authbridge/cmd/abctl/tui/keys.go b/authbridge/cmd/abctl/tui/keys.go index 0c62dca27..534bb6887 100644 --- a/authbridge/cmd/abctl/tui/keys.go +++ b/authbridge/cmd/abctl/tui/keys.go @@ -387,6 +387,7 @@ func (m *model) goBottom() { if n := len(m.eventsTbl.Rows()); n > 0 { m.eventsTbl.SetCursor(n - 1) } + m.follow = true // jumped to the tail: resume following new events case panePipeline: if n := len(m.pipelineTbl.Rows()); n > 0 { m.pipelineTbl.SetCursor(n - 1) diff --git a/authbridge/docs/plugin-reference.md b/authbridge/docs/plugin-reference.md index 8e11490f3..5a578f567 100644 --- a/authbridge/docs/plugin-reference.md +++ b/authbridge/docs/plugin-reference.md @@ -138,7 +138,11 @@ rewritten — it is the only header any listener reconciles to the upstream wire | `secret_dir` | — (required for `secret_dir`) | Directory; each KEY is read from a path-contained `/` file. | An unresolved placeholder, or a resolved value containing CR/LF/NUL, fails the -request closed (`401 auth.unauthorized`) — the placeholder is never forwarded. The +request closed (`401 auth.unauthorized`) — the placeholder is never forwarded **under +the default `enforce` policy**. Run placeholder-resolve under `enforce`: with +`on_error: observe` the deny is shadowed and the request is forwarded carrying the +literal placeholder. No real credential leaks (the deny precedes header injection, and +upstream rejects the bogus bearer), but fail-closed no longer holds. The `gateway` source requires running as a sidecar in the sandbox pod (shared SA + the `openshell.io/sandbox-id` annotation) and warms its cache in the background, so the pipeline gates traffic on `Ready()` until the first fetch succeeds. (The shared