From e4d22ccce7019f015daa435dd5d3bb0e17c95eb9 Mon Sep 17 00:00:00 2001 From: fullsend-code <278716306+fullsend-ai-coder[bot]@users.noreply.github.com> Date: Sun, 30 Aug 2026 16:12:50 +0000 Subject: [PATCH 1/5] feat(#6778): add fullsend inference openai command family Add three new subcommands under `fullsend inference openai` for OpenAI Workload Identity Federation enrollment: - `request [,...]`: generates the provider/mapping request document an admin needs, with every value computed from the repository names. Supports --format json (versioned schema for future API submission) and --format md (copy-paste ticket). - `import [reply.json]`: takes the admin's reply and writes inference.openai into .fullsend/config.yaml via the existing SetInferenceOpenAI setter, or sets FULLSEND_OPENAI_* repository variables with --variables. Enforces the all-three-or-none rule. - `status `: prints resolved identifiers and their source (config.yaml or env vars), flags a partial trio, and when inside GitHub Actions performs one exchange reporting scope and expiry without printing the token. The commands need no credentials and do not call the OpenAI API. The JSON request schema is versioned so a future `apply` command can submit it unchanged when OpenAI exposes an API for providers. Updates docs/cli/inference.md with the new command reference and adds route-B shortcut pointers in the OpenAI WIF guide. Closes #6778 --- docs/cli/inference.md | 73 ++ .../openai-workload-identity.md | 11 + internal/cli/inference.go | 1 + internal/cli/inference_openai.go | 682 ++++++++++++++ internal/cli/inference_openai_test.go | 859 ++++++++++++++++++ 5 files changed, 1626 insertions(+) create mode 100644 internal/cli/inference_openai.go create mode 100644 internal/cli/inference_openai_test.go diff --git a/docs/cli/inference.md b/docs/cli/inference.md index 27aba22550..707715d4db 100644 --- a/docs/cli/inference.md +++ b/docs/cli/inference.md @@ -13,6 +13,9 @@ Manage GCP Workload Identity Federation (WIF) infrastructure for Agent Platform | `fullsend inference provision ` | Create WIF pool/provider and grant Agent Platform access | | `fullsend inference deprovision ` | Remove org or repo from WIF | | `fullsend inference status ` | Check WIF health and print config values | +| `fullsend inference openai request [,…]` | Generate WIF provider/mapping request for OpenAI admin | +| `fullsend inference openai import [reply.json]` | Import OpenAI WIF identifiers into config | +| `fullsend inference openai status ` | Check OpenAI WIF configuration and exchange status | ## `inference provision` @@ -80,8 +83,78 @@ fullsend inference status \ Read-only — makes no changes. +## `inference openai` + +Commands for enrolling repositories with OpenAI Workload Identity Federation. These commands do not call the OpenAI API — they produce documents and update local configuration. + +### `inference openai request` + +Generates the request document an administrator needs to enable OpenAI WIF for one or more repositories. Every value is computed from the repository names. Nothing is sent anywhere; the command needs no credentials. + +```bash +fullsend inference openai request [,…] \ + [--audience ""] \ + [--project ""] \ + [--service-account ""] \ + [--format json|md] \ + [--out ] +``` + +| Flag | Default | Description | +|------|---------|-------------| +| `--audience` | `fullsend://` | OpenAI Workload Identity audience | +| `--project` | *(empty)* | OpenAI project name or ID for the service accounts | +| `--service-account` | `fullsend--ci` | Existing service account ID (default: one per repo) | +| `--format` | `json` | Output format: `json` (versioned schema) or `md` (copy-paste ticket) | +| `--out` | *(stdout)* | Write output to a file | + +### `inference openai import` + +Takes the administrator's reply and writes `inference.openai` into `.fullsend/config.yaml` through the same setters as `fullsend github setup --openai-*`. All three identifiers must be present — a partial trio is refused. + +```bash +# From a reply JSON file: +fullsend inference openai import reply.json + +# From flags: +fullsend inference openai import \ + --audience "fullsend://" \ + --identity-provider-id "" \ + --service-account-id "" + +# Set repository variables instead of config.yaml: +fullsend inference openai import \ + --variables --repo \ + --audience "fullsend://" \ + --identity-provider-id "" \ + --service-account-id "" +``` + +| Flag | Default | Description | +|------|---------|-------------| +| `--audience` | | OpenAI Workload Identity audience | +| `--identity-provider-id` | | OpenAI identity provider ID | +| `--service-account-id` | | OpenAI service account ID | +| `--fullsend-dir` | `.fullsend` | Path to the .fullsend configuration directory | +| `--variables` | `false` | Set `FULLSEND_OPENAI_*` repository variables instead of config.yaml | +| `--repo` | | Target repository (`owner/repo`) for `--variables` | + +### `inference openai status` + +Prints the resolved OpenAI WIF identifiers and their source (config layer or environment variables), and flags a partial trio. When run inside a GitHub Actions job with `id-token: write`, performs one exchange and reports the returned scope and expiry without ever printing the token. + +```bash +fullsend inference openai status \ + [--fullsend-dir ".fullsend"] +``` + +| Flag | Default | Description | +|------|---------|-------------| +| `--fullsend-dir` | `.fullsend` | Path to the .fullsend configuration directory | + ## See also - [Getting inference for fullsend](../guides/getting-started/getting-inference.md) — getting started guide +- [OpenAI Workload Identity](../guides/infrastructure/openai-workload-identity.md) — end-to-end OpenAI WIF setup guide - [Advanced setup](../guides/infrastructure/advanced-setup.md) — non-standard installation paths and WIF configuration - [CLI internals](../guides/dev/cli-internals.md) — command tree and implementation details diff --git a/docs/guides/infrastructure/openai-workload-identity.md b/docs/guides/infrastructure/openai-workload-identity.md index 0d978da960..673bf2ef1d 100644 --- a/docs/guides/infrastructure/openai-workload-identity.md +++ b/docs/guides/infrastructure/openai-workload-identity.md @@ -86,6 +86,12 @@ you are done. ## 3. Map the repository to a service account (once per repository) +> **Shortcut:** `fullsend inference openai request ` generates the +> complete request document (JSON or Markdown) with every value computed from the +> repository name — use it to produce the ticket for your administrator instead of +> filling in the tables below by hand. See +> [`fullsend inference openai`](../../cli/inference.md#inference-openai) for details. + Open the provider you created and add a **service account mapping**: | Field | Enter | @@ -126,6 +132,11 @@ so assert that repository instead. ## 4. Tell fullsend the three identifiers +> **Shortcut:** `fullsend inference openai import` writes the three identifiers +> into `.fullsend/config.yaml` (or sets repository variables with `--variables`). +> It accepts a JSON reply file or CLI flags. See +> [`fullsend inference openai import`](../../cli/inference.md#inference-openai-import). + Re-run the setup command you enrolled the repository with, adding the three values: ```bash diff --git a/internal/cli/inference.go b/internal/cli/inference.go index 428bb577ab..dc9af45724 100644 --- a/internal/cli/inference.go +++ b/internal/cli/inference.go @@ -29,6 +29,7 @@ to the GitHub admin who runs 'fullsend github setup'.`, cmd.AddCommand(newInferenceProvisionCmd()) cmd.AddCommand(newInferenceStatusCmd()) cmd.AddCommand(newInferenceDeprovisionCmd()) + cmd.AddCommand(newInferenceOpenAICmd()) return cmd } diff --git a/internal/cli/inference_openai.go b/internal/cli/inference_openai.go new file mode 100644 index 0000000000..813f808de7 --- /dev/null +++ b/internal/cli/inference_openai.go @@ -0,0 +1,682 @@ +package cli + +import ( + "encoding/json" + "fmt" + "os" + "os/exec" + "path/filepath" + "strings" + "text/template" + "time" + + "github.com/spf13/cobra" + + "github.com/fullsend-ai/fullsend/internal/config" + "github.com/fullsend-ai/fullsend/internal/inference/openaiwif" + "github.com/fullsend-ai/fullsend/internal/ui" +) + +const ( + // openAIRequestSchemaVersion is the versioned schema for the request + // JSON, so a future `fullsend inference openai apply request.json` + // can submit it unchanged. + openAIRequestSchemaVersion = "1" + + // defaultOpenAIAudiencePrefix is the default audience convention. + defaultOpenAIAudiencePrefix = "fullsend://" + + // githubOIDCIssuer is the fixed OIDC issuer for GitHub Actions. + githubOIDCIssuer = "https://token.actions.githubusercontent.com" + + // openAIDefaultPermission is the only permission an agent run needs. + openAIDefaultPermission = "api.model.request" + + // openAIDefaultRef is the ref assertion for the mapping. + openAIDefaultRef = "refs/heads/main" +) + +// --- request command --- + +// openAIRequestDoc is the top-level JSON schema for the request document. +type openAIRequestDoc struct { + Version string `json:"version"` + Provider openAIRequestProvider `json:"provider"` + Mappings []openAIRequestMapping `json:"mappings"` + Reply openAIRequestReply `json:"reply"` +} + +type openAIRequestProvider struct { + Issuer string `json:"issuer"` + Audience string `json:"audience"` + UploadedJWKS bool `json:"uploaded_jwks"` +} + +type openAIRequestMapping struct { + Repository string `json:"repository"` + Assertions openAIRequestAssertions `json:"assertions"` + Target openAIRequestTarget `json:"target"` +} + +type openAIRequestAssertions struct { + Iss string `json:"iss"` + Aud string `json:"aud"` + Repository string `json:"repository"` + Ref string `json:"ref"` +} + +type openAIRequestTarget struct { + Project string `json:"project"` + ServiceAccount string `json:"service_account"` + Permissions []string `json:"permissions"` +} + +type openAIRequestReply struct { + IdentityProviderID string `json:"identity_provider_id"` + Audience string `json:"audience"` + ServiceAccountIDs map[string]string `json:"service_account_ids"` +} + +func newInferenceOpenAICmd() *cobra.Command { + cmd := &cobra.Command{ + Use: "openai", + Short: "Manage OpenAI Workload Identity Federation enrollment", + Long: `Commands for enrolling repositories with OpenAI Workload Identity +Federation. Generate a request document for your administrator, +import the reply into fullsend configuration, and check the +exchange status. + +These commands do not call the OpenAI API. They produce documents +and update local configuration.`, + } + cmd.AddCommand(newInferenceOpenAIRequestCmd()) + cmd.AddCommand(newInferenceOpenAIImportCmd()) + cmd.AddCommand(newInferenceOpenAIStatusCmd()) + return cmd +} + +func newInferenceOpenAIRequestCmd() *cobra.Command { + var audience string + var project string + var serviceAccount string + var format string + var outFile string + + cmd := &cobra.Command{ + Use: "request [,…]", + Short: "Generate OpenAI WIF provider/mapping request", + Long: `Generates the request document an administrator needs to enable +OpenAI Workload Identity Federation for one or more repositories. + +Every value in the document is computed from the repository names. +Nothing is sent anywhere; the command needs no credentials. + +Output formats: + --format json A stable, versioned JSON schema suitable for a + future API submission. + --format md A copy-paste ticket/email matching the guide's + route-B template.`, + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + repos, err := parseRepoList(args[0]) + if err != nil { + return err + } + if len(repos) == 0 { + return fmt.Errorf("at least one owner/repo is required") + } + + // Derive owner from first repo for default audience. + owner := strings.SplitN(repos[0], "/", 2)[0] + if audience == "" { + audience = defaultOpenAIAudiencePrefix + owner + } + + doc := buildRequestDoc(repos, audience, project, serviceAccount) + + var output string + switch format { + case "json": + b, err := json.MarshalIndent(doc, "", " ") + if err != nil { + return fmt.Errorf("marshaling request JSON: %w", err) + } + output = string(b) + "\n" + case "md": + output, err = renderRequestMarkdown(doc) + if err != nil { + return fmt.Errorf("rendering request markdown: %w", err) + } + default: + return fmt.Errorf("--format must be one of: json, md (got %q)", format) + } + + if outFile != "" { + if err := os.WriteFile(outFile, []byte(output), 0o644); err != nil { + return fmt.Errorf("writing output to %s: %w", outFile, err) + } + printer := ui.New(cmd.OutOrStdout()) + printer.StepDone(fmt.Sprintf("Request written to %s", outFile)) + return nil + } + + fmt.Fprint(cmd.OutOrStdout(), output) + return nil + }, + } + + cmd.Flags().StringVar(&audience, "audience", "", "OpenAI Workload Identity audience (default: fullsend://)") + cmd.Flags().StringVar(&project, "project", "", "OpenAI project name or ID for the service accounts") + cmd.Flags().StringVar(&serviceAccount, "service-account", "", "existing service account ID to map (default: create fullsend--ci per repo)") + cmd.Flags().StringVar(&format, "format", "json", "output format: json, md") + cmd.Flags().StringVar(&outFile, "out", "", "write output to a file instead of stdout") + + return cmd +} + +// parseRepoList splits a comma-separated list of owner/repo arguments +// and validates each one. +func parseRepoList(arg string) ([]string, error) { + parts := strings.Split(arg, ",") + var repos []string + for _, p := range parts { + p = strings.TrimSpace(p) + if p == "" { + continue + } + if !strings.Contains(p, "/") { + return nil, fmt.Errorf("expected owner/repo format, got %q (org-only targets are not supported; specify the repository)", p) + } + _, _, err := parseOrgOrRepo(p) + if err != nil { + return nil, err + } + repos = append(repos, p) + } + return repos, nil +} + +// defaultServiceAccountID derives the default service account name for +// a repository: fullsend--ci. +func defaultServiceAccountID(repo string) string { + parts := strings.SplitN(repo, "/", 2) + repoName := parts[1] + return "fullsend-" + repoName + "-ci" +} + +func buildRequestDoc(repos []string, audience, project, serviceAccount string) openAIRequestDoc { + doc := openAIRequestDoc{ + Version: openAIRequestSchemaVersion, + Provider: openAIRequestProvider{ + Issuer: githubOIDCIssuer, + Audience: audience, + UploadedJWKS: false, + }, + Reply: openAIRequestReply{ + IdentityProviderID: "", + Audience: audience, + ServiceAccountIDs: make(map[string]string), + }, + } + + for _, repo := range repos { + sa := serviceAccount + if sa == "" { + sa = defaultServiceAccountID(repo) + } + + mapping := openAIRequestMapping{ + Repository: repo, + Assertions: openAIRequestAssertions{ + Iss: githubOIDCIssuer, + Aud: audience, + Repository: repo, + Ref: openAIDefaultRef, + }, + Target: openAIRequestTarget{ + Project: project, + ServiceAccount: sa, + Permissions: []string{openAIDefaultPermission}, + }, + } + doc.Mappings = append(doc.Mappings, mapping) + doc.Reply.ServiceAccountIDs[repo] = "" + } + + return doc +} + +// requestMarkdownTmpl matches the guide's route-B template structure. +var requestMarkdownTmpl = template.Must(template.New("request").Parse(`# OpenAI Workload Identity Federation Request + +## Provider (reuse or create) + +| Field | Value | +|---|---| +| OIDC issuer URL | ` + "`" + `{{ .Provider.Issuer }}` + "`" + ` | +| Audience | ` + "`" + `{{ .Provider.Audience }}` + "`" + ` | +| Use uploaded JWKS for token verification | **Off** | + +If the organization already has a provider with a different audience, +keep its audience and report it back (see Reply below). + +## Service account mappings + +One mapping per repository. Every assertion must be exact — no wildcards, +no ` + "`" + `repository_owner` + "`" + `, no ` + "`" + `workflow_ref` + "`" + ` (fullsend starts agent runs +from seven workflow files). Do **not** create an API key for the service account. +{{ range .Mappings }} +### {{ .Repository }} + +| Field | Value | +|---|---| +| Claim assertions | ` + "`" + `iss` + "`" + ` = ` + "`" + `{{ .Assertions.Iss }}` + "`" + ` · ` + "`" + `aud` + "`" + ` = ` + "`" + `{{ .Assertions.Aud }}` + "`" + ` · ` + "`" + `repository` + "`" + ` = ` + "`" + `{{ .Assertions.Repository }}` + "`" + ` · ` + "`" + `ref` + "`" + ` = ` + "`" + `{{ .Assertions.Ref }}` + "`" + ` | +| Project | {{ if .Target.Project }}` + "`" + `{{ .Target.Project }}` + "`" + `{{ else }}*(specify the project name or ID)*{{ end }} | +| Service account | {{ .Target.ServiceAccount }} (create inline in the mapping) | +| Permissions | ` + "`" + `{{ index .Target.Permissions 0 }}` + "`" + ` only | +{{ end }} +## Reply + +Please provide the following identifiers so we can configure the repository: + +| Identifier | Value | +|---|---| +| Identity provider ID | *(from the provider you created or reused)* | +| Provider's audience | ` + "`" + `{{ .Reply.Audience }}` + "`" + ` *(confirm or update if different)* | +{{ range .Mappings }}| Service account ID for {{ .Repository }} | *(from the mapping above)* | +{{ end }} +These identifiers are not secrets — they grant nothing on their own. +The mapping only trusts a GitHub OIDC token whose claims match. +`)) + +func renderRequestMarkdown(doc openAIRequestDoc) (string, error) { + var sb strings.Builder + if err := requestMarkdownTmpl.Execute(&sb, doc); err != nil { + return "", err + } + return sb.String(), nil +} + +// --- import command --- + +// openAIReplyDoc is the JSON shape accepted by the import command. It +// mirrors the request schema's reply section so an admin who fills the +// JSON in can be imported directly. +type openAIReplyDoc struct { + IdentityProviderID string `json:"identity_provider_id"` + Audience string `json:"audience"` + ServiceAccountID string `json:"service_account_id,omitempty"` + ServiceAccountIDs map[string]string `json:"service_account_ids,omitempty"` +} + +func newInferenceOpenAIImportCmd() *cobra.Command { + var ( + flagAudience string + flagIdentityProviderID string + flagServiceAccountID string + fullsendDir string + variables bool + repo string + ) + + cmd := &cobra.Command{ + Use: "import [reply.json]", + Short: "Import OpenAI WIF identifiers into fullsend config", + Long: `Takes the administrator's reply and writes inference.openai into +.fullsend/config.yaml through the same setters as +'fullsend github setup --openai-*'. + +The reply can be provided as a JSON file argument or via flags. +All three identifiers (audience, identity-provider-id, +service-account-id) must be present — a partial trio is refused. + +With --variables, sets the three FULLSEND_OPENAI_* repository +variables instead of writing to config.yaml (requires a GitHub +token with variable-write permissions and --repo).`, + Args: cobra.MaximumNArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + printer := ui.New(cmd.OutOrStdout()) + + ids, err := resolveImportIDs(args, flagAudience, flagIdentityProviderID, flagServiceAccountID) + if err != nil { + return err + } + + if err := validateImportIDs(ids); err != nil { + return err + } + + if variables { + return runImportVariables(printer, ids, repo) + } + + return runImportConfig(printer, ids, fullsendDir) + }, + } + + cmd.Flags().StringVar(&flagAudience, "audience", "", "OpenAI Workload Identity audience") + cmd.Flags().StringVar(&flagIdentityProviderID, "identity-provider-id", "", "OpenAI identity provider ID") + cmd.Flags().StringVar(&flagServiceAccountID, "service-account-id", "", "OpenAI service account ID") + cmd.Flags().StringVar(&fullsendDir, "fullsend-dir", ".fullsend", "path to the .fullsend configuration directory") + cmd.Flags().BoolVar(&variables, "variables", false, "set FULLSEND_OPENAI_* repository variables instead of writing config.yaml (requires --repo)") + cmd.Flags().StringVar(&repo, "repo", "", "target repository (owner/repo) for --variables") + + return cmd +} + +// resolveImportIDs takes the command arguments and flags and returns the +// OpenAI WIF config. Flags take precedence over the JSON file. +func resolveImportIDs(args []string, flagAudience, flagIdentityProviderID, flagServiceAccountID string) (config.OpenAIWIFConfig, error) { + var ids config.OpenAIWIFConfig + + // Load from JSON file if provided. + if len(args) == 1 { + data, err := os.ReadFile(args[0]) + if err != nil { + return ids, fmt.Errorf("reading reply file: %w", err) + } + var reply openAIReplyDoc + if err := json.Unmarshal(data, &reply); err != nil { + return ids, fmt.Errorf("parsing reply JSON: %w", err) + } + ids.Audience = reply.Audience + ids.IdentityProviderID = reply.IdentityProviderID + // Single service_account_id takes precedence; for multi-repo + // replies the caller must use --service-account-id to select. + if reply.ServiceAccountID != "" { + ids.ServiceAccountID = reply.ServiceAccountID + } else if len(reply.ServiceAccountIDs) == 1 { + for _, v := range reply.ServiceAccountIDs { + ids.ServiceAccountID = v + } + } + } + + // Flags override file values. + if flagAudience != "" { + ids.Audience = flagAudience + } + if flagIdentityProviderID != "" { + ids.IdentityProviderID = flagIdentityProviderID + } + if flagServiceAccountID != "" { + ids.ServiceAccountID = flagServiceAccountID + } + + return ids.Trimmed(), nil +} + +// validateImportIDs enforces the all-three-or-none rule. +func validateImportIDs(ids config.OpenAIWIFConfig) error { + if ids.IsZero() { + return fmt.Errorf("no identifiers provided: pass a reply JSON file or all three flags (--audience, --identity-provider-id, --service-account-id)") + } + if missing := ids.Missing(); len(missing) > 0 { + return fmt.Errorf("--audience, --identity-provider-id, and --service-account-id must all be set (missing %s)", strings.Join(missing, ", ")) + } + return nil +} + +func runImportConfig(printer *ui.Printer, ids config.OpenAIWIFConfig, fullsendDir string) error { + writer, err := config.LoadConfigWriter(fullsendDir, config.LoadOpts{MissingOK: true}) + if err != nil { + return fmt.Errorf("loading config from %s: %w", fullsendDir, err) + } + + perRepo, ok := writer.(config.PerRepoConfigWriter) + if !ok { + return fmt.Errorf("inference openai import writes per-repo config; %s contains an org-mode config", fullsendDir) + } + + perRepo.SetInferenceOpenAI(ids) + + data, err := perRepo.Marshal() + if err != nil { + return fmt.Errorf("marshaling config: %w", err) + } + + configPath := filepath.Join(fullsendDir, "config.yaml") + if err := os.MkdirAll(fullsendDir, 0o755); err != nil { + return fmt.Errorf("creating directory %s: %w", fullsendDir, err) + } + if err := os.WriteFile(configPath, data, 0o644); err != nil { + return fmt.Errorf("writing config to %s: %w", configPath, err) + } + + printer.StepDone("Wrote inference.openai to " + configPath) + printer.KeyValue("audience", ids.Audience) + printer.KeyValue("identity_provider_id", ids.IdentityProviderID) + printer.KeyValue("service_account_id", ids.ServiceAccountID) + + return nil +} + +// ghRunner runs a gh CLI command and returns its combined output. Tests +// can replace this variable with a stub. +var ghRunner = func(args ...string) (string, error) { + cmd := exec.Command("gh", args...) + out, err := cmd.CombinedOutput() + return string(out), err +} + +func runImportVariables(printer *ui.Printer, ids config.OpenAIWIFConfig, repo string) error { + if repo == "" { + return fmt.Errorf("--repo is required when using --variables") + } + if !strings.Contains(repo, "/") { + return fmt.Errorf("--repo must be in owner/repo format") + } + + parts := strings.SplitN(repo, "/", 2) + owner, repoName := parts[0], parts[1] + + if !githubOwnerPattern.MatchString(owner) { + return fmt.Errorf("invalid owner name %q", owner) + } + if !githubRepoPattern.MatchString(repoName) { + return fmt.Errorf("invalid repo name %q", repoName) + } + + type varEntry struct { + name string + value string + } + vars := []varEntry{ + {openAIAudienceEnv, ids.Audience}, + {openAIIdentityProviderIDEnv, ids.IdentityProviderID}, + {openAIServiceAccountIDEnv, ids.ServiceAccountID}, + } + + for _, v := range vars { + ghArgs := []string{"variable", "set", v.name, "--repo", repo, "--body", v.value} + printer.StepStart(fmt.Sprintf("Setting %s on %s", v.name, repo)) + out, err := ghRunner(ghArgs...) + if err != nil { + printer.StepFail(fmt.Sprintf("Failed to set %s", v.name)) + return fmt.Errorf("setting variable %s on %s: %w\n%s", v.name, repo, err, out) + } + printer.StepDone(fmt.Sprintf("Set %s on %s", v.name, repo)) + } + + return nil +} + +// --- status command --- + +func newInferenceOpenAIStatusCmd() *cobra.Command { + var fullsendDir string + + cmd := &cobra.Command{ + Use: "status ", + Short: "Check OpenAI WIF configuration and exchange status", + Long: `Prints the resolved OpenAI WIF identifiers and their source +(config.yaml or environment variables), and flags a partial trio. + +When run inside a GitHub Actions job with id-token: write, performs +one exchange through internal/inference/openaiwif and reports the +returned scope and expiry (the same code path as 'fullsend run') +without ever printing the token. Outside Actions, says so and stops +at the config checks.`, + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + _, repo, err := parseOrgOrRepo(args[0]) + if err != nil { + return err + } + if repo == "" { + return fmt.Errorf("expected owner/repo format, got org-only %q", args[0]) + } + + printer := ui.New(cmd.OutOrStdout()) + return runInferenceOpenAIStatus(cmd, printer, repo, fullsendDir) + }, + } + + cmd.Flags().StringVar(&fullsendDir, "fullsend-dir", ".fullsend", "path to the .fullsend configuration directory") + + return cmd +} + +// openAIStatusSource describes where an identifier was resolved from. +type openAIStatusSource struct { + Audience string + AudienceSource string + IdentityProviderID string + IDPSource string + ServiceAccountID string + SASource string +} + +func resolveOpenAIStatusSources(fullsendDir string) openAIStatusSource { + var s openAIStatusSource + + // Check environment variables first (they take precedence). + envAud := strings.TrimSpace(os.Getenv(openAIAudienceEnv)) + envIDP := strings.TrimSpace(os.Getenv(openAIIdentityProviderIDEnv)) + envSA := strings.TrimSpace(os.Getenv(openAIServiceAccountIDEnv)) + + // Load config as fallback. + var cfgIDs config.OpenAIWIFConfig + writer, err := config.LoadConfigWriter(fullsendDir, config.LoadOpts{MissingOK: true}) + if err == nil { + if perRepo, ok := writer.(config.PerRepoConfigReader); ok { + cfgIDs = perRepo.ConfigInferenceOpenAI().Trimmed() + } + } + + // Resolve each field: env var wins, then config. + if envAud != "" { + s.Audience = envAud + s.AudienceSource = "variable " + openAIAudienceEnv + } else if cfgIDs.Audience != "" { + s.Audience = cfgIDs.Audience + s.AudienceSource = "config.yaml" + } + + if envIDP != "" { + s.IdentityProviderID = envIDP + s.IDPSource = "variable " + openAIIdentityProviderIDEnv + } else if cfgIDs.IdentityProviderID != "" { + s.IdentityProviderID = cfgIDs.IdentityProviderID + s.IDPSource = "config.yaml" + } + + if envSA != "" { + s.ServiceAccountID = envSA + s.SASource = "variable " + openAIServiceAccountIDEnv + } else if cfgIDs.ServiceAccountID != "" { + s.ServiceAccountID = cfgIDs.ServiceAccountID + s.SASource = "config.yaml" + } + + return s +} + +func runInferenceOpenAIStatus(cmd *cobra.Command, printer *ui.Printer, repo, fullsendDir string) error { + printer.Banner(Version()) + printer.Blank() + printer.Header("OpenAI WIF Status: " + repo) + printer.Blank() + + sources := resolveOpenAIStatusSources(fullsendDir) + + // Print resolved identifiers. + printOpenAIStatusField(printer, "audience", sources.Audience, sources.AudienceSource) + printOpenAIStatusField(printer, "identity_provider_id", sources.IdentityProviderID, sources.IDPSource) + printOpenAIStatusField(printer, "service_account_id", sources.ServiceAccountID, sources.SASource) + printer.Blank() + + // Check completeness. + ids := config.OpenAIWIFConfig{ + Audience: sources.Audience, + IdentityProviderID: sources.IdentityProviderID, + ServiceAccountID: sources.ServiceAccountID, + } + + if ids.IsZero() { + printer.StepFail("No OpenAI WIF identifiers configured") + printer.StepInfo("Run 'fullsend inference openai import' or 'fullsend github setup --openai-*' to configure") + return nil + } + + if missing := ids.Missing(); len(missing) > 0 { + printer.StepWarn(fmt.Sprintf("Partial trio: missing %s", strings.Join(missing, ", "))) + printer.StepInfo("All three identifiers must be set for the exchange to work") + return nil + } + + printer.StepDone("All three identifiers are set") + printer.Blank() + + // Check if we're inside GitHub Actions with OIDC. + oidcURL := os.Getenv("ACTIONS_ID_TOKEN_REQUEST_URL") + oidcToken := os.Getenv("ACTIONS_ID_TOKEN_REQUEST_TOKEN") + + if oidcURL == "" || oidcToken == "" { + printer.StepInfo("Not inside a GitHub Actions job with id-token: write") + printer.StepInfo("The exchange can only be tested from a GitHub Actions workflow") + return nil + } + + // Perform the exchange. + printer.StepStart("Performing WIF exchange") + tok, err := openAIExchange(cmd.Context(), openaiwif.Config{ + Audience: ids.Audience, + IdentityProviderID: ids.IdentityProviderID, + ServiceAccountID: ids.ServiceAccountID, + OIDCRequestURL: oidcURL, + OIDCRequestToken: oidcToken, + }) + if err != nil { + printer.StepFail("Exchange failed") + // Never print the token in error messages — openaiwif already + // ensures that, but be explicit. + return fmt.Errorf("OpenAI WIF exchange: %w", err) + } + + printer.StepDone("Exchange succeeded") + printer.Blank() + + scope := tok.Scope + if scope == "" { + scope = "(not narrowed)" + } + printer.KeyValue("scope", scope) + printer.KeyValue("expires_in", time.Until(tok.ExpiresAt).Round(time.Second).String()) + + if warning, err := checkOpenAIScope(tok.Scope); err != nil { + printer.StepWarn(err.Error()) + } else if warning != "" { + printer.StepWarn(warning) + } + + return nil +} + +func printOpenAIStatusField(printer *ui.Printer, name, value, source string) { + if value == "" { + printer.StepInfo(fmt.Sprintf("%s: (not set)", name)) + } else { + printer.StepInfo(fmt.Sprintf("%s: %s (from %s)", name, value, source)) + } +} diff --git a/internal/cli/inference_openai_test.go b/internal/cli/inference_openai_test.go new file mode 100644 index 0000000000..6978e6a434 --- /dev/null +++ b/internal/cli/inference_openai_test.go @@ -0,0 +1,859 @@ +package cli + +import ( + "bytes" + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/fullsend-ai/fullsend/internal/config" + "github.com/fullsend-ai/fullsend/internal/ui" +) + +// --- openai parent command --- + +func TestInferenceOpenAICmd_HasSubcommands(t *testing.T) { + cmd := newInferenceOpenAICmd() + names := make(map[string]bool) + for _, sub := range cmd.Commands() { + names[sub.Name()] = true + } + assert.True(t, names["request"], "expected request subcommand") + assert.True(t, names["import"], "expected import subcommand") + assert.True(t, names["status"], "expected status subcommand") +} + +func TestInferenceOpenAICmd_RegisteredInInference(t *testing.T) { + cmd := newInferenceCmd() + found := false + for _, sub := range cmd.Commands() { + if sub.Use == "openai" { + found = true + break + } + } + assert.True(t, found, "expected openai subcommand registered in inference") +} + +// --- request command tests --- + +func TestInferenceOpenAIRequestCmd_RequiresArg(t *testing.T) { + cmd := newRootCmd() + cmd.SetArgs([]string{"inference", "openai", "request"}) + err := cmd.Execute() + require.Error(t, err) + assert.Contains(t, err.Error(), "accepts 1 arg(s)") +} + +func TestInferenceOpenAIRequestCmd_RejectsInvalidFormat(t *testing.T) { + cmd := newRootCmd() + cmd.SetArgs([]string{"inference", "openai", "request", "acme/widget", + "--format", "yaml"}) + err := cmd.Execute() + require.Error(t, err) + assert.Contains(t, err.Error(), "--format must be one of: json, md") +} + +func TestInferenceOpenAIRequestCmd_RejectsOrgOnlyTarget(t *testing.T) { + cmd := newRootCmd() + cmd.SetArgs([]string{"inference", "openai", "request", "acme"}) + err := cmd.Execute() + require.Error(t, err) + assert.Contains(t, err.Error(), "owner/repo") +} + +func TestInferenceOpenAIRequestCmd_JSONSingleRepo(t *testing.T) { + var buf bytes.Buffer + cmd := newRootCmd() + cmd.SetOut(&buf) + cmd.SetArgs([]string{"inference", "openai", "request", "acme/widget", + "--format", "json"}) + err := cmd.Execute() + require.NoError(t, err) + + var doc openAIRequestDoc + require.NoError(t, json.Unmarshal(buf.Bytes(), &doc)) + + assert.Equal(t, openAIRequestSchemaVersion, doc.Version) + assert.Equal(t, githubOIDCIssuer, doc.Provider.Issuer) + assert.Equal(t, "fullsend://acme", doc.Provider.Audience) + assert.False(t, doc.Provider.UploadedJWKS) + + require.Len(t, doc.Mappings, 1) + m := doc.Mappings[0] + assert.Equal(t, "acme/widget", m.Repository) + assert.Equal(t, githubOIDCIssuer, m.Assertions.Iss) + assert.Equal(t, "fullsend://acme", m.Assertions.Aud) + assert.Equal(t, "acme/widget", m.Assertions.Repository) + assert.Equal(t, "refs/heads/main", m.Assertions.Ref) + assert.Equal(t, "fullsend-widget-ci", m.Target.ServiceAccount) + assert.Equal(t, []string{"api.model.request"}, m.Target.Permissions) + assert.Empty(t, m.Target.Project) // no --project flag + + assert.Equal(t, "fullsend://acme", doc.Reply.Audience) + assert.Empty(t, doc.Reply.IdentityProviderID) + assert.Contains(t, doc.Reply.ServiceAccountIDs, "acme/widget") +} + +func TestInferenceOpenAIRequestCmd_JSONMultiRepo(t *testing.T) { + var buf bytes.Buffer + cmd := newRootCmd() + cmd.SetOut(&buf) + cmd.SetArgs([]string{"inference", "openai", "request", + "acme/widget,acme/gadget", + "--format", "json"}) + err := cmd.Execute() + require.NoError(t, err) + + var doc openAIRequestDoc + require.NoError(t, json.Unmarshal(buf.Bytes(), &doc)) + + require.Len(t, doc.Mappings, 2) + assert.Equal(t, "acme/widget", doc.Mappings[0].Repository) + assert.Equal(t, "acme/gadget", doc.Mappings[1].Repository) + + assert.Equal(t, "fullsend-widget-ci", doc.Mappings[0].Target.ServiceAccount) + assert.Equal(t, "fullsend-gadget-ci", doc.Mappings[1].Target.ServiceAccount) + + assert.Len(t, doc.Reply.ServiceAccountIDs, 2) +} + +func TestInferenceOpenAIRequestCmd_CustomAudienceAndProject(t *testing.T) { + var buf bytes.Buffer + cmd := newRootCmd() + cmd.SetOut(&buf) + cmd.SetArgs([]string{"inference", "openai", "request", "acme/widget", + "--audience", "https://custom.audience", + "--project", "my-openai-project", + "--format", "json"}) + err := cmd.Execute() + require.NoError(t, err) + + var doc openAIRequestDoc + require.NoError(t, json.Unmarshal(buf.Bytes(), &doc)) + + assert.Equal(t, "https://custom.audience", doc.Provider.Audience) + assert.Equal(t, "https://custom.audience", doc.Mappings[0].Assertions.Aud) + assert.Equal(t, "my-openai-project", doc.Mappings[0].Target.Project) +} + +func TestInferenceOpenAIRequestCmd_CustomServiceAccount(t *testing.T) { + var buf bytes.Buffer + cmd := newRootCmd() + cmd.SetOut(&buf) + cmd.SetArgs([]string{"inference", "openai", "request", "acme/widget", + "--service-account", "existing-sa-id", + "--format", "json"}) + err := cmd.Execute() + require.NoError(t, err) + + var doc openAIRequestDoc + require.NoError(t, json.Unmarshal(buf.Bytes(), &doc)) + + assert.Equal(t, "existing-sa-id", doc.Mappings[0].Target.ServiceAccount) +} + +func TestInferenceOpenAIRequestCmd_MarkdownOutput(t *testing.T) { + var buf bytes.Buffer + cmd := newRootCmd() + cmd.SetOut(&buf) + cmd.SetArgs([]string{"inference", "openai", "request", "acme/widget", + "--format", "md"}) + err := cmd.Execute() + require.NoError(t, err) + + output := buf.String() + assert.Contains(t, output, "# OpenAI Workload Identity Federation Request") + assert.Contains(t, output, "acme/widget") + assert.Contains(t, output, githubOIDCIssuer) + assert.Contains(t, output, "fullsend://acme") + assert.Contains(t, output, "fullsend-widget-ci") + assert.Contains(t, output, "api.model.request") + assert.Contains(t, output, "Identity provider ID") + assert.Contains(t, output, "Service account ID for acme/widget") +} + +func TestInferenceOpenAIRequestCmd_OutFile(t *testing.T) { + dir := t.TempDir() + outPath := filepath.Join(dir, "request.json") + + cmd := newRootCmd() + cmd.SetArgs([]string{"inference", "openai", "request", "acme/widget", + "--format", "json", + "--out", outPath}) + err := cmd.Execute() + require.NoError(t, err) + + data, err := os.ReadFile(outPath) + require.NoError(t, err) + + var doc openAIRequestDoc + require.NoError(t, json.Unmarshal(data, &doc)) + assert.Equal(t, "acme/widget", doc.Mappings[0].Repository) +} + +// --- parseRepoList tests --- + +func TestParseRepoList_SingleRepo(t *testing.T) { + repos, err := parseRepoList("acme/widget") + require.NoError(t, err) + assert.Equal(t, []string{"acme/widget"}, repos) +} + +func TestParseRepoList_MultipleRepos(t *testing.T) { + repos, err := parseRepoList("acme/widget, acme/gadget") + require.NoError(t, err) + assert.Equal(t, []string{"acme/widget", "acme/gadget"}, repos) +} + +func TestParseRepoList_RejectsOrgOnly(t *testing.T) { + _, err := parseRepoList("acme") + require.Error(t, err) + assert.Contains(t, err.Error(), "owner/repo") +} + +func TestParseRepoList_RejectsEmpty(t *testing.T) { + repos, err := parseRepoList("") + require.NoError(t, err) + assert.Empty(t, repos) +} + +func TestParseRepoList_RejectsInvalidRepo(t *testing.T) { + _, err := parseRepoList("acme/") + require.Error(t, err) + assert.Contains(t, err.Error(), "invalid") +} + +// --- defaultServiceAccountID tests --- + +func TestDefaultServiceAccountID(t *testing.T) { + assert.Equal(t, "fullsend-widget-ci", defaultServiceAccountID("acme/widget")) + assert.Equal(t, "fullsend-gadget-ci", defaultServiceAccountID("acme/gadget")) + assert.Equal(t, "fullsend-my-repo-ci", defaultServiceAccountID("org/my-repo")) +} + +// --- import command tests --- + +func TestInferenceOpenAIImportCmd_Flags(t *testing.T) { + cmd := newInferenceOpenAIImportCmd() + + assert.NotNil(t, cmd.Flags().Lookup("audience")) + assert.NotNil(t, cmd.Flags().Lookup("identity-provider-id")) + assert.NotNil(t, cmd.Flags().Lookup("service-account-id")) + assert.NotNil(t, cmd.Flags().Lookup("fullsend-dir")) + assert.NotNil(t, cmd.Flags().Lookup("variables")) + assert.NotNil(t, cmd.Flags().Lookup("repo")) +} + +func TestResolveImportIDs_FromFlags(t *testing.T) { + ids, err := resolveImportIDs(nil, "aud", "idp", "sa") + require.NoError(t, err) + assert.Equal(t, config.OpenAIWIFConfig{ + Audience: "aud", + IdentityProviderID: "idp", + ServiceAccountID: "sa", + }, ids) +} + +func TestResolveImportIDs_FromFile(t *testing.T) { + dir := t.TempDir() + replyPath := filepath.Join(dir, "reply.json") + reply := openAIReplyDoc{ + Audience: "fullsend://acme", + IdentityProviderID: "idp_123", + ServiceAccountID: "sa_456", + } + data, err := json.Marshal(reply) + require.NoError(t, err) + require.NoError(t, os.WriteFile(replyPath, data, 0o644)) + + ids, err := resolveImportIDs([]string{replyPath}, "", "", "") + require.NoError(t, err) + assert.Equal(t, "fullsend://acme", ids.Audience) + assert.Equal(t, "idp_123", ids.IdentityProviderID) + assert.Equal(t, "sa_456", ids.ServiceAccountID) +} + +func TestResolveImportIDs_FromFileWithSingleServiceAccountIDs(t *testing.T) { + dir := t.TempDir() + replyPath := filepath.Join(dir, "reply.json") + reply := openAIReplyDoc{ + Audience: "fullsend://acme", + IdentityProviderID: "idp_123", + ServiceAccountIDs: map[string]string{"acme/widget": "sa_widget"}, + } + data, err := json.Marshal(reply) + require.NoError(t, err) + require.NoError(t, os.WriteFile(replyPath, data, 0o644)) + + ids, err := resolveImportIDs([]string{replyPath}, "", "", "") + require.NoError(t, err) + assert.Equal(t, "sa_widget", ids.ServiceAccountID) +} + +func TestResolveImportIDs_FlagsOverrideFile(t *testing.T) { + dir := t.TempDir() + replyPath := filepath.Join(dir, "reply.json") + reply := openAIReplyDoc{ + Audience: "from-file", + IdentityProviderID: "idp-file", + ServiceAccountID: "sa-file", + } + data, err := json.Marshal(reply) + require.NoError(t, err) + require.NoError(t, os.WriteFile(replyPath, data, 0o644)) + + ids, err := resolveImportIDs([]string{replyPath}, "from-flag", "", "") + require.NoError(t, err) + assert.Equal(t, "from-flag", ids.Audience) + assert.Equal(t, "idp-file", ids.IdentityProviderID) +} + +func TestValidateImportIDs_RefusesPartialTrio(t *testing.T) { + err := validateImportIDs(config.OpenAIWIFConfig{Audience: "aud"}) + require.Error(t, err) + assert.Contains(t, err.Error(), "must all be set") + assert.Contains(t, err.Error(), "identity_provider_id") + assert.Contains(t, err.Error(), "service_account_id") +} + +func TestValidateImportIDs_RefusesEmpty(t *testing.T) { + err := validateImportIDs(config.OpenAIWIFConfig{}) + require.Error(t, err) + assert.Contains(t, err.Error(), "no identifiers provided") +} + +func TestValidateImportIDs_AcceptsComplete(t *testing.T) { + err := validateImportIDs(config.OpenAIWIFConfig{ + Audience: "aud", + IdentityProviderID: "idp", + ServiceAccountID: "sa", + }) + require.NoError(t, err) +} + +func TestRunImportConfig_WritesConfig(t *testing.T) { + dir := t.TempDir() + fullsendDir := filepath.Join(dir, ".fullsend") + + ids := config.OpenAIWIFConfig{ + Audience: "fullsend://acme", + IdentityProviderID: "idp_123", + ServiceAccountID: "sa_456", + } + + var buf bytes.Buffer + printer := newTestPrinter(&buf) + + err := runImportConfig(printer, ids, fullsendDir) + require.NoError(t, err) + + // Verify the config was written. + configPath := filepath.Join(fullsendDir, "config.yaml") + data, err := os.ReadFile(configPath) + require.NoError(t, err) + + cfg, err := config.LoadConfigWriter(fullsendDir, config.LoadOpts{}) + require.NoError(t, err) + perRepo, ok := cfg.(config.PerRepoConfigReader) + require.True(t, ok) + + openai := perRepo.ConfigInferenceOpenAI() + assert.Equal(t, "fullsend://acme", openai.Audience) + assert.Equal(t, "idp_123", openai.IdentityProviderID) + assert.Equal(t, "sa_456", openai.ServiceAccountID) + + // Make sure the file is valid YAML. + assert.NotEmpty(t, data) +} + +func TestRunImportConfig_PreservesExistingConfig(t *testing.T) { + dir := t.TempDir() + fullsendDir := filepath.Join(dir, ".fullsend") + require.NoError(t, os.MkdirAll(fullsendDir, 0o755)) + + // Write a pre-existing config with runtime set. + configPath := filepath.Join(fullsendDir, "config.yaml") + require.NoError(t, os.WriteFile(configPath, []byte("runtime: claude\nversion: \"1\"\n"), 0o644)) + + ids := config.OpenAIWIFConfig{ + Audience: "fullsend://acme", + IdentityProviderID: "idp_123", + ServiceAccountID: "sa_456", + } + + var buf bytes.Buffer + printer := newTestPrinter(&buf) + + err := runImportConfig(printer, ids, fullsendDir) + require.NoError(t, err) + + // Verify both runtime and openai are present. + cfg, err := config.LoadConfigWriter(fullsendDir, config.LoadOpts{}) + require.NoError(t, err) + perRepo, ok := cfg.(config.PerRepoConfigReader) + require.True(t, ok) + assert.Equal(t, "claude", perRepo.ConfigRuntime()) + assert.Equal(t, "fullsend://acme", perRepo.ConfigInferenceOpenAI().Audience) +} + +func TestRunImportVariables_RequiresRepo(t *testing.T) { + ids := config.OpenAIWIFConfig{ + Audience: "aud", + IdentityProviderID: "idp", + ServiceAccountID: "sa", + } + var buf bytes.Buffer + printer := newTestPrinter(&buf) + + err := runImportVariables(printer, ids, "") + require.Error(t, err) + assert.Contains(t, err.Error(), "--repo is required") +} + +func TestRunImportVariables_RequiresOwnerSlashRepo(t *testing.T) { + ids := config.OpenAIWIFConfig{ + Audience: "aud", + IdentityProviderID: "idp", + ServiceAccountID: "sa", + } + var buf bytes.Buffer + printer := newTestPrinter(&buf) + + err := runImportVariables(printer, ids, "acme") + require.Error(t, err) + assert.Contains(t, err.Error(), "owner/repo") +} + +func TestRunImportVariables_CallsGH(t *testing.T) { + // Replace ghRunner with a stub. + var calls [][]string + origGH := ghRunner + ghRunner = func(args ...string) (string, error) { + calls = append(calls, args) + return "", nil + } + defer func() { ghRunner = origGH }() + + ids := config.OpenAIWIFConfig{ + Audience: "fullsend://acme", + IdentityProviderID: "idp_123", + ServiceAccountID: "sa_456", + } + var buf bytes.Buffer + printer := newTestPrinter(&buf) + + err := runImportVariables(printer, ids, "acme/widget") + require.NoError(t, err) + + require.Len(t, calls, 3) + assert.Equal(t, []string{"variable", "set", "FULLSEND_OPENAI_AUDIENCE", "--repo", "acme/widget", "--body", "fullsend://acme"}, calls[0]) + assert.Equal(t, []string{"variable", "set", "FULLSEND_OPENAI_IDENTITY_PROVIDER_ID", "--repo", "acme/widget", "--body", "idp_123"}, calls[1]) + assert.Equal(t, []string{"variable", "set", "FULLSEND_OPENAI_SERVICE_ACCOUNT_ID", "--repo", "acme/widget", "--body", "sa_456"}, calls[2]) +} + +// --- status command tests --- + +func TestInferenceOpenAIStatusCmd_RequiresArg(t *testing.T) { + cmd := newRootCmd() + cmd.SetArgs([]string{"inference", "openai", "status"}) + err := cmd.Execute() + require.Error(t, err) + assert.Contains(t, err.Error(), "accepts 1 arg(s)") +} + +func TestInferenceOpenAIStatusCmd_RejectsOrgOnly(t *testing.T) { + cmd := newRootCmd() + cmd.SetArgs([]string{"inference", "openai", "status", "acme"}) + err := cmd.Execute() + require.Error(t, err) + assert.Contains(t, err.Error(), "owner/repo") +} + +func TestResolveOpenAIStatusSources_FromConfig(t *testing.T) { + dir := t.TempDir() + fullsendDir := filepath.Join(dir, ".fullsend") + require.NoError(t, os.MkdirAll(fullsendDir, 0o755)) + + configYAML := `inference: + openai: + audience: fullsend://acme + identity_provider_id: idp_cfg + service_account_id: sa_cfg +` + require.NoError(t, os.WriteFile( + filepath.Join(fullsendDir, "config.yaml"), + []byte(configYAML), 0o644)) + + // Clear env vars. + t.Setenv(openAIAudienceEnv, "") + t.Setenv(openAIIdentityProviderIDEnv, "") + t.Setenv(openAIServiceAccountIDEnv, "") + + s := resolveOpenAIStatusSources(fullsendDir) + assert.Equal(t, "fullsend://acme", s.Audience) + assert.Equal(t, "config.yaml", s.AudienceSource) + assert.Equal(t, "idp_cfg", s.IdentityProviderID) + assert.Equal(t, "config.yaml", s.IDPSource) + assert.Equal(t, "sa_cfg", s.ServiceAccountID) + assert.Equal(t, "config.yaml", s.SASource) +} + +func TestResolveOpenAIStatusSources_EnvOverridesConfig(t *testing.T) { + dir := t.TempDir() + fullsendDir := filepath.Join(dir, ".fullsend") + require.NoError(t, os.MkdirAll(fullsendDir, 0o755)) + + configYAML := `inference: + openai: + audience: from-config + identity_provider_id: idp_cfg + service_account_id: sa_cfg +` + require.NoError(t, os.WriteFile( + filepath.Join(fullsendDir, "config.yaml"), + []byte(configYAML), 0o644)) + + t.Setenv(openAIAudienceEnv, "from-env") + t.Setenv(openAIIdentityProviderIDEnv, "") + t.Setenv(openAIServiceAccountIDEnv, "") + + s := resolveOpenAIStatusSources(fullsendDir) + assert.Equal(t, "from-env", s.Audience) + assert.Contains(t, s.AudienceSource, "variable") + assert.Equal(t, "idp_cfg", s.IdentityProviderID) + assert.Equal(t, "config.yaml", s.IDPSource) +} + +func TestResolveOpenAIStatusSources_NoConfig(t *testing.T) { + dir := t.TempDir() + fullsendDir := filepath.Join(dir, ".fullsend") + + t.Setenv(openAIAudienceEnv, "") + t.Setenv(openAIIdentityProviderIDEnv, "") + t.Setenv(openAIServiceAccountIDEnv, "") + + s := resolveOpenAIStatusSources(fullsendDir) + assert.Empty(t, s.Audience) + assert.Empty(t, s.IdentityProviderID) + assert.Empty(t, s.ServiceAccountID) +} + +func TestRunInferenceOpenAIStatus_NoConfig(t *testing.T) { + dir := t.TempDir() + fullsendDir := filepath.Join(dir, ".fullsend") + + t.Setenv(openAIAudienceEnv, "") + t.Setenv(openAIIdentityProviderIDEnv, "") + t.Setenv(openAIServiceAccountIDEnv, "") + t.Setenv("ACTIONS_ID_TOKEN_REQUEST_URL", "") + t.Setenv("ACTIONS_ID_TOKEN_REQUEST_TOKEN", "") + + var buf bytes.Buffer + cmd := newRootCmd() + cmd.SetOut(&buf) + cmd.SetArgs([]string{"inference", "openai", "status", "acme/widget", + "--fullsend-dir", fullsendDir}) + err := cmd.Execute() + require.NoError(t, err) + assert.Contains(t, buf.String(), "No OpenAI WIF identifiers configured") +} + +func TestRunInferenceOpenAIStatus_PartialConfig(t *testing.T) { + dir := t.TempDir() + fullsendDir := filepath.Join(dir, ".fullsend") + require.NoError(t, os.MkdirAll(fullsendDir, 0o755)) + + configYAML := `inference: + openai: + audience: fullsend://acme +` + require.NoError(t, os.WriteFile( + filepath.Join(fullsendDir, "config.yaml"), + []byte(configYAML), 0o644)) + + t.Setenv(openAIAudienceEnv, "") + t.Setenv(openAIIdentityProviderIDEnv, "") + t.Setenv(openAIServiceAccountIDEnv, "") + t.Setenv("ACTIONS_ID_TOKEN_REQUEST_URL", "") + t.Setenv("ACTIONS_ID_TOKEN_REQUEST_TOKEN", "") + + var buf bytes.Buffer + cmd := newRootCmd() + cmd.SetOut(&buf) + cmd.SetArgs([]string{"inference", "openai", "status", "acme/widget", + "--fullsend-dir", fullsendDir}) + err := cmd.Execute() + require.NoError(t, err) + assert.Contains(t, buf.String(), "Partial trio") +} + +func TestRunInferenceOpenAIStatus_FullConfigNoActions(t *testing.T) { + dir := t.TempDir() + fullsendDir := filepath.Join(dir, ".fullsend") + require.NoError(t, os.MkdirAll(fullsendDir, 0o755)) + + configYAML := `inference: + openai: + audience: fullsend://acme + identity_provider_id: idp_123 + service_account_id: sa_456 +` + require.NoError(t, os.WriteFile( + filepath.Join(fullsendDir, "config.yaml"), + []byte(configYAML), 0o644)) + + t.Setenv(openAIAudienceEnv, "") + t.Setenv(openAIIdentityProviderIDEnv, "") + t.Setenv(openAIServiceAccountIDEnv, "") + t.Setenv("ACTIONS_ID_TOKEN_REQUEST_URL", "") + t.Setenv("ACTIONS_ID_TOKEN_REQUEST_TOKEN", "") + + var buf bytes.Buffer + cmd := newRootCmd() + cmd.SetOut(&buf) + cmd.SetArgs([]string{"inference", "openai", "status", "acme/widget", + "--fullsend-dir", fullsendDir}) + err := cmd.Execute() + require.NoError(t, err) + + output := buf.String() + assert.Contains(t, output, "All three identifiers are set") + assert.Contains(t, output, "Not inside a GitHub Actions job") +} + +// --- buildRequestDoc tests --- + +func TestBuildRequestDoc_DefaultAudience(t *testing.T) { + doc := buildRequestDoc([]string{"acme/widget"}, "fullsend://acme", "", "") + assert.Equal(t, "fullsend://acme", doc.Provider.Audience) + assert.Equal(t, "fullsend://acme", doc.Reply.Audience) +} + +func TestBuildRequestDoc_CorrectAssertions(t *testing.T) { + doc := buildRequestDoc([]string{"acme/widget", "acme/gadget"}, "fullsend://acme", "proj-1", "") + + require.Len(t, doc.Mappings, 2) + + for _, m := range doc.Mappings { + assert.Equal(t, githubOIDCIssuer, m.Assertions.Iss) + assert.Equal(t, "fullsend://acme", m.Assertions.Aud) + assert.Equal(t, m.Repository, m.Assertions.Repository) + assert.Equal(t, "refs/heads/main", m.Assertions.Ref) + assert.Equal(t, "proj-1", m.Target.Project) + assert.Equal(t, []string{"api.model.request"}, m.Target.Permissions) + } +} + +func TestBuildRequestDoc_ServiceAccountIDPerRepo(t *testing.T) { + doc := buildRequestDoc([]string{"acme/widget", "acme/gadget"}, "aud", "", "") + assert.Equal(t, "fullsend-widget-ci", doc.Mappings[0].Target.ServiceAccount) + assert.Equal(t, "fullsend-gadget-ci", doc.Mappings[1].Target.ServiceAccount) +} + +func TestBuildRequestDoc_SharedServiceAccount(t *testing.T) { + doc := buildRequestDoc([]string{"acme/widget", "acme/gadget"}, "aud", "", "shared-sa") + assert.Equal(t, "shared-sa", doc.Mappings[0].Target.ServiceAccount) + assert.Equal(t, "shared-sa", doc.Mappings[1].Target.ServiceAccount) +} + +// --- renderRequestMarkdown tests --- + +func TestRenderRequestMarkdown_ContainsExpectedSections(t *testing.T) { + doc := buildRequestDoc([]string{"acme/widget"}, "fullsend://acme", "", "") + md, err := renderRequestMarkdown(doc) + require.NoError(t, err) + + assert.Contains(t, md, "## Provider (reuse or create)") + assert.Contains(t, md, "## Service account mappings") + assert.Contains(t, md, "### acme/widget") + assert.Contains(t, md, "## Reply") + assert.Contains(t, md, "not secrets") +} + +// --- end-to-end import via root command --- + +func TestInferenceOpenAIImportCmd_FullFlowViaFlags(t *testing.T) { + dir := t.TempDir() + fullsendDir := filepath.Join(dir, ".fullsend") + + cmd := newRootCmd() + cmd.SetArgs([]string{"inference", "openai", "import", + "--audience", "fullsend://acme", + "--identity-provider-id", "idp_123", + "--service-account-id", "sa_456", + "--fullsend-dir", fullsendDir}) + err := cmd.Execute() + require.NoError(t, err) + + // Verify config was written correctly. + cfg, err := config.LoadConfigWriter(fullsendDir, config.LoadOpts{}) + require.NoError(t, err) + perRepo, ok := cfg.(config.PerRepoConfigReader) + require.True(t, ok) + openai := perRepo.ConfigInferenceOpenAI() + assert.Equal(t, "fullsend://acme", openai.Audience) + assert.Equal(t, "idp_123", openai.IdentityProviderID) + assert.Equal(t, "sa_456", openai.ServiceAccountID) +} + +func TestInferenceOpenAIImportCmd_RefusesPartialTrio(t *testing.T) { + cmd := newRootCmd() + cmd.SetArgs([]string{"inference", "openai", "import", + "--audience", "fullsend://acme"}) + err := cmd.Execute() + require.Error(t, err) + assert.Contains(t, err.Error(), "must all be set") +} + +func TestInferenceOpenAIImportCmd_RefusesNoArgs(t *testing.T) { + cmd := newRootCmd() + cmd.SetArgs([]string{"inference", "openai", "import"}) + err := cmd.Execute() + require.Error(t, err) + assert.Contains(t, err.Error(), "no identifiers provided") +} + +// newTestPrinter creates a ui.Printer that writes to the given buffer. +func newTestPrinter(buf *bytes.Buffer) *ui.Printer { + return ui.New(buf) +} + +// --- helpers --- + +func TestInferenceOpenAIRequestCmd_DoesNotRequireGitHubToken(t *testing.T) { + t.Setenv("GH_TOKEN", "") + t.Setenv("GITHUB_TOKEN", "") + + cmd := newRootCmd() + cmd.SetArgs([]string{"inference", "openai", "request", "acme/widget", + "--format", "json"}) + err := cmd.Execute() + require.NoError(t, err) +} + +func TestInferenceOpenAIStatusCmd_DoesNotRequireGitHubToken(t *testing.T) { + t.Setenv("GH_TOKEN", "") + t.Setenv("GITHUB_TOKEN", "") + t.Setenv(openAIAudienceEnv, "") + t.Setenv(openAIIdentityProviderIDEnv, "") + t.Setenv(openAIServiceAccountIDEnv, "") + t.Setenv("ACTIONS_ID_TOKEN_REQUEST_URL", "") + t.Setenv("ACTIONS_ID_TOKEN_REQUEST_TOKEN", "") + + dir := t.TempDir() + cmd := newRootCmd() + cmd.SetArgs([]string{"inference", "openai", "status", "acme/widget", + "--fullsend-dir", filepath.Join(dir, ".fullsend")}) + err := cmd.Execute() + require.NoError(t, err) // should not fail with "no GitHub token found" +} + +// --- request JSON round-trip test (golden) --- + +func TestBuildRequestDoc_JSONRoundTrip(t *testing.T) { + doc := buildRequestDoc( + []string{"acme/widget", "acme/gadget"}, + "fullsend://acme", + "openai-proj-001", + "", + ) + + b, err := json.MarshalIndent(doc, "", " ") + require.NoError(t, err) + + var roundTrip openAIRequestDoc + require.NoError(t, json.Unmarshal(b, &roundTrip)) + + assert.Equal(t, doc.Version, roundTrip.Version) + assert.Equal(t, doc.Provider, roundTrip.Provider) + assert.Len(t, roundTrip.Mappings, 2) + assert.Equal(t, doc.Mappings[0].Assertions, roundTrip.Mappings[0].Assertions) + assert.Equal(t, doc.Mappings[1].Assertions, roundTrip.Mappings[1].Assertions) + assert.Equal(t, doc.Reply.Audience, roundTrip.Reply.Audience) +} + +// --- import replaces existing openai block --- + +func TestRunImportConfig_ReplacesExistingOpenAI(t *testing.T) { + dir := t.TempDir() + fullsendDir := filepath.Join(dir, ".fullsend") + require.NoError(t, os.MkdirAll(fullsendDir, 0o755)) + + configYAML := `inference: + openai: + audience: old-audience + identity_provider_id: old-idp + service_account_id: old-sa +` + require.NoError(t, os.WriteFile( + filepath.Join(fullsendDir, "config.yaml"), + []byte(configYAML), 0o644)) + + ids := config.OpenAIWIFConfig{ + Audience: "new-audience", + IdentityProviderID: "new-idp", + ServiceAccountID: "new-sa", + } + + var buf bytes.Buffer + printer := newTestPrinter(&buf) + + err := runImportConfig(printer, ids, fullsendDir) + require.NoError(t, err) + + // Read back and verify the new values. + cfg, err := config.LoadConfigWriter(fullsendDir, config.LoadOpts{}) + require.NoError(t, err) + perRepo, ok := cfg.(config.PerRepoConfigReader) + require.True(t, ok) + openai := perRepo.ConfigInferenceOpenAI() + assert.Equal(t, "new-audience", openai.Audience) + assert.Equal(t, "new-idp", openai.IdentityProviderID) + assert.Equal(t, "new-sa", openai.ServiceAccountID) + + // Verify old values are gone. + raw, err := os.ReadFile(filepath.Join(fullsendDir, "config.yaml")) + require.NoError(t, err) + assert.NotContains(t, string(raw), "old-audience") +} + +// --- status with env vars from flags --- + +func TestInferenceOpenAIStatusCmd_Flags(t *testing.T) { + cmd := newInferenceOpenAIStatusCmd() + assert.NotNil(t, cmd.Flags().Lookup("fullsend-dir")) +} + +// --- request multi-repo with whitespace --- + +func TestParseRepoList_TrimsWhitespace(t *testing.T) { + repos, err := parseRepoList(" acme/widget , acme/gadget ") + require.NoError(t, err) + assert.Equal(t, []string{"acme/widget", "acme/gadget"}, repos) +} + +// Verify that Markdown output for multi-repo request contains all repos. +func TestInferenceOpenAIRequestCmd_MarkdownMultiRepo(t *testing.T) { + var buf bytes.Buffer + cmd := newRootCmd() + cmd.SetOut(&buf) + cmd.SetArgs([]string{"inference", "openai", "request", + "acme/widget,acme/gadget", + "--format", "md"}) + err := cmd.Execute() + require.NoError(t, err) + + output := buf.String() + assert.Contains(t, output, "### acme/widget") + assert.Contains(t, output, "### acme/gadget") + assert.Contains(t, output, "fullsend-widget-ci") + assert.Contains(t, output, "fullsend-gadget-ci") + assert.True(t, strings.Contains(output, "Service account ID for acme/widget")) + assert.True(t, strings.Contains(output, "Service account ID for acme/gadget")) +} From 0cf11b59229c16421bb84cf2b9bb815c7b7f30c1 Mon Sep 17 00:00:00 2001 From: Wayne Sun Date: Sun, 30 Aug 2026 12:16:21 -0400 Subject: [PATCH 2/5] docs(#6778): fold the two-route WIF guide into the command family Supersedes #6777, which rewrote the operator guide's setup half while this branch was being written. The guide read as if managing the organization's identity provider were a prerequisite; in most companies an IT administrator owns Organization Settings and the GitHub Actions provider, and a team owns (or requests) a project. It is now an explicit fork: shared claims check first, then route A (you manage providers) or route B (send the administrator a request), with common steps from "tell fullsend the three identifiers" on. Trust is stated as per repository for both routes: one mapping per company-owned repository, never a wildcard or a pattern over the organization, since an organization can contain repositories the company does not own. Mappings match exact values only and have no list form. The new commands are wired into that structure rather than bolted onto the old sections: route A points at `request` for the console fields, route B generates its ticket with `request --format md` (the hand-written template stays for readers who prefer it), B3 records the reply with `import` and checks it with `status`, and step 4 keeps a shortcut note. Also fixes the pi runtime doc's "three repository variables" wording. Assisted-by: Claude Signed-off-by: Wayne Sun --- .../openai-workload-identity.md | 234 ++++++++++++------ docs/runtimes/pi.md | 3 +- 2 files changed, 166 insertions(+), 71 deletions(-) diff --git a/docs/guides/infrastructure/openai-workload-identity.md b/docs/guides/infrastructure/openai-workload-identity.md index 673bf2ef1d..72e910f927 100644 --- a/docs/guides/infrastructure/openai-workload-identity.md +++ b/docs/guides/infrastructure/openai-workload-identity.md @@ -6,43 +6,65 @@ short-lived token (minutes — it never outlives the GitHub token it came from, fullsend renews it for as long as the run lasts, and the agent sandbox never sees any of it — it only ever holds a placeholder that the OpenShell gateway swaps for the real token on the way out. -You set this up once per OpenAI organization and once per repository. It takes one visit to the -OpenAI console and three repository variables. No key is created, downloaded or rotated. +Setting it up is one visit to the OpenAI console — yours, or your IT administrator's — and one +`fullsend github setup` run per repository. No key is created, downloaded or rotated. > **GitHub Actions only.** The exchange needs the job's OIDC endpoint. For GitLab CI and for runs > on your own machine, use an API key in the runner environment — see [Run it locally](#run-it-locally). -## Before you start - -- [ ] You can manage **Workload Identity Providers** in your OpenAI organization - (Organization Settings → Security). This is an organization-level permission; being an owner - of a project is not enough. -- [ ] You have an OpenAI **project** for the runs, ideally a dedicated one with a budget alert. -- [ ] The repository is enrolled per-repo on a fullsend release that includes this feature - (the release notes for #6689 name it). - -## 1. Add the identity provider (once per organization) - -In the OpenAI console go to **Organization Settings → Security → Workload Identity Provider** and -add a provider: - -| Field | Enter | -|---|---| -| OIDC issuer URL | `https://token.actions.githubusercontent.com` | -| Audience | `fullsend://` (or any string you choose — you will use it verbatim below) | -| Use uploaded JWKS for token verification | **Off** | - -Copy the **identity provider ID** — you need it in step 4. - -The audience is not issued by anyone: whatever you type here is what your runs will request, -character for character. Pick it once and reuse it for every repository. - -## 2. See what your repository actually claims (once) - -OpenAI decides whether to trust a run by comparing claims in the GitHub token with the mapping you -write in step 3. A wrong audience and a wrong claim fail the same way, so look at the real values -before writing the mapping. Add this temporary workflow to the repository, run it, and copy the -printed claims: +## What you end up with + +Three identifiers, which you hand to fullsend in [step 4](#4-tell-fullsend-the-three-identifiers). +They are not secrets: on their own they grant nothing. + +| Identifier | What it is | Where it comes from | +|---|---|---| +| **Audience** | The string your runs ask GitHub to put in the token's `aud` claim. It must equal what the identity provider was created with, character for character. | Whoever created the provider — you (route A) or your IT admin (route B). It is not issued by anyone and has no required format. | +| **Identity provider ID** | The OpenAI Workload Identity Provider for GitHub Actions in your organization. | The provider's page in the OpenAI console (route A), or your IT admin (route B). | +| **Service account ID** | The service account the mapping grants to your repository, in the project the runs are billed to. | The mapping (route A), or your IT admin (route B). | + +You also need an OpenAI **project** to bill the runs to (ideally a dedicated one with a budget +alert), and the repository must be enrolled per-repo on a fullsend release that includes this +feature (one that includes fullsend PR #6695; check the release notes). + +## Which route are you on? + +Workload Identity Providers and their service-account mappings are an **organization-level** +security setting in OpenAI (Organization Settings → Security). Who can edit them decides your route: + +- **Route A — you can manage providers in your OpenAI organization.** Being an owner of a project + is not enough; you need the organization-level permission. You create (or reuse) the provider and + add one mapping per repository yourself. Do [step 1](#1-see-what-your-repository-actually-claims-both-routes), + then [A2](#a2-add-or-reuse-the-identity-provider-route-a) and [A3](#a3-map-the-repository-to-a-service-account-route-a). +- **Route B — the provider is managed centrally.** This is the common shape in a company: an IT + administrator owns Organization Settings and the GitHub Actions provider; you own (or request) a + project. A service account is a non-human API principal inside a project; the administrator can + create it while adding the mapping, so all you need to know is the project's name or ID + (Organization → Projects). You send one request per repository and receive the three + identifiers back. Do [step 1](#1-see-what-your-repository-actually-claims-both-routes), then + [B2](#b2-send-the-request-route-b) and [B3](#b3-record-what-you-get-back-route-b). + +Either way, steps 4 and onwards are the same, and +[`fullsend inference openai`](../../cli/inference.md#inference-openai) does the paperwork on both: +`request` computes the provider and mapping values from the repository name (route A: what to type +into the console; route B: the ticket to send), and `import` records the identifiers you end up with. + +> **The rule that holds in both routes: trust is per repository.** A mapping asserts +> `repository == /` (and `ref == refs/heads/main`) for each company-owned +> repository you enrol — one mapping per repository (a mapping has no list form: its assertions are +> exact values, all of which must match). Never a pattern over the organization +> (`repository_owner`, a name prefix, a derived attribute): a GitHub organization often contains +> repositories the company does not own, and a pattern would let every one of them obtain your +> token. Adding a repository later means adding its assertion; that is the gate, by design. + +## 1. See what your repository actually claims (both routes) + +OpenAI decides whether to trust a run by comparing claims in the GitHub token with the mapping's +assertions. A wrong audience and a wrong claim fail the same way, so look at the real values before +writing the mapping (route A) or before asking for it (route B). Add this temporary workflow to +the repository, run it, and copy the printed claims. Put the provider's audience in `AUD` if you +already know it; if you do not yet, any string works for this check — `repository` and `ref` are +what you are after. ```yaml name: openai-wif-check @@ -55,7 +77,7 @@ jobs: runs-on: ubuntu-latest steps: - env: - AUD: fullsend:// + AUD: run: | TOK=$(curl -sSf -H "Authorization: bearer $ACTIONS_ID_TOKEN_REQUEST_TOKEN" \ "${ACTIONS_ID_TOKEN_REQUEST_URL}&audience=$(jq -rn --arg a "$AUD" '$a|@uri')" | jq -r .value) @@ -70,7 +92,7 @@ You will see something like: ```json { "iss": "https://token.actions.githubusercontent.com", - "aud": "fullsend://", + "aud": "", "repository": "/", "ref": "refs/heads/main", "workflow_ref": "//.github/workflows/openai-wif-check.yml@refs/heads/main", @@ -81,42 +103,47 @@ You will see something like: For the real agent runs, `repository` and `ref` are the same as above; `workflow_ref` names whichever of the fullsend workflows installed in your repository started the run (`code.yml`, `triage.yml`, `review.yml`, `fix.yml`, `prioritize.yml`, `retro.yml` or `dispatch.yml` under `.github/workflows/`), -and `job_workflow_ref` the pinned fullsend reusable workflow it calls. Delete the check workflow when -you are done. +and `job_workflow_ref` the pinned fullsend reusable workflow it calls — which is why a mapping asserts +`repository` and `ref`, not `workflow_ref`. Delete the check workflow when you are done. + +## A2. Add or reuse the identity provider (route A) + +If your organization already has a provider for GitHub Actions, reuse it: open it, note its +**audience** and copy its **identity provider ID**, and go to A3. Otherwise, in **Organization +Settings → Security → Workload Identity Provider**, add one: + +| Field | Enter | +|---|---| +| OIDC issuer URL | `https://token.actions.githubusercontent.com` | +| Audience | Any string you choose, for example `fullsend://` — your runs will request it verbatim | +| Use uploaded JWKS for token verification | **Off** | -## 3. Map the repository to a service account (once per repository) +Copy the **identity provider ID**. One provider serves every repository; OpenAI allows 50 providers +per organization and 50 mappings per provider. -> **Shortcut:** `fullsend inference openai request ` generates the -> complete request document (JSON or Markdown) with every value computed from the -> repository name — use it to produce the ticket for your administrator instead of -> filling in the tables below by hand. See -> [`fullsend inference openai`](../../cli/inference.md#inference-openai) for details. +## A3. Map the repository to a service account (route A) -Open the provider you created and add a **service account mapping**: +> **Shortcut.** `fullsend inference openai request / --project ` prints the +> exact values for the fields below (one block per repository), so you copy rather than retype them. + +Open the provider and add a **service account mapping**: | Field | Enter | |---|---| -| Claim assertions | `iss` = `https://token.actions.githubusercontent.com` · `aud` = your audience · `repository` = `/` · `ref` = `refs/heads/main` | +| Claim assertions | `iss` = `https://token.actions.githubusercontent.com` · `aud` = the provider's audience · `repository` = `/` · `ref` = `refs/heads/main` | | Project | the project the runs should be billed to | | Service account | create a new one, for example `fullsend--ci` | | Permissions | `api.model.request` (fullsend also accepts `api.model.read`; anything broader is refused at run time) | -Every assertion must match what step 2 printed character for character; a one-letter difference -fails every exchange. Do not assert `workflow_ref`: fullsend runs agents from seven workflow files -(see step 2), so a single `workflow_ref` value would exclude all but one of them. If you want that -narrowing anyway, create one mapping per workflow file. - -Copy the **service account ID** — you need it in step 4. +Every assertion must match what step 1 printed character for character; a one-letter difference +fails every exchange. Copy the **service account ID**. Two things not to do: - **Do not create an API key** for the service account. The mapping is the credential; a key would put a long-lived secret back into the picture. -- **Do not assert `repository_owner` instead of `repository`.** That would trust every repository - in your GitHub organization, including ones anybody with repository-create rights can add. - -OpenAI allows 50 mappings per provider and 50 providers per organization, and mappings are created -in the console only. Treat enrolment as a deliberate step per repository. +- **Do not assert `repository_owner`, a prefix or any pattern instead of `repository`** — see the + per-repository rule above. **Which runs this mapping trusts.** `ref` = `refs/heads/main` covers every fullsend agent workflow as installed on `main`, and GitHub mints the token for the repository the job runs in — so the @@ -126,24 +153,90 @@ dispatch authorization (only events from people with write access trigger agents If you want the mapping itself to exclude some triggers, add a claim the untrusted runs cannot carry — for example a GitHub `environment` that the agent jobs reference and that protects itself with required reviewers — and assert `environment == ""` here. A `workflow_dispatch` of a -fullsend workflow on a feature branch will not match `refs/heads/main`; that is expected. In the -org-wide installation mode the runs happen in the organization's central `.fullsend` repository, -so assert that repository instead. +fullsend workflow on a feature branch will not match `refs/heads/main`; that is expected. + +## B2. Send the request (route B) + +You cannot see the provider, so ask the administrator who owns it. Generate the request instead of +writing it — every value follows from the repository name: + +```bash +fullsend inference openai request / \ + --project "" \ + --audience "" \ + --format md --out openai-wif-request.md +``` + +Send one request per repository (or one listing several — the administrator still creates one +mapping per repository). `--format json` emits the same content as a machine-readable document, which +an administrator can fill in and hand back for [B3](#b3-record-what-you-get-back-route-b). What the +generated request says, and what to write by hand if you would rather not run the command: + +```text +Subject: OpenAI Workload Identity mapping for GitHub repository / + +Please add a service-account mapping on the organization's GitHub Actions identity provider +(issuer https://token.actions.githubusercontent.com) with these claim assertions, exactly: + + iss = https://token.actions.githubusercontent.com + aud = + repository = / + ref = refs/heads/main + +Target project: (the project these runs are billed to) +Service account: create a new one named fullsend--ci in that project + (or map the existing service account ) +Permissions on the mapping: api.model.request only — nothing broader, and please do not +create an API key for the service account; the mapping is the credential. + +Please do not add a workflow_ref or sub assertion: these runs start from seven different +workflow files in the repository, so a single value would exclude the others. + +One mapping per repository, please (a mapping matches exact values only) — not a wildcard +or a pattern over the organization, since not every repository in it is ours. + +Please send back: the identity provider ID, the provider's audience string, and the +service account ID. +``` + +If the administrator's standard mapping grants more than model access, ask for it to be narrowed: +fullsend refuses a token whose permissions exceed `api.model.request`/`api.model.read`, and only +warns when the mapping does not narrow at all. If you want the mapping to exclude some triggers +(a GitHub `environment` with required reviewers, for example), the reasoning is under +[A3](#a3-map-the-repository-to-a-service-account-route-a) — ask for the extra assertion in the +same request. + +## B3. Record what you get back (route B) + +The reply gives you the three identifiers from [What you end up with](#what-you-end-up-with). +Record them with +[`fullsend inference openai import`](../../cli/inference.md#inference-openai-import) — it takes the +filled-in reply JSON (`fullsend inference openai import reply.json`) or the three values as flags, +and writes them the same way [step 4](#4-tell-fullsend-the-three-identifiers) does. + +To check the enrolment before a real agent run, `fullsend inference openai status /` +prints the resolved identifiers and where they came from; inside a GitHub Actions job with +`id-token: write` it performs one exchange and reports the granted scope and expiry. Otherwise, +re-run the step-1 workflow with the real audience in `AUD` and confirm `aud` and `repository` print +as expected — that is the whole verification you can do from your side before the first run. If the +first run's exchange still returns 4xx, the assertions and the claims differ somewhere (compare them +character for character with the administrator); a `repository` that is not in any mapping is the +usual cause when a second repository is enrolled. ## 4. Tell fullsend the three identifiers -> **Shortcut:** `fullsend inference openai import` writes the three identifiers -> into `.fullsend/config.yaml` (or sets repository variables with `--variables`). -> It accepts a JSON reply file or CLI flags. See +> **Shortcut.** `fullsend inference openai import` writes the same block from a reply JSON file or +> from `--audience`/`--identity-provider-id`/`--service-account-id` flags, and `--variables` sets the +> repository variables instead. See > [`fullsend inference openai import`](../../cli/inference.md#inference-openai-import). Re-run the setup command you enrolled the repository with, adding the three values: ```bash fullsend github setup / \ - --openai-audience "fullsend://" \ - --openai-identity-provider-id "" \ - --openai-service-account-id "" + --openai-audience "" \ + --openai-identity-provider-id "" \ + --openai-service-account-id "" ``` It writes them into the repository's `.fullsend/config.yaml`, the same way it records the Vertex @@ -152,18 +245,19 @@ project and provider: ```yaml inference: openai: - audience: fullsend:// + audience: identity_provider_id: service_account_id: ``` Commit that change (setup opens a pull request for it unless you pass `--direct`). A base configuration (`config.base.yaml`, or a vendor preset) can carry the block for many repositories, -and a repository can restate any one of the three. +and a repository can restate any one of the three — with a centrally managed provider, the audience +and the provider ID are typically the same for every repository and only the service account differs. **Is it safe to commit these?** Yes. They are identifiers, not secrets: on their own they grant nothing. OpenAI issues a token only to a caller presenting a GitHub OIDC token whose claims match -the mapping from step 3, and only your repository's `main` workflow can obtain one. fullsend reads +a mapping, and only your repository's `main` workflow can obtain one. fullsend reads `.fullsend/config.yaml` from the base branch for pull-request events, so a pull request cannot change them for the run that reviews it. The worst thing someone with write access could do is point the block at their own OpenAI organization — and pay for your runs. fullsend prints the three @@ -267,9 +361,9 @@ agent starts and names the rule. | `no OpenAI credential: set FULLSEND_OPENAI_AUDIENCE, …` | Run step 4 (or add the three variables), or bump the workflow pin to a release that includes this feature. | | `OpenAI WIF is partially configured: missing …` / `inference.openai in config.yaml is partially configured` | One value is empty in the place you chose (variables, or `config.yaml`). Fill it in — fullsend will not silently fall back to an API key, and it does not mix the two sources. | | `… the job has no GitHub OIDC endpoint` | This is not a GitHub Actions job, or `permissions: id-token: write` is missing from the workflow. On GitLab CI or locally, use an API key. | -| `OpenAI WIF exchange failed: … token endpoint returned 4xx` | The mapping does not match this run. Check the audience first (one character off is enough), then compare the claims from step 2 with the mapping's assertions. Works on `main` but not on a pull request → the `ref`/`workflow_ref` differ. Works in one repository but not another → that repository has no mapping yet. | +| `OpenAI WIF exchange failed: … token endpoint returned 4xx` | The mapping does not match this run. Check the audience first (one character off is enough), then compare the claims from step 1 with the mapping's assertions (route B: with the administrator). Works in one repository but not another → that repository has no mapping yet. | | Exchange succeeded, the model call was refused | The mapping's permissions do not cover the call, or the project cannot use that model. The `scope` in the exchange response shows what was granted. | -| `OpenAI WIF token refused: the service-account mapping grants …` | The mapping grants more than model access. Narrow its permissions to `api.model.request` (step 3); fullsend will not run an agent with a broader token. | +| `OpenAI WIF token refused: the service-account mapping grants …` | The mapping grants more than model access. Narrow its permissions to `api.model.request` (route A: edit the mapping in A3; route B: ask your administrator); fullsend will not run an agent with a broader token. | | `the service-account mapping does not narrow permissions` (warning) | The mapping has no permission restriction, so the token holds whatever the service account holds. Add `api.model.request` on the mapping. | | `OPENAI_API_KEY in the sandbox is not a gateway placeholder` | A real key reached the sandbox environment by some other route (an env file copied into the sandbox, for example). Remove it; the provider is the only supported way in. | | `pi config dir has models.json or an auth.json that is not the runner-seeded openai placeholder` | Something wrote into pi's config directory between iterations. Re-run with `--keep-sandbox` and inspect it. | diff --git a/docs/runtimes/pi.md b/docs/runtimes/pi.md index 3108bcd865..34de380462 100644 --- a/docs/runtimes/pi.md +++ b/docs/runtimes/pi.md @@ -32,7 +32,8 @@ through fullsend's table, and a bare id gets the provider from `FULLSEND_PI_PROV > canonical spec. > **GPT via OpenAI** needs no API key in CI: the runner exchanges the job's GitHub identity for a -> short-lived OpenAI token (set three repository variables — see [OpenAI Workload +> short-lived OpenAI token (give fullsend three identifiers with `fullsend github setup --openai-*` +> or repository variables — see [OpenAI Workload > Identity](../guides/infrastructure/openai-workload-identity.md); GitHub Actions only) and keeps it > in a provider that belongs to this run, refreshed before it expires and removed when the run > ends. Locally, put `OPENAI_API_KEY` in an env file for the runner ([Running agents From 14c970ba6ee3d1101b62c08c7c9e6dac88e829d3 Mon Sep 17 00:00:00 2001 From: Wayne Sun Date: Sun, 30 Aug 2026 12:32:51 -0400 Subject: [PATCH 3/5] fix(#6778): round-trippable reply, and status that matches the run path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review findings from Codex (gpt-5.6-sol) and Grok 4.6, which agreed on the two blocking ones. import could not read the document request generates. The request JSON nests the fields an administrator fills under "reply", while import unmarshalled them from the root, so returning the very file we sent imported nothing. It now accepts both shapes, and picks the service account for --repo out of a multi-repository reply instead of silently importing none — an ambiguous reply says which repositories it names. status resolved each identifier independently, env or config per field, while a run takes the trio from one source: any FULLSEND_OPENAI_* variable means all three come from variables. status could therefore report a healthy trio, and exchange on it, that a run would refuse. It now applies the same rule, names the source, and explains the gap. Also in status: a malformed config is an error rather than "nothing configured yet"; an exchange is refused when the job's own repository is not the one being checked, since it could only ever prove that job's identity; and a token broader than model access fails the way the run path fails instead of printing "Exchange succeeded". In request: --ref for repositories whose default branch is not main (the assertion is exact, so a wrong ref fails every exchange), a guard against deriving one owner's default audience for another owner's mapping, duplicate repositories collapsed, an existing --service-account no longer described as "create inline", the permission list rendered rather than indexed, and the generated rules now name `sub` too. import validates the config before writing it, as `github setup` does, and a partial --variables write says which variables were set. Docs: the inference command group is no longer described as GCP-only, the "these commands do not call the OpenAI API" claim is qualified for status, and the command tree and key-files table in cli-internals cover the new subcommands. Assisted-by: Claude (fix), Codex (review), Grok (review) Signed-off-by: Wayne Sun --- docs/cli/inference.md | 6 +- docs/guides/dev/cli-internals.md | 20 +- .../openai-workload-identity.md | 6 +- internal/cli/inference.go | 15 +- internal/cli/inference_openai.go | 303 +++++++++++++----- internal/cli/inference_openai_test.go | 285 ++++++++++++++-- 6 files changed, 527 insertions(+), 108 deletions(-) diff --git a/docs/cli/inference.md b/docs/cli/inference.md index 707715d4db..ddb214c1a4 100644 --- a/docs/cli/inference.md +++ b/docs/cli/inference.md @@ -4,7 +4,7 @@ sidebar_label: fullsend inference # fullsend inference -Manage GCP Workload Identity Federation (WIF) infrastructure for Agent Platform access. These commands create, inspect, and remove the WIF pool, OIDC provider, and IAM bindings that allow GitHub Actions workflows to authenticate with GCP. +Manage the inference credentials agent runs use. `provision`, `deprovision` and `status` create, inspect, and remove the GCP Workload Identity Federation (WIF) pool, OIDC provider, and IAM bindings that let GitHub Actions workflows authenticate with GCP for Agent Platform (Vertex) access. [`openai`](#inference-openai) enrols repositories with OpenAI WIF for GPT models on the pi runtime. ## Commands @@ -85,7 +85,9 @@ Read-only — makes no changes. ## `inference openai` -Commands for enrolling repositories with OpenAI Workload Identity Federation. These commands do not call the OpenAI API — they produce documents and update local configuration. +Commands for enrolling repositories with OpenAI Workload Identity Federation (see the [operator guide](../guides/infrastructure/openai-workload-identity.md)). They need neither GCP access nor an OpenAI key. + +`request` and `import` never reach the network: they produce a document and update local configuration. `status` reads configuration too, and — only when run inside a GitHub Actions job with `id-token: write` — performs one token exchange with OpenAI to prove the mapping accepts that repository, reporting the granted scope and expiry without ever printing the token. ### `inference openai request` diff --git a/docs/guides/dev/cli-internals.md b/docs/guides/dev/cli-internals.md index 13825e4879..ae5581ef20 100644 --- a/docs/guides/dev/cli-internals.md +++ b/docs/guides/dev/cli-internals.md @@ -27,10 +27,23 @@ fullsend │ ├── --repos # Comma-separated repo names │ ├── --mint-url # Mint service URL ($FULLSEND_MINT_URL) │ └── --audience # OIDC audience (default: fullsend-mint) -├── inference # GCP: inference WIF management +├── inference # Inference credentials (GCP Vertex, OpenAI) │ ├── provision # Create WIF pool/provider for Agent Platform │ ├── deprovision # Remove WIF access for org or repo -│ └── status # Check WIF health, print config +│ ├── status # Check WIF health, print config +│ └── openai # OpenAI WIF enrolment (GPT on pi) +│ ├── request [,...] # Generate the provider/mapping request for an admin +│ │ ├── --audience # Provider audience (default: fullsend://) +│ │ ├── --project # OpenAI project to bill the runs to +│ │ ├── --service-account # Map an existing service account +│ │ ├── --ref # Ref assertion (default: refs/heads/main) +│ │ ├── --format # Document format +│ │ └── --out # Write to a file instead of stdout +│ ├── import [reply.json] # Record the admin's reply in config.yaml +│ │ ├── --audience/--identity-provider-id/--service-account-id +│ │ ├── --variables # Set FULLSEND_OPENAI_* repo variables instead +│ │ └── --repo # Select from a multi-repo reply; target for --variables +│ └── status # Resolved identifiers, and the exchange inside Actions ├── github # GitHub-only configuration │ ├── setup # Configure fullsend (no GCP needed) │ ├── enroll [repo...] # Enable repos for agent workflows @@ -682,7 +695,8 @@ var executableFiles = map[string]struct{}{ | `internal/cli/root.go` | ~34 | CLI entry point, command registration | | `internal/cli/admin.go` | ~2415 | Install/uninstall/analyze/enable/disable | | `internal/cli/mint.go` | ~1022 | Mint deploy/enroll/unenroll/status | -| `internal/cli/inference.go` | ~408 | Inference WIF provision/status | +| `internal/cli/inference.go` | ~408 | Inference WIF provision/status (GCP) | +| `internal/cli/inference_openai.go` | ~760 | OpenAI WIF enrolment: request document, reply import, status/exchange | | `internal/cli/github.go` | ~966 | GitHub setup/set/status/uninstall/sync-scaffold/enroll/unenroll | | `internal/cli/issues.go` | ~430 | Issue read/write commands (`fullsend issues get`, `post-comment`) | | `internal/cli/tracker_client.go` | ~122 | Tracker client factory (GitHub/GitLab/Jira) | diff --git a/docs/guides/infrastructure/openai-workload-identity.md b/docs/guides/infrastructure/openai-workload-identity.md index 72e910f927..c94983932d 100644 --- a/docs/guides/infrastructure/openai-workload-identity.md +++ b/docs/guides/infrastructure/openai-workload-identity.md @@ -7,7 +7,7 @@ fullsend renews it for as long as the run lasts, and the agent sandbox never see ever holds a placeholder that the OpenShell gateway swaps for the real token on the way out. Setting it up is one visit to the OpenAI console — yours, or your IT administrator's — and one -`fullsend github setup` run per repository. No key is created, downloaded or rotated. +command per repository. No key is created, downloaded or rotated. > **GitHub Actions only.** The exchange needs the job's OIDC endpoint. For GitLab CI and for runs > on your own machine, use an API key in the runner environment — see [Run it locally](#run-it-locally). @@ -30,7 +30,9 @@ feature (one that includes fullsend PR #6695; check the release notes). ## Which route are you on? Workload Identity Providers and their service-account mappings are an **organization-level** -security setting in OpenAI (Organization Settings → Security). Who can edit them decides your route: +security setting in OpenAI (Organization Settings → Security). Who can edit them decides your route — +open that page to find out: if you can see **Workload Identity Provider** and add one, you are on +route A; if the page or the setting is not there, you are on route B and someone else owns it. - **Route A — you can manage providers in your OpenAI organization.** Being an owner of a project is not enough; you need the organization-level permission. You create (or reuse) the provider and diff --git a/internal/cli/inference.go b/internal/cli/inference.go index dc9af45724..a35e6e1012 100644 --- a/internal/cli/inference.go +++ b/internal/cli/inference.go @@ -18,13 +18,16 @@ var gcpProjectPattern = regexp.MustCompile(`^[a-z][a-z0-9-]{4,28}[a-z0-9]$`) func newInferenceCmd() *cobra.Command { cmd := &cobra.Command{ Use: "inference", - Short: "Manage inference credentials (requires GCP access)", - Long: `Commands for provisioning and inspecting inference WIF infrastructure. + Short: "Manage inference credentials (GCP Vertex, OpenAI)", + Long: `Commands for provisioning and inspecting inference credentials. -These commands only require GCP project access — no GitHub token or -mint project is needed. Use them to set up Workload Identity Federation -for Agent Platform inference, then hand off the WIF provider resource name -to the GitHub admin who runs 'fullsend github setup'.`, +'provision', 'deprovision' and 'status' set up Workload Identity +Federation for Agent Platform (Vertex) inference and only require GCP +project access — no GitHub token or mint project is needed. Hand the WIF +provider resource name to the GitHub admin who runs 'fullsend github setup'. + +'openai' enrols repositories with OpenAI Workload Identity Federation for +GPT models on the pi runtime; it needs neither GCP nor an OpenAI key.`, } cmd.AddCommand(newInferenceProvisionCmd()) cmd.AddCommand(newInferenceStatusCmd()) diff --git a/internal/cli/inference_openai.go b/internal/cli/inference_openai.go index 813f808de7..5fd8b90002 100644 --- a/internal/cli/inference_openai.go +++ b/internal/cli/inference_openai.go @@ -1,11 +1,13 @@ package cli import ( + "context" "encoding/json" "fmt" "os" "os/exec" "path/filepath" + "sort" "strings" "text/template" "time" @@ -32,7 +34,10 @@ const ( // openAIDefaultPermission is the only permission an agent run needs. openAIDefaultPermission = "api.model.request" - // openAIDefaultRef is the ref assertion for the mapping. + // openAIDefaultRef is the ref assertion for the mapping: fullsend + // installs its agent workflows on the default branch and dispatches + // them there. A repository whose default branch is not main needs + // --ref, or every exchange fails on an assertion that cannot match. openAIDefaultRef = "refs/heads/main" ) @@ -66,9 +71,13 @@ type openAIRequestAssertions struct { } type openAIRequestTarget struct { - Project string `json:"project"` - ServiceAccount string `json:"service_account"` - Permissions []string `json:"permissions"` + Project string `json:"project"` + ServiceAccount string `json:"service_account"` + // CreateInline is false when the caller named an existing service + // account with --service-account, so the document does not tell the + // administrator to create one. + CreateInline bool `json:"create_inline"` + Permissions []string `json:"permissions"` } type openAIRequestReply struct { @@ -99,6 +108,7 @@ func newInferenceOpenAIRequestCmd() *cobra.Command { var audience string var project string var serviceAccount string + var ref string var format string var outFile string @@ -126,13 +136,19 @@ Output formats: return fmt.Errorf("at least one owner/repo is required") } - // Derive owner from first repo for default audience. - owner := strings.SplitN(repos[0], "/", 2)[0] + // The default audience is derived from the owner, so every + // repository in one request must share it — otherwise the + // second owner's mapping would silently carry the first + // owner's audience. if audience == "" { - audience = defaultOpenAIAudiencePrefix + owner + owners := repoOwners(repos) + if len(owners) > 1 { + return fmt.Errorf("repositories span more than one owner (%s): pass --audience with the provider's audience, since the default is derived from the owner", strings.Join(owners, ", ")) + } + audience = defaultOpenAIAudiencePrefix + owners[0] } - doc := buildRequestDoc(repos, audience, project, serviceAccount) + doc := buildRequestDoc(repos, audience, project, serviceAccount, ref) var output string switch format { @@ -168,6 +184,7 @@ Output formats: cmd.Flags().StringVar(&audience, "audience", "", "OpenAI Workload Identity audience (default: fullsend://)") cmd.Flags().StringVar(&project, "project", "", "OpenAI project name or ID for the service accounts") cmd.Flags().StringVar(&serviceAccount, "service-account", "", "existing service account ID to map (default: create fullsend--ci per repo)") + cmd.Flags().StringVar(&ref, "ref", openAIDefaultRef, "ref assertion for the mappings (the branch fullsend's agent workflows run from)") cmd.Flags().StringVar(&format, "format", "json", "output format: json, md") cmd.Flags().StringVar(&outFile, "out", "", "write output to a file instead of stdout") @@ -193,7 +210,32 @@ func parseRepoList(arg string) ([]string, error) { } repos = append(repos, p) } - return repos, nil + // A duplicate would render as two identical mappings in the request. + seen := make(map[string]bool, len(repos)) + deduped := repos[:0] + for _, r := range repos { + if seen[r] { + continue + } + seen[r] = true + deduped = append(deduped, r) + } + return deduped, nil +} + +// repoOwners returns the distinct owners of a repository list, in order. +func repoOwners(repos []string) []string { + var owners []string + seen := make(map[string]bool, len(repos)) + for _, r := range repos { + owner := strings.SplitN(r, "/", 2)[0] + if seen[owner] { + continue + } + seen[owner] = true + owners = append(owners, owner) + } + return owners } // defaultServiceAccountID derives the default service account name for @@ -204,7 +246,10 @@ func defaultServiceAccountID(repo string) string { return "fullsend-" + repoName + "-ci" } -func buildRequestDoc(repos []string, audience, project, serviceAccount string) openAIRequestDoc { +func buildRequestDoc(repos []string, audience, project, serviceAccount, ref string) openAIRequestDoc { + if strings.TrimSpace(ref) == "" { + ref = openAIDefaultRef + } doc := openAIRequestDoc{ Version: openAIRequestSchemaVersion, Provider: openAIRequestProvider{ @@ -221,7 +266,8 @@ func buildRequestDoc(repos []string, audience, project, serviceAccount string) o for _, repo := range repos { sa := serviceAccount - if sa == "" { + createInline := serviceAccount == "" + if createInline { sa = defaultServiceAccountID(repo) } @@ -231,11 +277,12 @@ func buildRequestDoc(repos []string, audience, project, serviceAccount string) o Iss: githubOIDCIssuer, Aud: audience, Repository: repo, - Ref: openAIDefaultRef, + Ref: ref, }, Target: openAIRequestTarget{ Project: project, ServiceAccount: sa, + CreateInline: createInline, Permissions: []string{openAIDefaultPermission}, }, } @@ -263,8 +310,9 @@ keep its audience and report it back (see Reply below). ## Service account mappings One mapping per repository. Every assertion must be exact — no wildcards, -no ` + "`" + `repository_owner` + "`" + `, no ` + "`" + `workflow_ref` + "`" + ` (fullsend starts agent runs -from seven workflow files). Do **not** create an API key for the service account. +no ` + "`" + `repository_owner` + "`" + `, no ` + "`" + `workflow_ref` + "`" + ` and no ` + "`" + `sub` + "`" + ` (fullsend starts agent +runs from several workflow files in the repository, so any single value would +exclude the others). Do **not** create an API key for the service account. {{ range .Mappings }} ### {{ .Repository }} @@ -272,8 +320,8 @@ from seven workflow files). Do **not** create an API key for the service account |---|---| | Claim assertions | ` + "`" + `iss` + "`" + ` = ` + "`" + `{{ .Assertions.Iss }}` + "`" + ` · ` + "`" + `aud` + "`" + ` = ` + "`" + `{{ .Assertions.Aud }}` + "`" + ` · ` + "`" + `repository` + "`" + ` = ` + "`" + `{{ .Assertions.Repository }}` + "`" + ` · ` + "`" + `ref` + "`" + ` = ` + "`" + `{{ .Assertions.Ref }}` + "`" + ` | | Project | {{ if .Target.Project }}` + "`" + `{{ .Target.Project }}` + "`" + `{{ else }}*(specify the project name or ID)*{{ end }} | -| Service account | {{ .Target.ServiceAccount }} (create inline in the mapping) | -| Permissions | ` + "`" + `{{ index .Target.Permissions 0 }}` + "`" + ` only | +| Service account | {{ .Target.ServiceAccount }}{{ if .Target.CreateInline }} (create inline in the mapping){{ else }} (existing — map it, do not create a new one){{ end }} | +| Permissions | {{ range $i, $p := .Target.Permissions }}{{ if $i }}, {{ end }}` + "`" + `{{ $p }}` + "`" + `{{ end }} only | {{ end }} ## Reply @@ -299,14 +347,86 @@ func renderRequestMarkdown(doc openAIRequestDoc) (string, error) { // --- import command --- -// openAIReplyDoc is the JSON shape accepted by the import command. It -// mirrors the request schema's reply section so an admin who fills the -// JSON in can be imported directly. +// openAIReplyDoc is the JSON shape accepted by the import command. Two +// documents are accepted, because both reach an administrator: the reply +// section on its own, and the whole request document from +// `inference openai request --format json` with its "reply" object filled +// in — which is what an administrator who edits the file we sent them +// hands back. type openAIReplyDoc struct { IdentityProviderID string `json:"identity_provider_id"` Audience string `json:"audience"` ServiceAccountID string `json:"service_account_id,omitempty"` ServiceAccountIDs map[string]string `json:"service_account_ids,omitempty"` + + // Reply carries the same fields when the file is a full request + // document; its values win, since that is the section the + // administrator was asked to fill in. + Reply *openAIReplyDoc `json:"reply,omitempty"` +} + +// resolved folds a full request document into the reply shape. +func (d openAIReplyDoc) resolved() openAIReplyDoc { + if d.Reply == nil { + return d + } + out := *d.Reply + out.Reply = nil + if out.Audience == "" { + out.Audience = d.Audience + } + if out.IdentityProviderID == "" { + out.IdentityProviderID = d.IdentityProviderID + } + if out.ServiceAccountID == "" { + out.ServiceAccountID = d.ServiceAccountID + } + if len(out.ServiceAccountIDs) == 0 { + out.ServiceAccountIDs = d.ServiceAccountIDs + } + return out +} + +// serviceAccountFor picks the service account for repo out of a reply. +// A reply for several repositories needs a selector, and says so rather +// than silently importing none. +func (d openAIReplyDoc) serviceAccountFor(repo string) (string, error) { + if d.ServiceAccountID != "" { + return d.ServiceAccountID, nil + } + filled := make(map[string]string, len(d.ServiceAccountIDs)) + for k, v := range d.ServiceAccountIDs { + if strings.TrimSpace(v) != "" { + filled[k] = v + } + } + if repo != "" { + if v, ok := filled[repo]; ok { + return v, nil + } + if len(filled) > 0 { + keys := make([]string, 0, len(filled)) + for k := range filled { + keys = append(keys, k) + } + sort.Strings(keys) + return "", fmt.Errorf("the reply has no service account for %s (it names %s)", repo, strings.Join(keys, ", ")) + } + } + switch len(filled) { + case 0: + return "", nil + case 1: + for _, v := range filled { + return v, nil + } + } + keys := make([]string, 0, len(filled)) + for k := range filled { + keys = append(keys, k) + } + sort.Strings(keys) + return "", fmt.Errorf("the reply names %d service accounts (%s): pass --repo to choose one, or --service-account-id", len(filled), strings.Join(keys, ", ")) } func newInferenceOpenAIImportCmd() *cobra.Command { @@ -337,7 +457,7 @@ token with variable-write permissions and --repo).`, RunE: func(cmd *cobra.Command, args []string) error { printer := ui.New(cmd.OutOrStdout()) - ids, err := resolveImportIDs(args, flagAudience, flagIdentityProviderID, flagServiceAccountID) + ids, err := resolveImportIDs(args, flagAudience, flagIdentityProviderID, flagServiceAccountID, repo) if err != nil { return err } @@ -347,7 +467,7 @@ token with variable-write permissions and --repo).`, } if variables { - return runImportVariables(printer, ids, repo) + return runImportVariables(cmd.Context(), printer, ids, repo) } return runImportConfig(printer, ids, fullsendDir) @@ -366,7 +486,7 @@ token with variable-write permissions and --repo).`, // resolveImportIDs takes the command arguments and flags and returns the // OpenAI WIF config. Flags take precedence over the JSON file. -func resolveImportIDs(args []string, flagAudience, flagIdentityProviderID, flagServiceAccountID string) (config.OpenAIWIFConfig, error) { +func resolveImportIDs(args []string, flagAudience, flagIdentityProviderID, flagServiceAccountID, repo string) (config.OpenAIWIFConfig, error) { var ids config.OpenAIWIFConfig // Load from JSON file if provided. @@ -375,21 +495,18 @@ func resolveImportIDs(args []string, flagAudience, flagIdentityProviderID, flagS if err != nil { return ids, fmt.Errorf("reading reply file: %w", err) } - var reply openAIReplyDoc - if err := json.Unmarshal(data, &reply); err != nil { + var doc openAIReplyDoc + if err := json.Unmarshal(data, &doc); err != nil { return ids, fmt.Errorf("parsing reply JSON: %w", err) } + reply := doc.resolved() ids.Audience = reply.Audience ids.IdentityProviderID = reply.IdentityProviderID - // Single service_account_id takes precedence; for multi-repo - // replies the caller must use --service-account-id to select. - if reply.ServiceAccountID != "" { - ids.ServiceAccountID = reply.ServiceAccountID - } else if len(reply.ServiceAccountIDs) == 1 { - for _, v := range reply.ServiceAccountIDs { - ids.ServiceAccountID = v - } + sa, err := reply.serviceAccountFor(repo) + if err != nil { + return ids, err } + ids.ServiceAccountID = sa } // Flags override file values. @@ -430,6 +547,12 @@ func runImportConfig(printer *ui.Printer, ids config.OpenAIWIFConfig, fullsendDi perRepo.SetInferenceOpenAI(ids) + // Fail closed the way `fullsend github setup` does, rather than + // writing a config the next run would reject. + if err := perRepo.Validate(); err != nil { + return fmt.Errorf("invalid config after import: %w", err) + } + data, err := perRepo.Marshal() if err != nil { return fmt.Errorf("marshaling config: %w", err) @@ -453,13 +576,13 @@ func runImportConfig(printer *ui.Printer, ids config.OpenAIWIFConfig, fullsendDi // ghRunner runs a gh CLI command and returns its combined output. Tests // can replace this variable with a stub. -var ghRunner = func(args ...string) (string, error) { - cmd := exec.Command("gh", args...) +var ghRunner = func(ctx context.Context, args ...string) (string, error) { + cmd := exec.CommandContext(ctx, "gh", args...) out, err := cmd.CombinedOutput() return string(out), err } -func runImportVariables(printer *ui.Printer, ids config.OpenAIWIFConfig, repo string) error { +func runImportVariables(ctx context.Context, printer *ui.Printer, ids config.OpenAIWIFConfig, repo string) error { if repo == "" { return fmt.Errorf("--repo is required when using --variables") } @@ -487,14 +610,22 @@ func runImportVariables(printer *ui.Printer, ids config.OpenAIWIFConfig, repo st {openAIServiceAccountIDEnv, ids.ServiceAccountID}, } + // The three are written one at a time, so a failure halfway leaves a + // partial trio on the repository — which a run refuses. Say exactly + // what was written so the operator can re-run or clear it. + var written []string for _, v := range vars { ghArgs := []string{"variable", "set", v.name, "--repo", repo, "--body", v.value} printer.StepStart(fmt.Sprintf("Setting %s on %s", v.name, repo)) - out, err := ghRunner(ghArgs...) + out, err := ghRunner(ctx, ghArgs...) if err != nil { printer.StepFail(fmt.Sprintf("Failed to set %s", v.name)) + if len(written) > 0 { + printer.StepWarn(fmt.Sprintf("%s already set on %s: the repository now holds a partial trio, which a run refuses — re-run the same command to finish, or remove them", strings.Join(written, ", "), repo)) + } return fmt.Errorf("setting variable %s on %s: %w\n%s", v.name, repo, err, out) } + written = append(written, v.name) printer.StepDone(fmt.Sprintf("Set %s on %s", v.name, repo)) } @@ -539,6 +670,9 @@ at the config checks.`, // openAIStatusSource describes where an identifier was resolved from. type openAIStatusSource struct { + // Source names where the trio came from as a whole ("variables" or + // "config.yaml"), mirroring the run path's all-or-nothing rule. + Source string Audience string AudienceSource string IdentityProviderID string @@ -547,49 +681,44 @@ type openAIStatusSource struct { SASource string } -func resolveOpenAIStatusSources(fullsendDir string) openAIStatusSource { +// resolveOpenAIStatusSources resolves the identifiers exactly as +// resolveOpenAICredential does for a real run: the FULLSEND_OPENAI_* +// variables win as a set — if any one of them is set, that is the source, +// and a gap in it is a gap — and the committed inference.openai block is +// used only when none of them is set. Reporting a mixed trio here would +// tell the operator a run will work when the run would refuse it. +func resolveOpenAIStatusSources(fullsendDir string) (openAIStatusSource, error) { var s openAIStatusSource - // Check environment variables first (they take precedence). envAud := strings.TrimSpace(os.Getenv(openAIAudienceEnv)) envIDP := strings.TrimSpace(os.Getenv(openAIIdentityProviderIDEnv)) envSA := strings.TrimSpace(os.Getenv(openAIServiceAccountIDEnv)) - // Load config as fallback. - var cfgIDs config.OpenAIWIFConfig - writer, err := config.LoadConfigWriter(fullsendDir, config.LoadOpts{MissingOK: true}) - if err == nil { - if perRepo, ok := writer.(config.PerRepoConfigReader); ok { - cfgIDs = perRepo.ConfigInferenceOpenAI().Trimmed() - } - } - - // Resolve each field: env var wins, then config. - if envAud != "" { - s.Audience = envAud - s.AudienceSource = "variable " + openAIAudienceEnv - } else if cfgIDs.Audience != "" { - s.Audience = cfgIDs.Audience - s.AudienceSource = "config.yaml" + if envAud != "" || envIDP != "" || envSA != "" { + s.Source = "variables" + s.Audience, s.AudienceSource = envAud, "variable "+openAIAudienceEnv + s.IdentityProviderID, s.IDPSource = envIDP, "variable "+openAIIdentityProviderIDEnv + s.ServiceAccountID, s.SASource = envSA, "variable "+openAIServiceAccountIDEnv + return s, nil } - if envIDP != "" { - s.IdentityProviderID = envIDP - s.IDPSource = "variable " + openAIIdentityProviderIDEnv - } else if cfgIDs.IdentityProviderID != "" { - s.IdentityProviderID = cfgIDs.IdentityProviderID - s.IDPSource = "config.yaml" - } - - if envSA != "" { - s.ServiceAccountID = envSA - s.SASource = "variable " + openAIServiceAccountIDEnv - } else if cfgIDs.ServiceAccountID != "" { - s.ServiceAccountID = cfgIDs.ServiceAccountID - s.SASource = "config.yaml" + writer, err := config.LoadConfigWriter(fullsendDir, config.LoadOpts{MissingOK: true}) + if err != nil { + // A malformed or unreadable config must not read as "nothing + // configured yet" — that is the one state an operator would + // misdiagnose. + return s, fmt.Errorf("loading config from %s: %w", fullsendDir, err) } - - return s + perRepo, ok := writer.(config.PerRepoConfigReader) + if !ok { + return s, nil + } + cfgIDs := perRepo.ConfigInferenceOpenAI().Trimmed() + s.Source = "config.yaml" + s.Audience, s.AudienceSource = cfgIDs.Audience, "config.yaml" + s.IdentityProviderID, s.IDPSource = cfgIDs.IdentityProviderID, "config.yaml" + s.ServiceAccountID, s.SASource = cfgIDs.ServiceAccountID, "config.yaml" + return s, nil } func runInferenceOpenAIStatus(cmd *cobra.Command, printer *ui.Printer, repo, fullsendDir string) error { @@ -598,7 +727,11 @@ func runInferenceOpenAIStatus(cmd *cobra.Command, printer *ui.Printer, repo, ful printer.Header("OpenAI WIF Status: " + repo) printer.Blank() - sources := resolveOpenAIStatusSources(fullsendDir) + sources, err := resolveOpenAIStatusSources(fullsendDir) + if err != nil { + printer.StepFail("Could not read the configuration") + return err + } // Print resolved identifiers. printOpenAIStatusField(printer, "audience", sources.Audience, sources.AudienceSource) @@ -620,7 +753,10 @@ func runInferenceOpenAIStatus(cmd *cobra.Command, printer *ui.Printer, repo, ful } if missing := ids.Missing(); len(missing) > 0 { - printer.StepWarn(fmt.Sprintf("Partial trio: missing %s", strings.Join(missing, ", "))) + printer.StepWarn(fmt.Sprintf("Partial trio in %s: missing %s", sources.Source, strings.Join(missing, ", "))) + if sources.Source == "variables" { + printer.StepInfo("A run takes the three identifiers from one source: with any FULLSEND_OPENAI_* variable set, all three must come from variables — the config.yaml block is not consulted") + } printer.StepInfo("All three identifiers must be set for the exchange to work") return nil } @@ -638,6 +774,16 @@ func runInferenceOpenAIStatus(cmd *cobra.Command, printer *ui.Printer, repo, ful return nil } + // Only now does the repository argument matter: an exchange proves + // the identity of the job it runs in, so checking another + // repository's name here would report a mapping as healthy on the + // strength of this repository's token. + if current := strings.TrimSpace(os.Getenv("GITHUB_REPOSITORY")); current != "" && !strings.EqualFold(current, repo) { + printer.StepWarn(fmt.Sprintf("This job runs in %s, so it cannot test %s: an exchange here would prove %s's mapping only", current, repo, current)) + printer.StepInfo("Run the command from a job in " + repo + " to test its mapping") + return nil + } + // Perform the exchange. printer.StepStart("Performing WIF exchange") tok, err := openAIExchange(cmd.Context(), openaiwif.Config{ @@ -654,7 +800,7 @@ func runInferenceOpenAIStatus(cmd *cobra.Command, printer *ui.Printer, repo, ful return fmt.Errorf("OpenAI WIF exchange: %w", err) } - printer.StepDone("Exchange succeeded") + printer.StepDone("Exchange succeeded for " + repo) printer.Blank() scope := tok.Scope @@ -664,9 +810,14 @@ func runInferenceOpenAIStatus(cmd *cobra.Command, printer *ui.Printer, repo, ful printer.KeyValue("scope", scope) printer.KeyValue("expires_in", time.Until(tok.ExpiresAt).Round(time.Second).String()) - if warning, err := checkOpenAIScope(tok.Scope); err != nil { - printer.StepWarn(err.Error()) - } else if warning != "" { + // A token broader than model access is refused by `fullsend run`, so + // it must not read as a healthy status here either. + warning, scopeErr := checkOpenAIScope(tok.Scope) + if scopeErr != nil { + printer.StepFail("The mapping grants more than model access") + return fmt.Errorf("OpenAI WIF token refused: %w", scopeErr) + } + if warning != "" { printer.StepWarn(warning) } diff --git a/internal/cli/inference_openai_test.go b/internal/cli/inference_openai_test.go index 6978e6a434..3e41e8ea81 100644 --- a/internal/cli/inference_openai_test.go +++ b/internal/cli/inference_openai_test.go @@ -2,16 +2,19 @@ package cli import ( "bytes" + "context" "encoding/json" "os" "path/filepath" "strings" "testing" + "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/fullsend-ai/fullsend/internal/config" + "github.com/fullsend-ai/fullsend/internal/inference/openaiwif" "github.com/fullsend-ai/fullsend/internal/ui" ) @@ -251,7 +254,7 @@ func TestInferenceOpenAIImportCmd_Flags(t *testing.T) { } func TestResolveImportIDs_FromFlags(t *testing.T) { - ids, err := resolveImportIDs(nil, "aud", "idp", "sa") + ids, err := resolveImportIDs(nil, "aud", "idp", "sa", "") require.NoError(t, err) assert.Equal(t, config.OpenAIWIFConfig{ Audience: "aud", @@ -272,7 +275,7 @@ func TestResolveImportIDs_FromFile(t *testing.T) { require.NoError(t, err) require.NoError(t, os.WriteFile(replyPath, data, 0o644)) - ids, err := resolveImportIDs([]string{replyPath}, "", "", "") + ids, err := resolveImportIDs([]string{replyPath}, "", "", "", "") require.NoError(t, err) assert.Equal(t, "fullsend://acme", ids.Audience) assert.Equal(t, "idp_123", ids.IdentityProviderID) @@ -291,7 +294,7 @@ func TestResolveImportIDs_FromFileWithSingleServiceAccountIDs(t *testing.T) { require.NoError(t, err) require.NoError(t, os.WriteFile(replyPath, data, 0o644)) - ids, err := resolveImportIDs([]string{replyPath}, "", "", "") + ids, err := resolveImportIDs([]string{replyPath}, "", "", "", "") require.NoError(t, err) assert.Equal(t, "sa_widget", ids.ServiceAccountID) } @@ -308,7 +311,7 @@ func TestResolveImportIDs_FlagsOverrideFile(t *testing.T) { require.NoError(t, err) require.NoError(t, os.WriteFile(replyPath, data, 0o644)) - ids, err := resolveImportIDs([]string{replyPath}, "from-flag", "", "") + ids, err := resolveImportIDs([]string{replyPath}, "from-flag", "", "", "") require.NoError(t, err) assert.Equal(t, "from-flag", ids.Audience) assert.Equal(t, "idp-file", ids.IdentityProviderID) @@ -411,7 +414,7 @@ func TestRunImportVariables_RequiresRepo(t *testing.T) { var buf bytes.Buffer printer := newTestPrinter(&buf) - err := runImportVariables(printer, ids, "") + err := runImportVariables(context.Background(), printer, ids, "") require.Error(t, err) assert.Contains(t, err.Error(), "--repo is required") } @@ -425,7 +428,7 @@ func TestRunImportVariables_RequiresOwnerSlashRepo(t *testing.T) { var buf bytes.Buffer printer := newTestPrinter(&buf) - err := runImportVariables(printer, ids, "acme") + err := runImportVariables(context.Background(), printer, ids, "acme") require.Error(t, err) assert.Contains(t, err.Error(), "owner/repo") } @@ -434,7 +437,7 @@ func TestRunImportVariables_CallsGH(t *testing.T) { // Replace ghRunner with a stub. var calls [][]string origGH := ghRunner - ghRunner = func(args ...string) (string, error) { + ghRunner = func(_ context.Context, args ...string) (string, error) { calls = append(calls, args) return "", nil } @@ -448,7 +451,7 @@ func TestRunImportVariables_CallsGH(t *testing.T) { var buf bytes.Buffer printer := newTestPrinter(&buf) - err := runImportVariables(printer, ids, "acme/widget") + err := runImportVariables(context.Background(), printer, ids, "acme/widget") require.NoError(t, err) require.Len(t, calls, 3) @@ -495,7 +498,8 @@ func TestResolveOpenAIStatusSources_FromConfig(t *testing.T) { t.Setenv(openAIIdentityProviderIDEnv, "") t.Setenv(openAIServiceAccountIDEnv, "") - s := resolveOpenAIStatusSources(fullsendDir) + s, err := resolveOpenAIStatusSources(fullsendDir) + require.NoError(t, err) assert.Equal(t, "fullsend://acme", s.Audience) assert.Equal(t, "config.yaml", s.AudienceSource) assert.Equal(t, "idp_cfg", s.IdentityProviderID) @@ -504,7 +508,12 @@ func TestResolveOpenAIStatusSources_FromConfig(t *testing.T) { assert.Equal(t, "config.yaml", s.SASource) } -func TestResolveOpenAIStatusSources_EnvOverridesConfig(t *testing.T) { +func TestResolveOpenAIStatusSources_VariablesWinAsASet(t *testing.T) { + // The run path (resolveOpenAICredential) treats the FULLSEND_OPENAI_* + // variables as one source: any of them set means all three come from + // variables, and the committed block is not consulted. status must + // report the same thing, or it would call a trio healthy that a run + // refuses. dir := t.TempDir() fullsendDir := filepath.Join(dir, ".fullsend") require.NoError(t, os.MkdirAll(fullsendDir, 0o755)) @@ -523,11 +532,35 @@ func TestResolveOpenAIStatusSources_EnvOverridesConfig(t *testing.T) { t.Setenv(openAIIdentityProviderIDEnv, "") t.Setenv(openAIServiceAccountIDEnv, "") - s := resolveOpenAIStatusSources(fullsendDir) + s, err := resolveOpenAIStatusSources(fullsendDir) + require.NoError(t, err) + assert.Equal(t, "variables", s.Source) assert.Equal(t, "from-env", s.Audience) assert.Contains(t, s.AudienceSource, "variable") - assert.Equal(t, "idp_cfg", s.IdentityProviderID) - assert.Equal(t, "config.yaml", s.IDPSource) + assert.Empty(t, s.IdentityProviderID, "config.yaml is not consulted once a variable is set") + assert.Empty(t, s.ServiceAccountID, "config.yaml is not consulted once a variable is set") + + ids := config.OpenAIWIFConfig{ + Audience: s.Audience, + IdentityProviderID: s.IdentityProviderID, + ServiceAccountID: s.ServiceAccountID, + } + assert.Equal(t, []string{"identity_provider_id", "service_account_id"}, ids.Missing(), + "the same partial trio a run would refuse") +} + +func TestResolveOpenAIStatusSources_MalformedConfigIsAnError(t *testing.T) { + dir := t.TempDir() + fullsendDir := filepath.Join(dir, ".fullsend") + require.NoError(t, os.MkdirAll(fullsendDir, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(fullsendDir, "config.yaml"), + []byte("inference: [this is not a mapping\n"), 0o644)) + t.Setenv(openAIAudienceEnv, "") + t.Setenv(openAIIdentityProviderIDEnv, "") + t.Setenv(openAIServiceAccountIDEnv, "") + + _, err := resolveOpenAIStatusSources(fullsendDir) + require.Error(t, err, "a broken config must not read as 'not configured yet'") } func TestResolveOpenAIStatusSources_NoConfig(t *testing.T) { @@ -538,7 +571,8 @@ func TestResolveOpenAIStatusSources_NoConfig(t *testing.T) { t.Setenv(openAIIdentityProviderIDEnv, "") t.Setenv(openAIServiceAccountIDEnv, "") - s := resolveOpenAIStatusSources(fullsendDir) + s, err := resolveOpenAIStatusSources(fullsendDir) + require.NoError(t, err) assert.Empty(t, s.Audience) assert.Empty(t, s.IdentityProviderID) assert.Empty(t, s.ServiceAccountID) @@ -553,6 +587,8 @@ func TestRunInferenceOpenAIStatus_NoConfig(t *testing.T) { t.Setenv(openAIServiceAccountIDEnv, "") t.Setenv("ACTIONS_ID_TOKEN_REQUEST_URL", "") t.Setenv("ACTIONS_ID_TOKEN_REQUEST_TOKEN", "") + // CI itself sets this; the command must behave the same either way. + t.Setenv("GITHUB_REPOSITORY", "") var buf bytes.Buffer cmd := newRootCmd() @@ -630,13 +666,13 @@ func TestRunInferenceOpenAIStatus_FullConfigNoActions(t *testing.T) { // --- buildRequestDoc tests --- func TestBuildRequestDoc_DefaultAudience(t *testing.T) { - doc := buildRequestDoc([]string{"acme/widget"}, "fullsend://acme", "", "") + doc := buildRequestDoc([]string{"acme/widget"}, "fullsend://acme", "", "", openAIDefaultRef) assert.Equal(t, "fullsend://acme", doc.Provider.Audience) assert.Equal(t, "fullsend://acme", doc.Reply.Audience) } func TestBuildRequestDoc_CorrectAssertions(t *testing.T) { - doc := buildRequestDoc([]string{"acme/widget", "acme/gadget"}, "fullsend://acme", "proj-1", "") + doc := buildRequestDoc([]string{"acme/widget", "acme/gadget"}, "fullsend://acme", "proj-1", "", openAIDefaultRef) require.Len(t, doc.Mappings, 2) @@ -651,13 +687,13 @@ func TestBuildRequestDoc_CorrectAssertions(t *testing.T) { } func TestBuildRequestDoc_ServiceAccountIDPerRepo(t *testing.T) { - doc := buildRequestDoc([]string{"acme/widget", "acme/gadget"}, "aud", "", "") + doc := buildRequestDoc([]string{"acme/widget", "acme/gadget"}, "aud", "", "", openAIDefaultRef) assert.Equal(t, "fullsend-widget-ci", doc.Mappings[0].Target.ServiceAccount) assert.Equal(t, "fullsend-gadget-ci", doc.Mappings[1].Target.ServiceAccount) } func TestBuildRequestDoc_SharedServiceAccount(t *testing.T) { - doc := buildRequestDoc([]string{"acme/widget", "acme/gadget"}, "aud", "", "shared-sa") + doc := buildRequestDoc([]string{"acme/widget", "acme/gadget"}, "aud", "", "shared-sa", openAIDefaultRef) assert.Equal(t, "shared-sa", doc.Mappings[0].Target.ServiceAccount) assert.Equal(t, "shared-sa", doc.Mappings[1].Target.ServiceAccount) } @@ -665,7 +701,7 @@ func TestBuildRequestDoc_SharedServiceAccount(t *testing.T) { // --- renderRequestMarkdown tests --- func TestRenderRequestMarkdown_ContainsExpectedSections(t *testing.T) { - doc := buildRequestDoc([]string{"acme/widget"}, "fullsend://acme", "", "") + doc := buildRequestDoc([]string{"acme/widget"}, "fullsend://acme", "", "", openAIDefaultRef) md, err := renderRequestMarkdown(doc) require.NoError(t, err) @@ -762,6 +798,7 @@ func TestBuildRequestDoc_JSONRoundTrip(t *testing.T) { "fullsend://acme", "openai-proj-001", "", + openAIDefaultRef, ) b, err := json.MarshalIndent(doc, "", " ") @@ -857,3 +894,213 @@ func TestInferenceOpenAIRequestCmd_MarkdownMultiRepo(t *testing.T) { assert.True(t, strings.Contains(output, "Service account ID for acme/widget")) assert.True(t, strings.Contains(output, "Service account ID for acme/gadget")) } + +// --- round trip: the document we generate is the document an admin returns --- + +func TestImport_AcceptsTheFilledInRequestDocument(t *testing.T) { + doc := buildRequestDoc([]string{"acme/widget"}, "fullsend://acme", "proj-1", "", openAIDefaultRef) + // What an administrator does: fill in the reply section of the file + // `request --format json` produced, and send the same file back. + doc.Reply.IdentityProviderID = "idp_live" + doc.Reply.ServiceAccountIDs["acme/widget"] = "sa_live" + + b, err := json.MarshalIndent(doc, "", " ") + require.NoError(t, err) + path := filepath.Join(t.TempDir(), "reply.json") + require.NoError(t, os.WriteFile(path, b, 0o644)) + + ids, err := resolveImportIDs([]string{path}, "", "", "", "") + require.NoError(t, err) + assert.Equal(t, "fullsend://acme", ids.Audience) + assert.Equal(t, "idp_live", ids.IdentityProviderID) + assert.Equal(t, "sa_live", ids.ServiceAccountID) + require.NoError(t, validateImportIDs(ids)) +} + +func TestImport_MultiRepoReplyNeedsASelector(t *testing.T) { + doc := buildRequestDoc([]string{"acme/widget", "acme/gadget"}, "fullsend://acme", "proj-1", "", openAIDefaultRef) + doc.Reply.IdentityProviderID = "idp_live" + doc.Reply.ServiceAccountIDs["acme/widget"] = "sa_widget" + doc.Reply.ServiceAccountIDs["acme/gadget"] = "sa_gadget" + b, err := json.MarshalIndent(doc, "", " ") + require.NoError(t, err) + path := filepath.Join(t.TempDir(), "reply.json") + require.NoError(t, os.WriteFile(path, b, 0o644)) + + _, err = resolveImportIDs([]string{path}, "", "", "", "") + require.Error(t, err, "two service accounts and nothing to choose with") + assert.Contains(t, err.Error(), "--repo") + assert.Contains(t, err.Error(), "acme/gadget") + + ids, err := resolveImportIDs([]string{path}, "", "", "", "acme/gadget") + require.NoError(t, err) + assert.Equal(t, "sa_gadget", ids.ServiceAccountID, "--repo selects that repository's account") + + _, err = resolveImportIDs([]string{path}, "", "", "", "acme/absent") + require.Error(t, err, "a repository the reply does not name") + assert.Contains(t, err.Error(), "no service account for acme/absent") +} + +// --- request generation guards --- + +func TestRequest_MixedOwnersNeedAnExplicitAudience(t *testing.T) { + cmd := newRootCmd() + var out strings.Builder + cmd.SetOut(&out) + cmd.SetErr(&out) + cmd.SetArgs([]string{"inference", "openai", "request", "acme/widget,other/gadget", "--project", "p"}) + err := cmd.Execute() + require.Error(t, err, "the default audience is derived from the owner, so two owners are ambiguous") + assert.Contains(t, err.Error(), "--audience") + + cmd = newRootCmd() + out.Reset() + cmd.SetOut(&out) + cmd.SetErr(&out) + cmd.SetArgs([]string{"inference", "openai", "request", "acme/widget,other/gadget", "--project", "p", "--audience", "shared-aud", "--format", "json"}) + require.NoError(t, cmd.Execute(), "explicit audience covers both owners") + assert.Contains(t, out.String(), "shared-aud") +} + +func TestRequest_RefIsOverridable(t *testing.T) { + doc := buildRequestDoc([]string{"acme/widget"}, "aud", "", "", "refs/heads/trunk") + require.Len(t, doc.Mappings, 1) + assert.Equal(t, "refs/heads/trunk", doc.Mappings[0].Assertions.Ref, + "a repository whose default branch is not main needs its own ref") + + assert.Equal(t, openAIDefaultRef, buildRequestDoc([]string{"acme/widget"}, "aud", "", "", "").Mappings[0].Assertions.Ref) +} + +func TestParseRepoList_Dedupes(t *testing.T) { + repos, err := parseRepoList("acme/widget, acme/widget ,acme/gadget") + require.NoError(t, err) + assert.Equal(t, []string{"acme/widget", "acme/gadget"}, repos) +} + +func TestRequestMarkdown_ExistingServiceAccountIsNotCreated(t *testing.T) { + md, err := renderRequestMarkdown(buildRequestDoc([]string{"acme/widget"}, "aud", "p", "sa_existing", openAIDefaultRef)) + require.NoError(t, err) + assert.Contains(t, md, "sa_existing (existing — map it, do not create a new one)") + assert.NotContains(t, md, "sa_existing (create inline") + + md, err = renderRequestMarkdown(buildRequestDoc([]string{"acme/widget"}, "aud", "p", "", openAIDefaultRef)) + require.NoError(t, err) + assert.Contains(t, md, "create inline in the mapping") +} + +func TestRequestMarkdown_StatesTheAssertionRules(t *testing.T) { + md, err := renderRequestMarkdown(buildRequestDoc([]string{"acme/widget"}, "aud", "p", "", openAIDefaultRef)) + require.NoError(t, err) + for _, want := range []string{ + "no wildcards", + "`repository_owner`", + "`workflow_ref`", + "`sub`", + "Do **not** create an API key", + "`api.model.request` only", + "`refs/heads/main`", + } { + assert.Contains(t, md, want, "the generated request must carry the rule: %s", want) + } +} + +func TestRunInferenceOpenAIStatus_RefusesToTestAnotherRepository(t *testing.T) { + dir := t.TempDir() + fullsendDir := filepath.Join(dir, ".fullsend") + require.NoError(t, os.MkdirAll(fullsendDir, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(fullsendDir, "config.yaml"), []byte(`inference: + openai: + audience: fullsend://acme + identity_provider_id: idp_123 + service_account_id: sa_456 +`), 0o644)) + + t.Setenv(openAIAudienceEnv, "") + t.Setenv(openAIIdentityProviderIDEnv, "") + t.Setenv(openAIServiceAccountIDEnv, "") + // A job that does have an OIDC endpoint, but belongs to another repo. + t.Setenv("ACTIONS_ID_TOKEN_REQUEST_URL", "https://oidc.example/token") + t.Setenv("ACTIONS_ID_TOKEN_REQUEST_TOKEN", "runner-token") + t.Setenv("GITHUB_REPOSITORY", "acme/other") + + exchanged := false + stubOpenAIExchange(t, func(context.Context, openaiwif.Config) (*openaiwif.Token, error) { + exchanged = true + return &openaiwif.Token{Value: "tok-abcdef123456", ExpiresAt: time.Now().Add(time.Hour), Scope: "api.model.request"}, nil + }) + + var buf bytes.Buffer + cmd := newRootCmd() + cmd.SetOut(&buf) + cmd.SetArgs([]string{"inference", "openai", "status", "acme/widget", "--fullsend-dir", fullsendDir}) + require.NoError(t, cmd.Execute()) + + assert.False(t, exchanged, "this job's token proves nothing about acme/widget") + assert.Contains(t, buf.String(), "cannot test acme/widget") +} + +func TestRunInferenceOpenAIStatus_ExchangesForItsOwnRepository(t *testing.T) { + dir := t.TempDir() + fullsendDir := filepath.Join(dir, ".fullsend") + require.NoError(t, os.MkdirAll(fullsendDir, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(fullsendDir, "config.yaml"), []byte(`inference: + openai: + audience: fullsend://acme + identity_provider_id: idp_123 + service_account_id: sa_456 +`), 0o644)) + + t.Setenv(openAIAudienceEnv, "") + t.Setenv(openAIIdentityProviderIDEnv, "") + t.Setenv(openAIServiceAccountIDEnv, "") + t.Setenv("ACTIONS_ID_TOKEN_REQUEST_URL", "https://oidc.example/token") + t.Setenv("ACTIONS_ID_TOKEN_REQUEST_TOKEN", "runner-token") + t.Setenv("GITHUB_REPOSITORY", "acme/widget") + + stubOpenAIExchange(t, func(context.Context, openaiwif.Config) (*openaiwif.Token, error) { + return &openaiwif.Token{Value: "tok-secret-value-abcdef", ExpiresAt: time.Now().Add(30 * time.Minute), Scope: "api.model.request"}, nil + }) + + var buf bytes.Buffer + cmd := newRootCmd() + cmd.SetOut(&buf) + cmd.SetArgs([]string{"inference", "openai", "status", "acme/widget", "--fullsend-dir", fullsendDir}) + require.NoError(t, cmd.Execute()) + + out := buf.String() + assert.Contains(t, out, "Exchange succeeded for acme/widget") + assert.Contains(t, out, "api.model.request") + assert.NotContains(t, out, "tok-secret-value-abcdef", "the token is never printed") +} + +func TestRunInferenceOpenAIStatus_OverBroadScopeFailsClosed(t *testing.T) { + dir := t.TempDir() + fullsendDir := filepath.Join(dir, ".fullsend") + require.NoError(t, os.MkdirAll(fullsendDir, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(fullsendDir, "config.yaml"), []byte(`inference: + openai: + audience: fullsend://acme + identity_provider_id: idp_123 + service_account_id: sa_456 +`), 0o644)) + + t.Setenv(openAIAudienceEnv, "") + t.Setenv(openAIIdentityProviderIDEnv, "") + t.Setenv(openAIServiceAccountIDEnv, "") + t.Setenv("ACTIONS_ID_TOKEN_REQUEST_URL", "https://oidc.example/token") + t.Setenv("ACTIONS_ID_TOKEN_REQUEST_TOKEN", "runner-token") + t.Setenv("GITHUB_REPOSITORY", "acme/widget") + + stubOpenAIExchange(t, func(context.Context, openaiwif.Config) (*openaiwif.Token, error) { + return &openaiwif.Token{Value: "tok-abcdef123456", ExpiresAt: time.Now().Add(time.Hour), Scope: "api.model.request api.admin"}, nil + }) + + var buf bytes.Buffer + cmd := newRootCmd() + cmd.SetOut(&buf) + cmd.SetErr(&buf) + cmd.SetArgs([]string{"inference", "openai", "status", "acme/widget", "--fullsend-dir", fullsendDir}) + err := cmd.Execute() + require.Error(t, err, "a run refuses a broader token, so status must not call it healthy") + assert.Contains(t, err.Error(), "refused") +} From 036f8f08b6ad612c9b95570127d4caf0c72d2771 Mon Sep 17 00:00:00 2001 From: Wayne Sun Date: Sun, 30 Aug 2026 13:06:35 -0400 Subject: [PATCH 4/5] fix(#6778): close the round-2 review gaps in status, import and the docs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Second review pass by Codex (gpt-5.6-sol) against the fixes, which confirmed five items closed and found four still open. status announced "Exchange succeeded" and printed the granted scope before checking it, so a token the run path refuses was reported as a success and only then rejected. The scope is checked first now, and nothing is called a success until it passes. The repository guard failed open: with a valid OIDC endpoint but no GITHUB_REPOSITORY, the mismatch check was skipped and the exchange ran, attributing the result to whatever repository was on the command line. An unknown job repository is now refused like a mismatched one. status still diverged from the run path in one case: run ignores the committed inference.openai block when there is no OIDC endpoint and OPENAI_API_KEY is set (a developer's key is not overridden by the repository's CI configuration), while status read the block anyway and named it as the source. It now applies the same rule and says so. import applied --service-account-id only after the reply document had already failed on the ambiguity that flag exists to resolve; the flag is now honoured first. Repository keys — deduplication, the mixed-owner guard, and reply lookup — compare case-insensitively, as GitHub does. Docs: the network claim is now a per-command table (request is offline, import reaches GitHub only with --variables, status exchanges with OpenAI inside Actions), --ref is documented, --repo's two roles are spelled out, and the schema comment no longer promises compatibility with an API OpenAI does not offer. The guide now states the mapping's real trust boundary — any job in the repository at the asserted ref that can request an OIDC token, not one named workflow — so a reader weighing "is it safe to commit these identifiers" sees that write access to main is the boundary that matters, and the spend limit is what bounds it. Tests: the scope path asserts the output carries no success line, the unknown-repository and static-key cases are pinned, and every status test now fixes OPENAI_API_KEY as well — one of them passed here only because of a key in the author's own shell. Assisted-by: Claude (fix), Codex (review), Grok (review) Signed-off-by: Wayne Sun --- docs/cli/inference.md | 20 ++- .../openai-workload-identity.md | 27 +++- internal/cli/inference_openai.go | 94 +++++++---- internal/cli/inference_openai_test.go | 151 ++++++++++++++++++ 4 files changed, 253 insertions(+), 39 deletions(-) diff --git a/docs/cli/inference.md b/docs/cli/inference.md index ddb214c1a4..304177041d 100644 --- a/docs/cli/inference.md +++ b/docs/cli/inference.md @@ -87,7 +87,15 @@ Read-only — makes no changes. Commands for enrolling repositories with OpenAI Workload Identity Federation (see the [operator guide](../guides/infrastructure/openai-workload-identity.md)). They need neither GCP access nor an OpenAI key. -`request` and `import` never reach the network: they produce a document and update local configuration. `status` reads configuration too, and — only when run inside a GitHub Actions job with `id-token: write` — performs one token exchange with OpenAI to prove the mapping accepts that repository, reporting the granted scope and expiry without ever printing the token. +What reaches the network, and what does not: + +| Command | Network | +|---|---| +| `request` | None. The document is computed from the repository names. | +| `import` | None, unless `--variables` is passed: that calls the GitHub API through `gh` to set the three repository variables. | +| `status` | Reads configuration only, except inside a GitHub Actions job with `id-token: write`, where it performs one token exchange with OpenAI to prove the mapping accepts *that* job's repository — reporting the granted scope and expiry, never the token. | + +No OpenAI API key is used or created by any of them. ### `inference openai request` @@ -98,6 +106,7 @@ fullsend inference openai request [,…] \ [--audience ""] \ [--project ""] \ [--service-account ""] \ + [--ref "refs/heads/"] \ [--format json|md] \ [--out ] ``` @@ -106,13 +115,16 @@ fullsend inference openai request [,…] \ |------|---------|-------------| | `--audience` | `fullsend://` | OpenAI Workload Identity audience | | `--project` | *(empty)* | OpenAI project name or ID for the service accounts | -| `--service-account` | `fullsend--ci` | Existing service account ID (default: one per repo) | +| `--service-account` | `fullsend--ci` | Existing service account ID to map (default: ask for one to be created per repository) | +| `--ref` | `refs/heads/main` | The `ref` assertion: the branch fullsend's agent workflows run from. Assertions are exact, so a repository whose default branch is not `main` needs this or every exchange fails | | `--format` | `json` | Output format: `json` (versioned schema) or `md` (copy-paste ticket) | | `--out` | *(stdout)* | Write output to a file | ### `inference openai import` -Takes the administrator's reply and writes `inference.openai` into `.fullsend/config.yaml` through the same setters as `fullsend github setup --openai-*`. All three identifiers must be present — a partial trio is refused. +Takes the administrator's reply and writes `inference.openai` into `.fullsend/config.yaml` through the same setters as `fullsend github setup --openai-*`. All three identifiers must be present — a partial trio is refused, and the config is validated before it is written. + +The reply file may be either the bare reply object or the whole document `request --format json` produced with its `reply` section filled in — an administrator can edit and return the same file. When it names service accounts for several repositories, pass `--repo ` to choose one, or `--service-account-id` to give the value outright. ```bash # From a reply JSON file: @@ -139,7 +151,7 @@ fullsend inference openai import \ | `--service-account-id` | | OpenAI service account ID | | `--fullsend-dir` | `.fullsend` | Path to the .fullsend configuration directory | | `--variables` | `false` | Set `FULLSEND_OPENAI_*` repository variables instead of config.yaml | -| `--repo` | | Target repository (`owner/repo`) for `--variables` | +| `--repo` | | Two roles: the target repository for `--variables`, and the repository to select from a reply that names several (`service_account_ids`). Matched case-insensitively | ### `inference openai status` diff --git a/docs/guides/infrastructure/openai-workload-identity.md b/docs/guides/infrastructure/openai-workload-identity.md index c94983932d..5e607dd44c 100644 --- a/docs/guides/infrastructure/openai-workload-identity.md +++ b/docs/guides/infrastructure/openai-workload-identity.md @@ -147,11 +147,19 @@ Two things not to do: - **Do not assert `repository_owner`, a prefix or any pattern instead of `repository`** — see the per-repository rule above. -**Which runs this mapping trusts.** `ref` = `refs/heads/main` covers every fullsend agent workflow -as installed on `main`, and GitHub mints the token for the repository the job runs in — so the -agent jobs fullsend starts for pull requests from forks are covered too (they run in your -repository, not the fork). What keeps an untrusted actor from starting such a run is fullsend's own -dispatch authorization (only events from people with write access trigger agents), not the mapping. +**Which runs this mapping trusts.** Be precise about this, because it is the boundary you are +setting: the mapping trusts **any job in that repository, at that ref, that can request an OIDC +token** — not one named workflow. `ref` = `refs/heads/main` covers every fullsend agent workflow as +installed on `main`, and it equally covers any other workflow on `main` with +`permissions: id-token: write`. Anyone who can merge to `main` can therefore add a workflow that +obtains a model token: the mapping's blast radius is "write access to this repository", and the +service account's spend limit is what bounds it. That is the trade for not asserting `workflow_ref`, +which cannot cover fullsend's several agent workflow files with one value. + +GitHub mints the token for the repository the job runs in, so the agent jobs fullsend starts for +pull requests from forks are covered too (they run in your repository, not the fork). What keeps an +untrusted *contributor* from starting such a run is fullsend's own dispatch authorization (only +events from people with write access trigger agents), not the mapping. If you want the mapping itself to exclude some triggers, add a claim the untrusted runs cannot carry — for example a GitHub `environment` that the agent jobs reference and that protects itself with required reviewers — and assert `environment == ""` here. A `workflow_dispatch` of a @@ -261,9 +269,12 @@ and the provider ID are typically the same for every repository and only the ser nothing. OpenAI issues a token only to a caller presenting a GitHub OIDC token whose claims match a mapping, and only your repository's `main` workflow can obtain one. fullsend reads `.fullsend/config.yaml` from the base branch for pull-request events, so a pull request cannot -change them for the run that reviews it. The worst thing someone with write access could do is -point the block at their own OpenAI organization — and pay for your runs. fullsend prints the three -values in the run log so you can always see which mapping a run used. +change them for the run that reviews it. Someone with write access could point the block at their +own OpenAI organization — and pay for their own runs — but note that write access already carries +the greater power described under [A3](#a3-map-the-repository-to-a-service-account-route-a): a +workflow they merge to `main` can obtain a model token from your mapping. Guard write access and the +project's spend limit accordingly. fullsend prints the three values in the run log so you can always +see which mapping a run used. If you would rather keep them out of the repository, set them as **repository variables** instead (Settings → Secrets and variables → Actions → Variables): `FULLSEND_OPENAI_AUDIENCE`, diff --git a/internal/cli/inference_openai.go b/internal/cli/inference_openai.go index 5fd8b90002..04cbe2d854 100644 --- a/internal/cli/inference_openai.go +++ b/internal/cli/inference_openai.go @@ -20,9 +20,10 @@ import ( ) const ( - // openAIRequestSchemaVersion is the versioned schema for the request - // JSON, so a future `fullsend inference openai apply request.json` - // can submit it unchanged. + // openAIRequestSchemaVersion versions this interchange format so a + // document written by one release is readable by the next. OpenAI has + // no API for providers or mappings today, so it is deliberately not a + // claim about any future submission contract. openAIRequestSchemaVersion = "1" // defaultOpenAIAudiencePrefix is the default audience convention. @@ -95,8 +96,11 @@ Federation. Generate a request document for your administrator, import the reply into fullsend configuration, and check the exchange status. -These commands do not call the OpenAI API. They produce documents -and update local configuration.`, +'request' and 'import' produce a document and update local +configuration. 'import --variables' calls the GitHub API through gh to +set repository variables, and 'status' performs one OpenAI token +exchange when it runs inside a GitHub Actions job with id-token: write. +No OpenAI API key is used or created by any of them.`, } cmd.AddCommand(newInferenceOpenAIRequestCmd()) cmd.AddCommand(newInferenceOpenAIImportCmd()) @@ -211,13 +215,16 @@ func parseRepoList(arg string) ([]string, error) { repos = append(repos, p) } // A duplicate would render as two identical mappings in the request. + // GitHub compares owner/repo case-insensitively, so Acme/Widget and + // acme/widget are the same repository; the first spelling is kept. seen := make(map[string]bool, len(repos)) deduped := repos[:0] for _, r := range repos { - if seen[r] { + key := strings.ToLower(r) + if seen[key] { continue } - seen[r] = true + seen[key] = true deduped = append(deduped, r) } return deduped, nil @@ -229,10 +236,11 @@ func repoOwners(repos []string) []string { seen := make(map[string]bool, len(repos)) for _, r := range repos { owner := strings.SplitN(r, "/", 2)[0] - if seen[owner] { + key := strings.ToLower(owner) + if seen[key] { continue } - seen[owner] = true + seen[key] = true owners = append(owners, owner) } return owners @@ -305,7 +313,10 @@ var requestMarkdownTmpl = template.Must(template.New("request").Parse(`# OpenAI | Use uploaded JWKS for token verification | **Off** | If the organization already has a provider with a different audience, -keep its audience and report it back (see Reply below). +use that audience — in the mapping assertions below as well as in the +Reply, since the two must match — and report it back. Re-running +` + "`" + `fullsend inference openai request --audience ""` + "`" + ` +regenerates this document with it. ## Service account mappings @@ -401,8 +412,10 @@ func (d openAIReplyDoc) serviceAccountFor(repo string) (string, error) { } } if repo != "" { - if v, ok := filled[repo]; ok { - return v, nil + for k, v := range filled { + if strings.EqualFold(k, repo) { + return v, nil + } } if len(filled) > 0 { keys := make([]string, 0, len(filled)) @@ -502,11 +515,16 @@ func resolveImportIDs(args []string, flagAudience, flagIdentityProviderID, flagS reply := doc.resolved() ids.Audience = reply.Audience ids.IdentityProviderID = reply.IdentityProviderID - sa, err := reply.serviceAccountFor(repo) - if err != nil { - return ids, err + // An explicit --service-account-id is the answer to the very + // ambiguity serviceAccountFor reports, so it is applied first + // rather than after an error the operator was told to fix that way. + if flagServiceAccountID == "" { + sa, err := reply.serviceAccountFor(repo) + if err != nil { + return ids, err + } + ids.ServiceAccountID = sa } - ids.ServiceAccountID = sa } // Flags override file values. @@ -702,6 +720,16 @@ func resolveOpenAIStatusSources(fullsendDir string) (openAIStatusSource, error) return s, nil } + // The run path ignores the committed block where an exchange is + // impossible and a static key is present — a developer's + // OPENAI_API_KEY is not overridden by the repository's CI + // configuration (run_openai.go, configApplies). Reporting the block + // as the source there would describe a run that will not happen. + if os.Getenv("ACTIONS_ID_TOKEN_REQUEST_URL") == "" && strings.TrimSpace(os.Getenv(openAIStaticKeyEnv)) != "" { + s.Source = "static key" + return s, nil + } + writer, err := config.LoadConfigWriter(fullsendDir, config.LoadOpts{MissingOK: true}) if err != nil { // A malformed or unreadable config must not read as "nothing @@ -747,6 +775,10 @@ func runInferenceOpenAIStatus(cmd *cobra.Command, printer *ui.Printer, repo, ful } if ids.IsZero() { + if sources.Source == "static key" { + printer.StepInfo(openAIStaticKeyEnv + " is set and this is not a GitHub Actions job, so a run here would use that key and ignore inference.openai — the same rule fullsend run applies") + return nil + } printer.StepFail("No OpenAI WIF identifiers configured") printer.StepInfo("Run 'fullsend inference openai import' or 'fullsend github setup --openai-*' to configure") return nil @@ -775,10 +807,17 @@ func runInferenceOpenAIStatus(cmd *cobra.Command, printer *ui.Printer, repo, ful } // Only now does the repository argument matter: an exchange proves - // the identity of the job it runs in, so checking another - // repository's name here would report a mapping as healthy on the - // strength of this repository's token. - if current := strings.TrimSpace(os.Getenv("GITHUB_REPOSITORY")); current != "" && !strings.EqualFold(current, repo) { + // the identity of the job it runs in, so exchanging for another + // repository's name would report a mapping as healthy on the strength + // of this repository's token. An unknown job repository is refused + // too — without it there is nothing to attribute the result to. + current := strings.TrimSpace(os.Getenv("GITHUB_REPOSITORY")) + switch { + case current == "": + printer.StepWarn("GITHUB_REPOSITORY is not set, so the exchange could not be attributed to " + repo) + printer.StepInfo("Run the command from a GitHub Actions job in " + repo) + return nil + case !strings.EqualFold(current, repo): printer.StepWarn(fmt.Sprintf("This job runs in %s, so it cannot test %s: an exchange here would prove %s's mapping only", current, repo, current)) printer.StepInfo("Run the command from a job in " + repo + " to test its mapping") return nil @@ -800,6 +839,14 @@ func runInferenceOpenAIStatus(cmd *cobra.Command, printer *ui.Printer, repo, ful return fmt.Errorf("OpenAI WIF exchange: %w", err) } + // A token broader than model access is refused by `fullsend run`, so + // nothing may be reported as succeeded until the scope has passed. + warning, scopeErr := checkOpenAIScope(tok.Scope) + if scopeErr != nil { + printer.StepFail("Exchange refused: the mapping grants more than model access") + return fmt.Errorf("OpenAI WIF token refused: %w", scopeErr) + } + printer.StepDone("Exchange succeeded for " + repo) printer.Blank() @@ -810,13 +857,6 @@ func runInferenceOpenAIStatus(cmd *cobra.Command, printer *ui.Printer, repo, ful printer.KeyValue("scope", scope) printer.KeyValue("expires_in", time.Until(tok.ExpiresAt).Round(time.Second).String()) - // A token broader than model access is refused by `fullsend run`, so - // it must not read as a healthy status here either. - warning, scopeErr := checkOpenAIScope(tok.Scope) - if scopeErr != nil { - printer.StepFail("The mapping grants more than model access") - return fmt.Errorf("OpenAI WIF token refused: %w", scopeErr) - } if warning != "" { printer.StepWarn(warning) } diff --git a/internal/cli/inference_openai_test.go b/internal/cli/inference_openai_test.go index 3e41e8ea81..6b16839422 100644 --- a/internal/cli/inference_openai_test.go +++ b/internal/cli/inference_openai_test.go @@ -497,6 +497,7 @@ func TestResolveOpenAIStatusSources_FromConfig(t *testing.T) { t.Setenv(openAIAudienceEnv, "") t.Setenv(openAIIdentityProviderIDEnv, "") t.Setenv(openAIServiceAccountIDEnv, "") + t.Setenv(openAIStaticKeyEnv, "") s, err := resolveOpenAIStatusSources(fullsendDir) require.NoError(t, err) @@ -531,6 +532,7 @@ func TestResolveOpenAIStatusSources_VariablesWinAsASet(t *testing.T) { t.Setenv(openAIAudienceEnv, "from-env") t.Setenv(openAIIdentityProviderIDEnv, "") t.Setenv(openAIServiceAccountIDEnv, "") + t.Setenv(openAIStaticKeyEnv, "") s, err := resolveOpenAIStatusSources(fullsendDir) require.NoError(t, err) @@ -558,6 +560,7 @@ func TestResolveOpenAIStatusSources_MalformedConfigIsAnError(t *testing.T) { t.Setenv(openAIAudienceEnv, "") t.Setenv(openAIIdentityProviderIDEnv, "") t.Setenv(openAIServiceAccountIDEnv, "") + t.Setenv(openAIStaticKeyEnv, "") _, err := resolveOpenAIStatusSources(fullsendDir) require.Error(t, err, "a broken config must not read as 'not configured yet'") @@ -570,6 +573,7 @@ func TestResolveOpenAIStatusSources_NoConfig(t *testing.T) { t.Setenv(openAIAudienceEnv, "") t.Setenv(openAIIdentityProviderIDEnv, "") t.Setenv(openAIServiceAccountIDEnv, "") + t.Setenv(openAIStaticKeyEnv, "") s, err := resolveOpenAIStatusSources(fullsendDir) require.NoError(t, err) @@ -585,6 +589,7 @@ func TestRunInferenceOpenAIStatus_NoConfig(t *testing.T) { t.Setenv(openAIAudienceEnv, "") t.Setenv(openAIIdentityProviderIDEnv, "") t.Setenv(openAIServiceAccountIDEnv, "") + t.Setenv(openAIStaticKeyEnv, "") t.Setenv("ACTIONS_ID_TOKEN_REQUEST_URL", "") t.Setenv("ACTIONS_ID_TOKEN_REQUEST_TOKEN", "") // CI itself sets this; the command must behave the same either way. @@ -616,6 +621,7 @@ func TestRunInferenceOpenAIStatus_PartialConfig(t *testing.T) { t.Setenv(openAIAudienceEnv, "") t.Setenv(openAIIdentityProviderIDEnv, "") t.Setenv(openAIServiceAccountIDEnv, "") + t.Setenv(openAIStaticKeyEnv, "") t.Setenv("ACTIONS_ID_TOKEN_REQUEST_URL", "") t.Setenv("ACTIONS_ID_TOKEN_REQUEST_TOKEN", "") @@ -647,6 +653,7 @@ func TestRunInferenceOpenAIStatus_FullConfigNoActions(t *testing.T) { t.Setenv(openAIAudienceEnv, "") t.Setenv(openAIIdentityProviderIDEnv, "") t.Setenv(openAIServiceAccountIDEnv, "") + t.Setenv(openAIStaticKeyEnv, "") t.Setenv("ACTIONS_ID_TOKEN_REQUEST_URL", "") t.Setenv("ACTIONS_ID_TOKEN_REQUEST_TOKEN", "") @@ -779,6 +786,7 @@ func TestInferenceOpenAIStatusCmd_DoesNotRequireGitHubToken(t *testing.T) { t.Setenv(openAIAudienceEnv, "") t.Setenv(openAIIdentityProviderIDEnv, "") t.Setenv(openAIServiceAccountIDEnv, "") + t.Setenv(openAIStaticKeyEnv, "") t.Setenv("ACTIONS_ID_TOKEN_REQUEST_URL", "") t.Setenv("ACTIONS_ID_TOKEN_REQUEST_TOKEN", "") @@ -1018,6 +1026,7 @@ func TestRunInferenceOpenAIStatus_RefusesToTestAnotherRepository(t *testing.T) { t.Setenv(openAIAudienceEnv, "") t.Setenv(openAIIdentityProviderIDEnv, "") t.Setenv(openAIServiceAccountIDEnv, "") + t.Setenv(openAIStaticKeyEnv, "") // A job that does have an OIDC endpoint, but belongs to another repo. t.Setenv("ACTIONS_ID_TOKEN_REQUEST_URL", "https://oidc.example/token") t.Setenv("ACTIONS_ID_TOKEN_REQUEST_TOKEN", "runner-token") @@ -1053,6 +1062,7 @@ func TestRunInferenceOpenAIStatus_ExchangesForItsOwnRepository(t *testing.T) { t.Setenv(openAIAudienceEnv, "") t.Setenv(openAIIdentityProviderIDEnv, "") t.Setenv(openAIServiceAccountIDEnv, "") + t.Setenv(openAIStaticKeyEnv, "") t.Setenv("ACTIONS_ID_TOKEN_REQUEST_URL", "https://oidc.example/token") t.Setenv("ACTIONS_ID_TOKEN_REQUEST_TOKEN", "runner-token") t.Setenv("GITHUB_REPOSITORY", "acme/widget") @@ -1087,6 +1097,7 @@ func TestRunInferenceOpenAIStatus_OverBroadScopeFailsClosed(t *testing.T) { t.Setenv(openAIAudienceEnv, "") t.Setenv(openAIIdentityProviderIDEnv, "") t.Setenv(openAIServiceAccountIDEnv, "") + t.Setenv(openAIStaticKeyEnv, "") t.Setenv("ACTIONS_ID_TOKEN_REQUEST_URL", "https://oidc.example/token") t.Setenv("ACTIONS_ID_TOKEN_REQUEST_TOKEN", "runner-token") t.Setenv("GITHUB_REPOSITORY", "acme/widget") @@ -1104,3 +1115,143 @@ func TestRunInferenceOpenAIStatus_OverBroadScopeFailsClosed(t *testing.T) { require.Error(t, err, "a run refuses a broader token, so status must not call it healthy") assert.Contains(t, err.Error(), "refused") } + +func TestRunInferenceOpenAIStatus_OverBroadScopeNeverPrintsSuccess(t *testing.T) { + dir := t.TempDir() + fullsendDir := filepath.Join(dir, ".fullsend") + require.NoError(t, os.MkdirAll(fullsendDir, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(fullsendDir, "config.yaml"), []byte(`inference: + openai: + audience: fullsend://acme + identity_provider_id: idp_123 + service_account_id: sa_456 +`), 0o644)) + + t.Setenv(openAIAudienceEnv, "") + t.Setenv(openAIIdentityProviderIDEnv, "") + t.Setenv(openAIServiceAccountIDEnv, "") + t.Setenv(openAIStaticKeyEnv, "") + t.Setenv("ACTIONS_ID_TOKEN_REQUEST_URL", "https://oidc.example/token") + t.Setenv("ACTIONS_ID_TOKEN_REQUEST_TOKEN", "runner-token") + t.Setenv("GITHUB_REPOSITORY", "acme/widget") + + stubOpenAIExchange(t, func(context.Context, openaiwif.Config) (*openaiwif.Token, error) { + return &openaiwif.Token{Value: "tok-abcdef123456", ExpiresAt: time.Now().Add(time.Hour), Scope: "api.model.request api.admin"}, nil + }) + + var buf bytes.Buffer + cmd := newRootCmd() + cmd.SetOut(&buf) + cmd.SetErr(&buf) + cmd.SetArgs([]string{"inference", "openai", "status", "acme/widget", "--fullsend-dir", fullsendDir}) + require.Error(t, cmd.Execute()) + assert.NotContains(t, buf.String(), "Exchange succeeded", + "a token the run path refuses must never be announced as a success") +} + +func TestRunInferenceOpenAIStatus_UnknownJobRepositoryFailsClosed(t *testing.T) { + dir := t.TempDir() + fullsendDir := filepath.Join(dir, ".fullsend") + require.NoError(t, os.MkdirAll(fullsendDir, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(fullsendDir, "config.yaml"), []byte(`inference: + openai: + audience: fullsend://acme + identity_provider_id: idp_123 + service_account_id: sa_456 +`), 0o644)) + + t.Setenv(openAIAudienceEnv, "") + t.Setenv(openAIIdentityProviderIDEnv, "") + t.Setenv(openAIServiceAccountIDEnv, "") + t.Setenv(openAIStaticKeyEnv, "") + t.Setenv("ACTIONS_ID_TOKEN_REQUEST_URL", "https://oidc.example/token") + t.Setenv("ACTIONS_ID_TOKEN_REQUEST_TOKEN", "runner-token") + t.Setenv("GITHUB_REPOSITORY", "") + + exchanged := false + stubOpenAIExchange(t, func(context.Context, openaiwif.Config) (*openaiwif.Token, error) { + exchanged = true + return &openaiwif.Token{Value: "tok-abcdef123456", ExpiresAt: time.Now().Add(time.Hour), Scope: "api.model.request"}, nil + }) + + var buf bytes.Buffer + cmd := newRootCmd() + cmd.SetOut(&buf) + cmd.SetArgs([]string{"inference", "openai", "status", "acme/widget", "--fullsend-dir", fullsendDir}) + require.NoError(t, cmd.Execute()) + assert.False(t, exchanged, "with no job repository there is nothing to attribute the exchange to") + assert.Contains(t, buf.String(), "GITHUB_REPOSITORY is not set") +} + +func TestResolveOpenAIStatusSources_StaticKeyWinsOffActions(t *testing.T) { + // resolveOpenAICredential ignores the committed block when there is + // no OIDC endpoint and OPENAI_API_KEY is set; status must say the + // same, or it would describe a run that cannot happen. + dir := t.TempDir() + fullsendDir := filepath.Join(dir, ".fullsend") + require.NoError(t, os.MkdirAll(fullsendDir, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(fullsendDir, "config.yaml"), []byte(`inference: + openai: + audience: fullsend://acme + identity_provider_id: idp_123 + service_account_id: sa_456 +`), 0o644)) + + t.Setenv(openAIAudienceEnv, "") + t.Setenv(openAIIdentityProviderIDEnv, "") + t.Setenv(openAIServiceAccountIDEnv, "") + t.Setenv(openAIStaticKeyEnv, "") + t.Setenv("ACTIONS_ID_TOKEN_REQUEST_URL", "") + t.Setenv(openAIStaticKeyEnv, "sk-local-developer-key") + + s, err := resolveOpenAIStatusSources(fullsendDir) + require.NoError(t, err) + assert.Equal(t, "static key", s.Source) + assert.Empty(t, s.Audience, "the committed block is not what a run here would use") + + // With an OIDC endpoint the block applies again, as it does for a run. + t.Setenv("ACTIONS_ID_TOKEN_REQUEST_URL", "https://oidc.example/token") + s, err = resolveOpenAIStatusSources(fullsendDir) + require.NoError(t, err) + assert.Equal(t, "config.yaml", s.Source) + assert.Equal(t, "fullsend://acme", s.Audience) +} + +func TestImport_ServiceAccountFlagResolvesAnAmbiguousReply(t *testing.T) { + doc := buildRequestDoc([]string{"acme/widget", "acme/gadget"}, "fullsend://acme", "proj-1", "", openAIDefaultRef) + doc.Reply.IdentityProviderID = "idp_live" + doc.Reply.ServiceAccountIDs["acme/widget"] = "sa_widget" + doc.Reply.ServiceAccountIDs["acme/gadget"] = "sa_gadget" + b, err := json.MarshalIndent(doc, "", " ") + require.NoError(t, err) + path := filepath.Join(t.TempDir(), "reply.json") + require.NoError(t, os.WriteFile(path, b, 0o644)) + + ids, err := resolveImportIDs([]string{path}, "", "", "sa_chosen", "") + require.NoError(t, err, "--service-account-id is the answer to the ambiguity, not a value applied after failing on it") + assert.Equal(t, "sa_chosen", ids.ServiceAccountID) +} + +func TestImport_RepoSelectionIsCaseInsensitive(t *testing.T) { + doc := buildRequestDoc([]string{"acme/widget", "acme/gadget"}, "fullsend://acme", "proj-1", "", openAIDefaultRef) + doc.Reply.IdentityProviderID = "idp_live" + doc.Reply.ServiceAccountIDs["acme/widget"] = "sa_widget" + doc.Reply.ServiceAccountIDs["acme/gadget"] = "sa_gadget" + b, err := json.MarshalIndent(doc, "", " ") + require.NoError(t, err) + path := filepath.Join(t.TempDir(), "reply.json") + require.NoError(t, os.WriteFile(path, b, 0o644)) + + ids, err := resolveImportIDs([]string{path}, "", "", "", "Acme/Widget") + require.NoError(t, err, "GitHub matches owner/repo case-insensitively") + assert.Equal(t, "sa_widget", ids.ServiceAccountID) +} + +func TestParseRepoList_DedupesCaseInsensitively(t *testing.T) { + repos, err := parseRepoList("acme/widget,Acme/Widget,acme/gadget") + require.NoError(t, err) + assert.Equal(t, []string{"acme/widget", "acme/gadget"}, repos) + + // The same rule keeps the mixed-owner guard honest. + assert.Equal(t, []string{"acme"}, repoOwners([]string{"acme/widget", "Acme/gadget"})) +} From b8a47ccfbe33d226b4f837e674b7a504ef93ea16 Mon Sep 17 00:00:00 2001 From: Wayne Sun Date: Sun, 30 Aug 2026 13:23:36 -0400 Subject: [PATCH 5/5] fix(#6778): set repository variables through the forge client, not gh MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Third review pass by Grok 4.6, whose blocking finding was an architectural one: `import --variables` shelled out to the gh CLI. docs/contributing/forge-abstraction.md prohibits that outside internal/forge/github, and forge.Client.CreateOrUpdateRepoVariable is what `github setup` and `github set` already use. It now goes through the same client and the same token resolution, so it needs no gh binary on PATH and reports API errors like every other forge call; the partial-write warning is kept, and a test fails the second write to prove it. Correctness, from the same pass: - A full request document could lose the audience. An administrator reusing an existing provider is told to record its audience in the provider block, but import read only the reply section, so it kept the audience we had proposed and every exchange then failed the assertion. The provider block is now the fallback. - status exited 0 for an unconfigured repository and for a partial trio — the two states a run refuses — so it could not gate a check the way the GCP status command does. Both now exit non-zero. - status met an org-mode config with a generic "nothing configured"; it now names it as import does (org mode is deprecated, ADR 0044). - A reply naming several repositories with only one service account filled in was treated as unambiguous; filling the second in later would then have silently misattributed it. A selector is required whenever the reply names more than one repository. - request now trims its flag values, so a whitespace-only --audience cannot slip past the mixed-owner guard into the document. Docs: the inference group is described accurately in docs/cli/README.md and the operations command index; --service-account's default column no longer claims a value the flag does not have; the guide and the CLI reference both say import writes locally and the file must be committed; and the schema's long help matches the version constant's comment. Assisted-by: Claude (fix), Codex (review), Grok (review) Signed-off-by: Wayne Sun --- docs/cli/README.md | 2 +- docs/cli/inference.md | 4 +- docs/guides/dev/cli-internals.md | 2 +- docs/guides/getting-started/operations.md | 3 + .../openai-workload-identity.md | 4 +- internal/cli/inference_openai.go | 88 ++++++++--- internal/cli/inference_openai_test.go | 144 +++++++++++++++--- 7 files changed, 202 insertions(+), 45 deletions(-) diff --git a/docs/cli/README.md b/docs/cli/README.md index 51297619a5..449e4eb0a9 100644 --- a/docs/cli/README.md +++ b/docs/cli/README.md @@ -16,7 +16,7 @@ Download the latest binary from [GitHub Releases](https://github.com/fullsend-ai |--------------|-------------| | [`fullsend agent`](agent.md) | Manage agent registrations — add, list, set, update, remove | | [`fullsend github`](github.md) | Configure GitHub orgs and repos — setup, enrollment, day-2 operations | -| [`fullsend inference`](inference.md) | Manage GCP Workload Identity Federation for Agent Platform access | +| [`fullsend inference`](inference.md) | Manage inference credentials — GCP Workload Identity Federation for Agent Platform, and OpenAI WIF enrolment for GPT on pi | | [`fullsend mint`](mint.md) | Deploy and manage the OIDC token mint service | | [`fullsend repos`](repos.md) | Manage per-repo installations at scale via declarative manifest | diff --git a/docs/cli/inference.md b/docs/cli/inference.md index 304177041d..5f3b85b016 100644 --- a/docs/cli/inference.md +++ b/docs/cli/inference.md @@ -115,14 +115,14 @@ fullsend inference openai request [,…] \ |------|---------|-------------| | `--audience` | `fullsend://` | OpenAI Workload Identity audience | | `--project` | *(empty)* | OpenAI project name or ID for the service accounts | -| `--service-account` | `fullsend--ci` | Existing service account ID to map (default: ask for one to be created per repository) | +| `--service-account` | *(none)* | Map this existing service account instead of asking for `fullsend--ci` to be created in the mapping | | `--ref` | `refs/heads/main` | The `ref` assertion: the branch fullsend's agent workflows run from. Assertions are exact, so a repository whose default branch is not `main` needs this or every exchange fails | | `--format` | `json` | Output format: `json` (versioned schema) or `md` (copy-paste ticket) | | `--out` | *(stdout)* | Write output to a file | ### `inference openai import` -Takes the administrator's reply and writes `inference.openai` into `.fullsend/config.yaml` through the same setters as `fullsend github setup --openai-*`. All three identifiers must be present — a partial trio is refused, and the config is validated before it is written. +Takes the administrator's reply and writes `inference.openai` into `.fullsend/config.yaml` through the same setters as `fullsend github setup --openai-*`. All three identifiers must be present — a partial trio is refused, and the config is validated before it is written. The write is local: commit `.fullsend/config.yaml` afterwards, since fullsend reads the base branch for pull-request events. The reply file may be either the bare reply object or the whole document `request --format json` produced with its `reply` section filled in — an administrator can edit and return the same file. When it names service accounts for several repositories, pass `--repo ` to choose one, or `--service-account-id` to give the value outright. diff --git a/docs/guides/dev/cli-internals.md b/docs/guides/dev/cli-internals.md index ae5581ef20..61368bf999 100644 --- a/docs/guides/dev/cli-internals.md +++ b/docs/guides/dev/cli-internals.md @@ -696,7 +696,7 @@ var executableFiles = map[string]struct{}{ | `internal/cli/admin.go` | ~2415 | Install/uninstall/analyze/enable/disable | | `internal/cli/mint.go` | ~1022 | Mint deploy/enroll/unenroll/status | | `internal/cli/inference.go` | ~408 | Inference WIF provision/status (GCP) | -| `internal/cli/inference_openai.go` | ~760 | OpenAI WIF enrolment: request document, reply import, status/exchange | +| `internal/cli/inference_openai.go` | ~900 | OpenAI WIF enrolment: request document, reply import, status/exchange | | `internal/cli/github.go` | ~966 | GitHub setup/set/status/uninstall/sync-scaffold/enroll/unenroll | | `internal/cli/issues.go` | ~430 | Issue read/write commands (`fullsend issues get`, `post-comment`) | | `internal/cli/tracker_client.go` | ~122 | Tracker client factory (GitHub/GitLab/Jira) | diff --git a/docs/guides/getting-started/operations.md b/docs/guides/getting-started/operations.md index d94cbcca96..59d1039df3 100644 --- a/docs/guides/getting-started/operations.md +++ b/docs/guides/getting-started/operations.md @@ -92,6 +92,9 @@ For organizations that separate GCP and GitHub responsibilities across teams, fu | GCP Admin (Inference) | `fullsend inference provision ` | Create WIF pool/provider and grant Agent Platform access (idempotent — safe to re-run for new orgs) | | GCP Admin (Inference) | `fullsend inference deprovision ` | Remove org or repo from WIF | | GCP Admin (Inference) | `fullsend inference status ` | Check WIF health, print config values | +| Repo Maintainer (OpenAI) | `fullsend inference openai request [,...]` | Generate the provider/mapping request for an OpenAI organization admin (GPT on pi) | +| Repo Maintainer (OpenAI) | `fullsend inference openai import [reply.json]` | Record the admin's reply in `config.yaml`, or set the repository variables | +| Repo Maintainer (OpenAI) | `fullsend inference openai status ` | Check the OpenAI WIF identifiers, and the exchange when run inside Actions | | GitHub Maintainer | `fullsend github setup ` | Configure GitHub org or repo (no GCP needed) | | GitHub Maintainer | `fullsend github enroll [repo...]` | Add repositories to agent enrollment | | GitHub Maintainer | `fullsend github unenroll [repo...]` | Remove repositories from agent enrollment | diff --git a/docs/guides/infrastructure/openai-workload-identity.md b/docs/guides/infrastructure/openai-workload-identity.md index 5e607dd44c..46b293c19f 100644 --- a/docs/guides/infrastructure/openai-workload-identity.md +++ b/docs/guides/infrastructure/openai-workload-identity.md @@ -222,7 +222,9 @@ The reply gives you the three identifiers from [What you end up with](#what-you- Record them with [`fullsend inference openai import`](../../cli/inference.md#inference-openai-import) — it takes the filled-in reply JSON (`fullsend inference openai import reply.json`) or the three values as flags, -and writes them the same way [step 4](#4-tell-fullsend-the-three-identifiers) does. +and writes the same `inference.openai` block [step 4](#4-tell-fullsend-the-three-identifiers) +describes. Unlike `fullsend github setup`, which opens a pull request, `import` only writes the file +on your machine: commit it, or a CI run will not see the identifiers. To check the enrolment before a real agent run, `fullsend inference openai status /` prints the resolved identifiers and where they came from; inside a GitHub Actions job with diff --git a/internal/cli/inference_openai.go b/internal/cli/inference_openai.go index 04cbe2d854..9d9b574887 100644 --- a/internal/cli/inference_openai.go +++ b/internal/cli/inference_openai.go @@ -5,7 +5,6 @@ import ( "encoding/json" "fmt" "os" - "os/exec" "path/filepath" "sort" "strings" @@ -15,6 +14,7 @@ import ( "github.com/spf13/cobra" "github.com/fullsend-ai/fullsend/internal/config" + gh "github.com/fullsend-ai/fullsend/internal/forge/github" "github.com/fullsend-ai/fullsend/internal/inference/openaiwif" "github.com/fullsend-ai/fullsend/internal/ui" ) @@ -126,8 +126,8 @@ Every value in the document is computed from the repository names. Nothing is sent anywhere; the command needs no credentials. Output formats: - --format json A stable, versioned JSON schema suitable for a - future API submission. + --format json A versioned interchange schema, so a document written + by one release is readable by the next. --format md A copy-paste ticket/email matching the guide's route-B template.`, Args: cobra.ExactArgs(1), @@ -140,6 +140,11 @@ Output formats: return fmt.Errorf("at least one owner/repo is required") } + audience = strings.TrimSpace(audience) + project = strings.TrimSpace(project) + serviceAccount = strings.TrimSpace(serviceAccount) + ref = strings.TrimSpace(ref) + // The default audience is derived from the owner, so every // repository in one request must share it — otherwise the // second owner's mapping would silently carry the first @@ -370,6 +375,13 @@ type openAIReplyDoc struct { ServiceAccountID string `json:"service_account_id,omitempty"` ServiceAccountIDs map[string]string `json:"service_account_ids,omitempty"` + // Provider carries the audience when an administrator reusing an + // existing provider edited it there — which is what the generated + // document and the guide tell them to do. + Provider *struct { + Audience string `json:"audience"` + } `json:"provider,omitempty"` + // Reply carries the same fields when the file is a full request // document; its values win, since that is the section the // administrator was asked to fill in. @@ -386,6 +398,12 @@ func (d openAIReplyDoc) resolved() openAIReplyDoc { if out.Audience == "" { out.Audience = d.Audience } + // reply.audience wins, then the provider block: an administrator who + // reused a provider updates one of the two, and the mapping was + // written against whichever they set. + if out.Audience == "" && d.Provider != nil { + out.Audience = d.Provider.Audience + } if out.IdentityProviderID == "" { out.IdentityProviderID = d.IdentityProviderID } @@ -426,14 +444,22 @@ func (d openAIReplyDoc) serviceAccountFor(repo string) (string, error) { return "", fmt.Errorf("the reply has no service account for %s (it names %s)", repo, strings.Join(keys, ", ")) } } - switch len(filled) { - case 0: - return "", nil - case 1: - for _, v := range filled { - return v, nil + // One filled entry among several named repositories is still + // ambiguous: the others are filled in later, and a silent pick would + // then attribute the wrong account. + if len(d.ServiceAccountIDs) <= 1 { + switch len(filled) { + case 0: + return "", nil + case 1: + for _, v := range filled { + return v, nil + } } } + if len(filled) == 0 { + return "", nil + } keys := make([]string, 0, len(filled)) for k := range filled { keys = append(keys, k) @@ -492,7 +518,7 @@ token with variable-write permissions and --repo).`, cmd.Flags().StringVar(&flagServiceAccountID, "service-account-id", "", "OpenAI service account ID") cmd.Flags().StringVar(&fullsendDir, "fullsend-dir", ".fullsend", "path to the .fullsend configuration directory") cmd.Flags().BoolVar(&variables, "variables", false, "set FULLSEND_OPENAI_* repository variables instead of writing config.yaml (requires --repo)") - cmd.Flags().StringVar(&repo, "repo", "", "target repository (owner/repo) for --variables") + cmd.Flags().StringVar(&repo, "repo", "", "target repository (owner/repo) for --variables, and the repository to select from a reply naming several") return cmd } @@ -585,6 +611,7 @@ func runImportConfig(printer *ui.Printer, ids config.OpenAIWIFConfig, fullsendDi } printer.StepDone("Wrote inference.openai to " + configPath) + printer.StepInfo("This is a local write: commit " + configPath + " so CI runs see it (fullsend reads the base branch for pull-request events)") printer.KeyValue("audience", ids.Audience) printer.KeyValue("identity_provider_id", ids.IdentityProviderID) printer.KeyValue("service_account_id", ids.ServiceAccountID) @@ -592,12 +619,22 @@ func runImportConfig(printer *ui.Printer, ids config.OpenAIWIFConfig, fullsendDi return nil } -// ghRunner runs a gh CLI command and returns its combined output. Tests -// can replace this variable with a stub. -var ghRunner = func(ctx context.Context, args ...string) (string, error) { - cmd := exec.CommandContext(ctx, "gh", args...) - out, err := cmd.CombinedOutput() - return string(out), err +// repoVariableSetter is the forge operation `import --variables` needs. +// Setting a repository variable is a forge operation, so it goes through +// forge.Client like every other one (docs/contributing/forge-abstraction.md +// prohibits shelling out to the gh CLI outside internal/forge/github). +type repoVariableSetter interface { + CreateOrUpdateRepoVariable(ctx context.Context, owner, repo, name, value string) error +} + +// newRepoVariableSetter builds the client from the usual token +// resolution; tests replace it. +var newRepoVariableSetter = func() (repoVariableSetter, error) { + token, err := resolveToken() + if err != nil { + return nil, err + } + return gh.New(token), nil } func runImportVariables(ctx context.Context, printer *ui.Printer, ids config.OpenAIWIFConfig, repo string) error { @@ -618,6 +655,11 @@ func runImportVariables(ctx context.Context, printer *ui.Printer, ids config.Ope return fmt.Errorf("invalid repo name %q", repoName) } + client, err := newRepoVariableSetter() + if err != nil { + return err + } + type varEntry struct { name string value string @@ -633,15 +675,13 @@ func runImportVariables(ctx context.Context, printer *ui.Printer, ids config.Ope // what was written so the operator can re-run or clear it. var written []string for _, v := range vars { - ghArgs := []string{"variable", "set", v.name, "--repo", repo, "--body", v.value} printer.StepStart(fmt.Sprintf("Setting %s on %s", v.name, repo)) - out, err := ghRunner(ctx, ghArgs...) - if err != nil { + if err := client.CreateOrUpdateRepoVariable(ctx, owner, repoName, v.name, v.value); err != nil { printer.StepFail(fmt.Sprintf("Failed to set %s", v.name)) if len(written) > 0 { printer.StepWarn(fmt.Sprintf("%s already set on %s: the repository now holds a partial trio, which a run refuses — re-run the same command to finish, or remove them", strings.Join(written, ", "), repo)) } - return fmt.Errorf("setting variable %s on %s: %w\n%s", v.name, repo, err, out) + return fmt.Errorf("setting variable %s on %s: %w", v.name, repo, err) } written = append(written, v.name) printer.StepDone(fmt.Sprintf("Set %s on %s", v.name, repo)) @@ -739,7 +779,9 @@ func resolveOpenAIStatusSources(fullsendDir string) (openAIStatusSource, error) } perRepo, ok := writer.(config.PerRepoConfigReader) if !ok { - return s, nil + // Same condition import refuses by name, rather than a generic + // "nothing configured" (org mode is deprecated, ADR 0044). + return s, fmt.Errorf("%s contains an org-mode config; OpenAI WIF enrolment is per-repo", fullsendDir) } cfgIDs := perRepo.ConfigInferenceOpenAI().Trimmed() s.Source = "config.yaml" @@ -781,7 +823,7 @@ func runInferenceOpenAIStatus(cmd *cobra.Command, printer *ui.Printer, repo, ful } printer.StepFail("No OpenAI WIF identifiers configured") printer.StepInfo("Run 'fullsend inference openai import' or 'fullsend github setup --openai-*' to configure") - return nil + return fmt.Errorf("no OpenAI WIF identifiers configured for %s", repo) } if missing := ids.Missing(); len(missing) > 0 { @@ -790,7 +832,7 @@ func runInferenceOpenAIStatus(cmd *cobra.Command, printer *ui.Printer, repo, ful printer.StepInfo("A run takes the three identifiers from one source: with any FULLSEND_OPENAI_* variable set, all three must come from variables — the config.yaml block is not consulted") } printer.StepInfo("All three identifiers must be set for the exchange to work") - return nil + return fmt.Errorf("OpenAI WIF is partially configured in %s: missing %s", sources.Source, strings.Join(missing, ", ")) } printer.StepDone("All three identifiers are set") diff --git a/internal/cli/inference_openai_test.go b/internal/cli/inference_openai_test.go index 6b16839422..172172894c 100644 --- a/internal/cli/inference_openai_test.go +++ b/internal/cli/inference_openai_test.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "encoding/json" + "fmt" "os" "path/filepath" "strings" @@ -433,15 +434,34 @@ func TestRunImportVariables_RequiresOwnerSlashRepo(t *testing.T) { assert.Contains(t, err.Error(), "owner/repo") } -func TestRunImportVariables_CallsGH(t *testing.T) { - // Replace ghRunner with a stub. - var calls [][]string - origGH := ghRunner - ghRunner = func(_ context.Context, args ...string) (string, error) { - calls = append(calls, args) - return "", nil +// fakeVariableSetter records repository-variable writes and can fail on +// a chosen call, so the partial-write path is exercised. +type fakeVariableSetter struct { + calls [][4]string + failOn int + failWith error +} + +func (f *fakeVariableSetter) CreateOrUpdateRepoVariable(_ context.Context, owner, repo, name, value string) error { + f.calls = append(f.calls, [4]string{owner, repo, name, value}) + if f.failWith != nil && len(f.calls) == f.failOn { + return f.failWith } - defer func() { ghRunner = origGH }() + return nil +} + +func stubVariableSetter(t *testing.T, f *fakeVariableSetter) { + t.Helper() + orig := newRepoVariableSetter + newRepoVariableSetter = func() (repoVariableSetter, error) { return f, nil } + t.Cleanup(func() { newRepoVariableSetter = orig }) +} + +func TestRunImportVariables_UsesTheForgeClient(t *testing.T) { + // Repository variables are a forge operation: they go through + // forge.Client, never the gh CLI (docs/contributing/forge-abstraction.md). + f := &fakeVariableSetter{} + stubVariableSetter(t, f) ids := config.OpenAIWIFConfig{ Audience: "fullsend://acme", @@ -451,13 +471,32 @@ func TestRunImportVariables_CallsGH(t *testing.T) { var buf bytes.Buffer printer := newTestPrinter(&buf) - err := runImportVariables(context.Background(), printer, ids, "acme/widget") - require.NoError(t, err) + require.NoError(t, runImportVariables(context.Background(), printer, ids, "acme/widget")) + require.Len(t, f.calls, 3) + assert.Equal(t, [4]string{"acme", "widget", openAIAudienceEnv, "fullsend://acme"}, f.calls[0]) + assert.Equal(t, [4]string{"acme", "widget", openAIIdentityProviderIDEnv, "idp_123"}, f.calls[1]) + assert.Equal(t, [4]string{"acme", "widget", openAIServiceAccountIDEnv, "sa_456"}, f.calls[2]) +} + +func TestRunImportVariables_PartialWriteIsReported(t *testing.T) { + f := &fakeVariableSetter{failOn: 2, failWith: fmt.Errorf("403 forbidden")} + stubVariableSetter(t, f) + + ids := config.OpenAIWIFConfig{ + Audience: "fullsend://acme", + IdentityProviderID: "idp_123", + ServiceAccountID: "sa_456", + } + var buf bytes.Buffer + printer := newTestPrinter(&buf) - require.Len(t, calls, 3) - assert.Equal(t, []string{"variable", "set", "FULLSEND_OPENAI_AUDIENCE", "--repo", "acme/widget", "--body", "fullsend://acme"}, calls[0]) - assert.Equal(t, []string{"variable", "set", "FULLSEND_OPENAI_IDENTITY_PROVIDER_ID", "--repo", "acme/widget", "--body", "idp_123"}, calls[1]) - assert.Equal(t, []string{"variable", "set", "FULLSEND_OPENAI_SERVICE_ACCOUNT_ID", "--repo", "acme/widget", "--body", "sa_456"}, calls[2]) + err := runImportVariables(context.Background(), printer, ids, "acme/widget") + require.Error(t, err) + assert.Contains(t, err.Error(), openAIIdentityProviderIDEnv) + assert.Contains(t, buf.String(), "partial trio", + "the operator must be told the repository is now in a state a run refuses") + assert.Contains(t, buf.String(), openAIAudienceEnv, "and which variable did land") + assert.Len(t, f.calls, 2, "the third write is not attempted after a failure") } // --- status command tests --- @@ -601,7 +640,10 @@ func TestRunInferenceOpenAIStatus_NoConfig(t *testing.T) { cmd.SetArgs([]string{"inference", "openai", "status", "acme/widget", "--fullsend-dir", fullsendDir}) err := cmd.Execute() - require.NoError(t, err) + // An unconfigured repository is a failure, like the GCP status + // command's unhealthy mapping — it has to be able to gate a check. + require.Error(t, err) + assert.Contains(t, err.Error(), "no OpenAI WIF identifiers configured") assert.Contains(t, buf.String(), "No OpenAI WIF identifiers configured") } @@ -631,7 +673,8 @@ func TestRunInferenceOpenAIStatus_PartialConfig(t *testing.T) { cmd.SetArgs([]string{"inference", "openai", "status", "acme/widget", "--fullsend-dir", fullsendDir}) err := cmd.Execute() - require.NoError(t, err) + require.Error(t, err, "a partial trio is a state the run path refuses") + assert.Contains(t, err.Error(), "partially configured") assert.Contains(t, buf.String(), "Partial trio") } @@ -795,7 +838,11 @@ func TestInferenceOpenAIStatusCmd_DoesNotRequireGitHubToken(t *testing.T) { cmd.SetArgs([]string{"inference", "openai", "status", "acme/widget", "--fullsend-dir", filepath.Join(dir, ".fullsend")}) err := cmd.Execute() - require.NoError(t, err) // should not fail with "no GitHub token found" + // It reports an unconfigured repository, but never a missing GitHub + // token: status needs no forge credentials. + require.Error(t, err) + assert.Contains(t, err.Error(), "no OpenAI WIF identifiers configured") + assert.NotContains(t, err.Error(), "token") } // --- request JSON round-trip test (golden) --- @@ -1255,3 +1302,66 @@ func TestParseRepoList_DedupesCaseInsensitively(t *testing.T) { // The same rule keeps the mixed-owner guard honest. assert.Equal(t, []string{"acme"}, repoOwners([]string{"acme/widget", "Acme/gadget"})) } + +func TestImport_AudienceFromProviderBlockWhenReplyLeavesItDefault(t *testing.T) { + // An administrator who reuses an existing provider is told to put its + // audience in the provider block; import must honour that rather than + // recording the audience we proposed. + doc := buildRequestDoc([]string{"acme/widget"}, "fullsend://acme", "proj-1", "", openAIDefaultRef) + doc.Reply.IdentityProviderID = "idp_live" + doc.Reply.Audience = "" + doc.Reply.ServiceAccountIDs["acme/widget"] = "sa_live" + doc.Provider.Audience = "corp-existing-audience" + + b, err := json.MarshalIndent(doc, "", " ") + require.NoError(t, err) + path := filepath.Join(t.TempDir(), "reply.json") + require.NoError(t, os.WriteFile(path, b, 0o644)) + + ids, err := resolveImportIDs([]string{path}, "", "", "", "") + require.NoError(t, err) + assert.Equal(t, "corp-existing-audience", ids.Audience) +} + +func TestRunInferenceOpenAIStatus_PartialTrioExitsNonZero(t *testing.T) { + // The GCP status command fails on an unhealthy mapping; this one must + // too, or a broken enrolment cannot gate anything. + dir := t.TempDir() + fullsendDir := filepath.Join(dir, ".fullsend") + require.NoError(t, os.MkdirAll(fullsendDir, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(fullsendDir, "config.yaml"), []byte(`inference: + openai: + audience: fullsend://acme +`), 0o644)) + + t.Setenv(openAIAudienceEnv, "") + t.Setenv(openAIIdentityProviderIDEnv, "") + t.Setenv(openAIServiceAccountIDEnv, "") + t.Setenv(openAIStaticKeyEnv, "") + t.Setenv("ACTIONS_ID_TOKEN_REQUEST_URL", "") + t.Setenv("ACTIONS_ID_TOKEN_REQUEST_TOKEN", "") + t.Setenv("GITHUB_REPOSITORY", "") + + var buf bytes.Buffer + cmd := newRootCmd() + cmd.SetOut(&buf) + cmd.SetErr(&buf) + cmd.SetArgs([]string{"inference", "openai", "status", "acme/widget", "--fullsend-dir", fullsendDir}) + err := cmd.Execute() + require.Error(t, err) + assert.Contains(t, err.Error(), "partially configured") +} + +func TestServiceAccountFor_SeveralNamedRepositoriesAlwaysNeedASelector(t *testing.T) { + reply := openAIReplyDoc{ServiceAccountIDs: map[string]string{ + "acme/widget": "sa_widget", + "acme/gadget": "", // not filled in yet + }} + _, err := reply.serviceAccountFor("") + require.Error(t, err, "picking the only filled entry would misattribute once the other is filled") + assert.Contains(t, err.Error(), "--repo") + + sa, err := reply.serviceAccountFor("acme/widget") + require.NoError(t, err) + assert.Equal(t, "sa_widget", sa) +}