From e565541cd5e43487657e05e7fe05a9a89ed3af75 Mon Sep 17 00:00:00 2001 From: barckcode Date: Wed, 14 Oct 2026 15:53:36 +0200 Subject: [PATCH 1/7] feat: write API commands for secrets and automations (WIP, tests next) Co-Authored-By: Claude Opus 5.5 --- cmd/automations.go | 57 +- cmd/automations_write.go | 508 +++++++++++++ cmd/exit.go | 4 +- cmd/secrets.go | 666 ++++++++++++++++-- cmd/writeapi.go | 289 ++++++++ go.mod | 1 + go.sum | 1 + internal/api/automations.go | 12 +- internal/api/runs.go | 21 +- internal/api/secrets.go | 9 +- .../api/testdata/phase2/automation_get.json | 4 + .../testdata/phase2/automation_triggers.json | 3 +- .../api/testdata/phase2/automations_list.json | 4 + .../api/testdata/phase2/trigger_create.json | 34 + .../api/testdata/phase2/trigger_rotate.json | 3 + internal/api/write.go | 190 +++++ 16 files changed, 1681 insertions(+), 125 deletions(-) create mode 100644 cmd/automations_write.go create mode 100644 cmd/writeapi.go create mode 100644 internal/api/testdata/phase2/trigger_create.json create mode 100644 internal/api/testdata/phase2/trigger_rotate.json create mode 100644 internal/api/write.go diff --git a/cmd/automations.go b/cmd/automations.go index bd26006..ec2f9ed 100644 --- a/cmd/automations.go +++ b/cmd/automations.go @@ -33,51 +33,46 @@ var ( autoDeliveryJSON bool ) -// portalOnlyAutomationVerbs are the writes a member may reach for. Each one -// answers with where to do it instead of "unknown command". -var portalOnlyAutomationVerbs = map[string]bool{ - "create": true, "new": true, "add": true, "edit": true, "update": true, "set": true, - "rm": true, "delete": true, "remove": true, "enable": true, "disable": true, - "triggers": true, "trigger": true, "rotate": true, +// automationVerbHints answer the verbs a member may reach for that are not +// commands, with the one that is. +var automationVerbHints = map[string]string{ + "new": "create", "add": "create", "update": "edit", "set": "edit", + "rotate": "triggers rotate", } -const automationsPortalOnly = "automations are created and edited in the portal: " + runs.AutomationsURL + " (new: " + runs.NewAutomationURL + ")" + - ". An automation decides which of your secrets an agent receives and which of its actions need your approval, " + - "so writing one needs you signed in through the browser. From here you can list, inspect and run them " + - "(nan automations --help)" - var automationsCmd = &cobra.Command{ Use: "automations", Short: "List, inspect and run your automations", - Long: `List, inspect and run the automations you set up in the portal, and read -their webhook delivery log. + Long: `Create, change, list, inspect and run your automations, manage their +triggers (webhooks, schedules), and read their webhook delivery log. An automation is a saved agent task: a workspace, an agent, instructions, the secrets it receives, and its triggers (webhooks, schedules). Its runs can propose actions on your repository (a push, a pull request, a comment) that apply automatically or wait for your approval, as you configured. -Portal-only, and why: creating and editing automations, writing secrets and -approving proposals need you signed in through the browser at -` + runs.PortalURL + `. Each of them can hand a secret to an agent -or let an agent act without asking, so none can be done with a credential an -agent or a CI job could read: this machine's CLI session, a platform token or -an API key. +Approving a proposal stays portal-only, signed in through the browser at +` + runs.PortalURL + `: a credential an agent or a script can hold +must never approve what an agent proposed. + +Authentication of ls, show, run and deliveries is the same as nan run (see +nan run --help): a platform token needs the automations:read scope to list +and inspect, and automations:run to start a run. Workspace names are looked +up with workspaces:read; without it the workspace's id is shown instead. -Authentication is the same as nan run (see nan run --help). A platform token -needs the automations:read scope to list and inspect, and automations:run to -start a run. Workspace names are looked up with workspaces:read; without it -the workspace's id is shown instead. +Creating, changing and deleting automations and triggers: +` + writeCredentialHelp + ` Docs: https://nan.builders/docs/runs`, Annotations: sysexits, Args: cobra.ArbitraryArgs, RunE: func(cmd *cobra.Command, args []string) error { - if len(args) > 0 && portalOnlyAutomationVerbs[args[0]] { - return usageErrorf("%s", automationsPortalOnly) - } if len(args) > 0 { - return usageErrorf("unknown command %q for \"nan automations\"; try: nan automations --help", runs.Truncate(runs.SanitizeLine(args[0]), 64)) + shown := runs.Truncate(runs.SanitizeLine(args[0]), 64) + if hint, ok := automationVerbHints[args[0]]; ok { + return usageErrorf("there is no nan automations %s; try: nan automations %s", shown, hint) + } + return usageErrorf("unknown command %q for \"nan automations\"; try: nan automations --help", shown) } return cmd.Help() }, @@ -112,7 +107,7 @@ approval settings, and stream what it does. --input passes a JSON object to the run: @FILE reads it from a file, - from stdin, or give the JSON itself. The agent receives it as untrusted data next to the automation's instructions, which it never replaces. Up to 48 KiB. -Do not put secrets in it: the agent reads it; assign secrets in the portal. +Do not put secrets in it: the agent reads it; assign secrets to the automation\ninstead (nan automations edit, or the portal). When the run proposes actions that need your approval, it stops in awaiting_approval and this command exits 10: read the proposal with nan runs @@ -244,7 +239,7 @@ func doAutomationsLs(ctx context.Context, env *runsEnv) error { return printJSON(env.stdout, list, true) } if len(list.Data) == 0 { - fmt.Fprintf(env.stderr, "No automations yet. Create one in the portal: %s\n", runs.NewAutomationURL) + fmt.Fprintf(env.stderr, "No automations yet. Create one with nan automations create -f FILE, or in the portal: %s\n", runs.NewAutomationURL) return nil } for i := range list.Data { @@ -355,6 +350,7 @@ func doAutomationsShow(ctx context.Context, env *runsEnv, ref string) error { } } row("updated", localTime(a.UpdatedAt)) + row("last changed from", updatedByLabel(a.UpdatedBy)) if err := tw.Flush(); err != nil { return err } @@ -368,7 +364,8 @@ func doAutomationsShow(ctx context.Context, env *runsEnv, ref string) error { } printBlock(env, "instructions", a.Instructions) - fmt.Fprintf(env.stdout, "\nrun it: nan automations run %s\nedit it in the portal: %s\n", runs.SanitizeLine(a.ID), runs.AutomationURL(a.ID)) + fmt.Fprintf(env.stdout, "\nrun it: nan automations run %s\nchange it: nan automations edit %s -f FILE, or in the portal: %s\n", + runs.SanitizeLine(a.ID), runs.SanitizeLine(a.ID), runs.AutomationURL(a.ID)) return nil } diff --git a/cmd/automations_write.go b/cmd/automations_write.go new file mode 100644 index 0000000..b8a8882 --- /dev/null +++ b/cmd/automations_write.go @@ -0,0 +1,508 @@ +package cmd + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "text/tabwriter" + + "github.com/nxssie/nan-cli/internal/api" + "github.com/nxssie/nan-cli/internal/runs" + "github.com/spf13/cobra" +) + +// Automation writes from the CLI (write API): create, edit, delete, +// enable/disable, and the triggers of an automation. Bodies are the +// member's own YAML or JSON file, sent as written: the platform validates +// every field, exactly as for the portal. + +var ( + autoWriteFile string + autoWriteJSON bool + autoRmYes bool + trigLsJSON bool + trigRmYes bool + trigRotateURL bool + trigRotateSec bool + trigRotateHard bool +) + +const automationFileHelp = `The file is YAML or JSON (- reads it from stdin) with the fields of the +API's automation object, for example: + + name: PR review api + workspace: my-workspace-id + agent: pi + model: glm5.3 + template: pr_review + github_repo: acme/api + instructions: Review the pull request for correctness. + secrets: + - secret: GITHUB_TOKEN + env: GITHUB_TOKEN + exposure: actions + approval_kinds: [github_pr_review] + +The full list of fields is in the API reference: https://nan.builders/docs/runs` + +var automationsCreateCmd = &cobra.Command{ + Use: "create -f FILE", + Short: "Create an automation from a YAML or JSON file", + Long: `Create an automation from a YAML or JSON file. + +` + automationFileHelp + ` + +An automation receives the secrets you assign it, and approval_kinds: [] +lets every action it proposes apply without asking you. Then add a trigger +with nan automations triggers add. + +` + writeCredentialHelp + ` + +Docs: ` + writeDocs, + Example: ` nan automations create -f pr-review.yaml + nan automations create -f - < pr-review.json`, + Args: cobra.NoArgs, + Annotations: sysexits, + RunE: func(cmd *cobra.Command, args []string) error { + body, err := readBodyFile(autoWriteFile) + if err != nil { + return err + } + return withWriteEnv(func(env *runsEnv) error { return doAutomationsCreate(cmd.Context(), env, body) }) + }, +} + +var automationsEditCmd = &cobra.Command{ + Use: "edit -f FILE", + Short: "Change an automation: only the fields in the file change", + Long: `Change an automation. The file (YAML or JSON, - for stdin) holds only the +fields to change; the others keep their value. For example, to stop asking +before pull request reviews are posted: + + approval_kinds: [] + +` + writeCredentialHelp + ` + +Docs: ` + writeDocs, + Args: oneAutomation, + Annotations: sysexits, + RunE: func(cmd *cobra.Command, args []string) error { + body, err := readBodyFile(autoWriteFile) + if err != nil { + return err + } + return withWriteEnv(func(env *runsEnv) error { return doAutomationsUpdate(cmd.Context(), env, args[0], body, "updated") }) + }, +} + +var automationsEnableCmd = &cobra.Command{ + Use: "enable ", + Short: "Enable an automation: its triggers start runs again", + Args: oneAutomation, + Annotations: sysexits, + RunE: func(cmd *cobra.Command, args []string) error { + return withWriteEnv(func(env *runsEnv) error { + return doAutomationsUpdate(cmd.Context(), env, args[0], json.RawMessage(`{"enabled":true}`), "enabled") + }) + }, +} + +var automationsDisableCmd = &cobra.Command{ + Use: "disable ", + Short: "Disable an automation: its triggers stop starting runs", + Args: oneAutomation, + Annotations: sysexits, + RunE: func(cmd *cobra.Command, args []string) error { + return withWriteEnv(func(env *runsEnv) error { + return doAutomationsUpdate(cmd.Context(), env, args[0], json.RawMessage(`{"enabled":false}`), "disabled") + }) + }, +} + +var automationsRmCmd = &cobra.Command{ + Use: "rm [--yes]", + Aliases: []string{"delete", "remove"}, + Short: "Delete an automation and its triggers", + Long: `Delete an automation and its triggers: its webhook URLs stop working at once. +Its past runs stay in nan runs ls. + +Asks before deleting on a terminal; elsewhere, confirm with --yes. + +` + writeCredentialHelp + ` + +Docs: ` + writeDocs, + Args: oneAutomation, + Annotations: sysexits, + RunE: func(cmd *cobra.Command, args []string) error { + return withWriteEnv(func(env *runsEnv) error { return doAutomationsRm(cmd.Context(), env, args[0]) }) + }, +} + +const oneTimeNote = "Store these now: they are never shown again. Lost them? Issue new ones with: nan automations triggers rotate" + +var automationsTriggersCmd = &cobra.Command{ + Use: "triggers", + Aliases: []string{"trigger"}, + Short: "List, add, change, rotate and delete an automation's triggers", + Long: `List, add, change, rotate and delete the triggers of an automation: webhooks +and schedules. + +A webhook's URL and signing secret are shown once, when the trigger is added +or rotated, and never again: store them in the sender (GitHub, your CI) +right away. They go to stdout only, never to a file this CLI writes. + +` + writeCredentialHelp + ` + +Docs: https://nan.builders/docs/webhooks`, + Annotations: sysexits, + Args: cobra.ArbitraryArgs, + RunE: func(cmd *cobra.Command, args []string) error { + if len(args) > 0 { + return usageErrorf("unknown command %q for \"nan automations triggers\"; try: nan automations triggers --help", runs.Truncate(runs.SanitizeLine(args[0]), 64)) + } + return cmd.Help() + }, +} + +var triggersLsCmd = &cobra.Command{ + Use: "ls ", + Short: "List an automation's triggers", + Args: oneAutomation, + Annotations: sysexits, + RunE: func(cmd *cobra.Command, args []string) error { + return withWriteEnv(func(env *runsEnv) error { return doTriggersLs(cmd.Context(), env, args[0]) }) + }, +} + +var triggersAddCmd = &cobra.Command{ + Use: "add -f FILE", + Short: "Add a webhook or a schedule; a webhook's URL and secret are shown once", + Long: `Add a trigger from a YAML or JSON file (- for stdin). A GitHub webhook: + + kind: webhook + signature_scheme: github + event_filter: + events: [pull_request] + +A schedule: + + kind: schedule + cron: "0 7 * * 1-5" + timezone: Europe/Madrid + +For a webhook, the URL and the signing secret are printed once, to stdout. +` + oneTimeNote + `. +Adding is not retried: if the answer is lost, list the triggers and rotate +or delete the one you do not have the secret of. + +Docs: https://nan.builders/docs/webhooks`, + Args: oneAutomation, + Annotations: sysexits, + RunE: func(cmd *cobra.Command, args []string) error { + body, err := readBodyFile(autoWriteFile) + if err != nil { + return err + } + return withWriteEnv(func(env *runsEnv) error { return doTriggersAdd(cmd.Context(), env, args[0], body) }) + }, +} + +var triggersEditCmd = &cobra.Command{ + Use: "edit -f FILE", + Short: "Change a trigger: only the fields in the file change", + Long: `Change a trigger. The file (YAML or JSON, - for stdin) holds only the fields +to change, for example: + + enabled: false + +Docs: https://nan.builders/docs/webhooks`, + Args: automationAndTrigger, + Annotations: sysexits, + RunE: func(cmd *cobra.Command, args []string) error { + body, err := readBodyFile(autoWriteFile) + if err != nil { + return err + } + return withWriteEnv(func(env *runsEnv) error { return doTriggersEdit(cmd.Context(), env, args[0], args[1], body) }) + }, +} + +var triggersRotateCmd = &cobra.Command{ + Use: "rotate [--url] [--secret] [--no-grace]", + Short: "Issue a new webhook URL and/or signing secret, shown once", + Long: `Issue a new webhook URL, a new signing secret, or both (the default). The new +values are printed once, to stdout. ` + oneTimeNote + `. + +With --secret, the previous signing secret keeps working for 24 hours so +the sender can be updated; --no-grace ends it at once (for a leaked secret). +A new URL replaces the old one at once. + +Docs: https://nan.builders/docs/webhooks`, + Args: automationAndTrigger, + Annotations: sysexits, + RunE: func(cmd *cobra.Command, args []string) error { + in := api.RotateRequest{Grace: !trigRotateHard} + if trigRotateURL || !trigRotateSec { + in.Rotate = append(in.Rotate, "url") + } + if trigRotateSec || !trigRotateURL { + in.Rotate = append(in.Rotate, "secret") + } + return withWriteEnv(func(env *runsEnv) error { return doTriggersRotate(cmd.Context(), env, args[0], args[1], in) }) + }, +} + +var triggersRmCmd = &cobra.Command{ + Use: "rm [--yes]", + Aliases: []string{"delete", "remove"}, + Short: "Delete a trigger: its webhook URL stops working at once", + Args: automationAndTrigger, + Annotations: sysexits, + RunE: func(cmd *cobra.Command, args []string) error { + return withWriteEnv(func(env *runsEnv) error { return doTriggersRm(cmd.Context(), env, args[0], args[1]) }) + }, +} + +func init() { + automationsCmd.AddCommand(automationsCreateCmd, automationsEditCmd, automationsEnableCmd, automationsDisableCmd, + automationsRmCmd, automationsTriggersCmd) + automationsTriggersCmd.AddCommand(triggersLsCmd, triggersAddCmd, triggersEditCmd, triggersRotateCmd, triggersRmCmd) + + for _, c := range []*cobra.Command{automationsCreateCmd, automationsEditCmd, triggersAddCmd, triggersEditCmd} { + c.Flags().StringVarP(&autoWriteFile, "file", "f", "", "YAML or JSON `FILE` (- for stdin)") + } + for _, c := range []*cobra.Command{automationsCreateCmd, automationsEditCmd, automationsEnableCmd, automationsDisableCmd, + triggersAddCmd, triggersEditCmd, triggersRotateCmd} { + c.Flags().BoolVar(&autoWriteJSON, "json", false, "Print the API's answer as JSON") + } + automationsRmCmd.Flags().BoolVar(&autoRmYes, "yes", false, "Delete without asking") + triggersRmCmd.Flags().BoolVar(&trigRmYes, "yes", false, "Delete without asking") + triggersLsCmd.Flags().BoolVar(&trigLsJSON, "json", false, "Print the triggers as JSON") + triggersRotateCmd.Flags().BoolVar(&trigRotateURL, "url", false, "Rotate the webhook URL") + triggersRotateCmd.Flags().BoolVar(&trigRotateSec, "secret", false, "Rotate the signing secret") + triggersRotateCmd.Flags().BoolVar(&trigRotateHard, "no-grace", false, "End the previous signing secret at once (a leaked secret)") +} + +func automationAndTrigger(cmd *cobra.Command, args []string) error { + switch { + case len(args) < 2: + return usageErrorf("give the automation (id or name) and the trigger id; see: nan automations triggers ls ") + case len(args) > 2: + return usageErrorf("one automation and one trigger id (quote a name with spaces)") + case !runIDPattern.MatchString(args[1]): + return usageErrorf("%q is not a trigger id; see: nan automations triggers ls ", runs.Truncate(runs.SanitizeLine(args[1]), 64)) + } + return oneAutomation(cmd, args[:1]) +} + +// automationWriteExit is writeExit with a 404 that names the automation. +func automationWriteExit(err error, ref string, cred credential) *ExitError { + var apiErr *api.APIError + if errors.As(err, &apiErr) && apiErr.Status == 404 { + return automationExit(err, ref, cred) + } + return writeExit(err, cred, "automations", scopeAutomationsWrite) +} + +// resolveForWrite is resolveAutomation after checking the write API is on. +func resolveForWrite(ctx context.Context, env *runsEnv, ref string) (string, error) { + if err := requireWriteAPI(ctx, env, "automations"); err != nil { + return "", err + } + return resolveAutomation(ctx, env, ref) +} + +func doAutomationsCreate(ctx context.Context, env *runsEnv, body json.RawMessage) error { + if err := requireWriteAPI(ctx, env, "automations"); err != nil { + return err + } + a, raw, err := env.client.CreateAutomation(ctx, body) + if err != nil { + return writeExit(err, env.cred, "automations", scopeAutomationsWrite) + } + if autoWriteJSON { + return printJSON(env.stdout, raw, true) + } + fmt.Fprintf(env.stdout, "automation %s created: %s\n", runs.SanitizeLine(a.Name), runs.SanitizeLine(a.ID)) + warnAutomatic(env, a) + fmt.Fprintf(env.stderr, "add a trigger: nan automations triggers add %s -f trigger.yaml\nrun it now: nan automations run %s\n", + runs.SanitizeLine(a.ID), runs.SanitizeLine(a.ID)) + return nil +} + +// warnAutomatic says when nothing an automation proposes waits for the +// member: pushes and pull requests apply on their own. +func warnAutomatic(env *runsEnv, a *api.Automation) { + if len(a.ApprovalKinds) == 0 { + fmt.Fprintln(env.stderr, "note: approval_kinds is empty, so every action its runs propose applies without asking you") + } +} + +func doAutomationsUpdate(ctx context.Context, env *runsEnv, ref string, body json.RawMessage, verb string) error { + id, err := resolveForWrite(ctx, env, ref) + if err != nil { + return err + } + a, raw, err := env.client.UpdateAutomation(ctx, id, body) + if err != nil { + return automationWriteExit(err, ref, env.cred) + } + if autoWriteJSON { + return printJSON(env.stdout, raw, true) + } + fmt.Fprintf(env.stdout, "automation %s %s (%s)\n", runs.SanitizeLine(a.Name), verb, automationStatus(*a)) + if verb == "updated" { + warnAutomatic(env, a) + } + return nil +} + +func doAutomationsRm(ctx context.Context, env *runsEnv, ref string) error { + id, err := resolveForWrite(ctx, env, ref) + if err != nil { + return err + } + shown := runs.Truncate(runs.SanitizeLine(ref), 64) + if err := confirm(env, autoRmYes, "delete automation "+shown+" and its triggers?"); err != nil { + return err + } + if err := env.client.DeleteAutomation(ctx, id); err != nil { + return automationWriteExit(err, ref, env.cred) + } + fmt.Fprintf(env.stdout, "automation %s deleted\n", shown) + return nil +} + +func doTriggersLs(ctx context.Context, env *runsEnv, ref string) error { + id, err := resolveAutomation(ctx, env, ref) + if err != nil { + return err + } + list, err := env.client.ListTriggers(ctx, id) + if err != nil { + return automationExit(err, ref, env.cred) + } + if trigLsJSON { + return printJSON(env.stdout, list, true) + } + if len(list.Data) == 0 { + fmt.Fprintf(env.stderr, "No triggers. Add one with: nan automations triggers add %s -f trigger.yaml\n", runs.SanitizeLine(id)) + return nil + } + tw := tabwriter.NewWriter(env.stdout, 0, 0, 2, ' ', 0) + fmt.Fprintln(tw, "ID\tTRIGGER\tCHANGED FROM") + for _, t := range list.Data { + fmt.Fprintf(tw, "%s\t%s\t%s\n", runs.SanitizeLine(t.ID), triggerLine(t), dashIfEmpty(updatedByLabel(t.UpdatedBy))) + } + return tw.Flush() +} + +func doTriggersAdd(ctx context.Context, env *runsEnv, ref string, body json.RawMessage) error { + id, err := resolveForWrite(ctx, env, ref) + if err != nil { + return err + } + out, err := env.client.CreateTrigger(ctx, id, body) + if err != nil { + return automationWriteExit(err, ref, env.cred) + } + if autoWriteJSON { + if err := printJSON(env.stdout, out, true); err != nil { + return err + } + if out.WebhookURL != nil || out.SigningSecret != nil { + fmt.Fprintln(env.stderr, oneTimeNote) + } + return nil + } + if out.Trigger != nil { + fmt.Fprintf(env.stderr, "trigger %s added: %s\n", runs.SanitizeLine(out.Trigger.ID), triggerLine(*out.Trigger)) + } + printTriggerSecrets(env, out) + return nil +} + +// printTriggerSecrets prints the one-time webhook URL and signing secret to +// stdout, and the note about them to stderr. +func printTriggerSecrets(env *runsEnv, s *api.TriggerSecrets) { + if s.WebhookURL == nil && s.SigningSecret == nil { + return + } + if s.WebhookURL != nil { + fmt.Fprintf(env.stdout, "webhook URL: %s\n", runs.SanitizeLine(*s.WebhookURL)) + } + if s.SigningSecret != nil { + fmt.Fprintf(env.stdout, "signing secret: %s\n", runs.SanitizeLine(*s.SigningSecret)) + } + fmt.Fprintln(env.stderr, oneTimeNote) +} + +func doTriggersEdit(ctx context.Context, env *runsEnv, ref, triggerID string, body json.RawMessage) error { + id, err := resolveForWrite(ctx, env, ref) + if err != nil { + return err + } + t, err := env.client.UpdateTrigger(ctx, id, triggerID, body) + if err != nil { + return triggerWriteExit(err, ref, triggerID, env.cred) + } + if autoWriteJSON { + return printJSON(env.stdout, t, true) + } + fmt.Fprintf(env.stdout, "trigger %s updated: %s\n", runs.SanitizeLine(t.ID), triggerLine(*t)) + return nil +} + +func doTriggersRotate(ctx context.Context, env *runsEnv, ref, triggerID string, in api.RotateRequest) error { + id, err := resolveForWrite(ctx, env, ref) + if err != nil { + return err + } + out, err := env.client.RotateTrigger(ctx, id, triggerID, in) + if err != nil { + return triggerWriteExit(err, ref, triggerID, env.cred) + } + if autoWriteJSON { + if err := printJSON(env.stdout, out, true); err != nil { + return err + } + fmt.Fprintln(env.stderr, oneTimeNote) + return nil + } + printTriggerSecrets(env, out) + if out.SigningSecret != nil { + if in.Grace { + fmt.Fprintln(env.stderr, "the previous signing secret keeps working for 24 hours") + } else { + fmt.Fprintln(env.stderr, "the previous signing secret no longer works") + } + } + return nil +} + +func doTriggersRm(ctx context.Context, env *runsEnv, ref, triggerID string) error { + id, err := resolveForWrite(ctx, env, ref) + if err != nil { + return err + } + if err := confirm(env, trigRmYes, "delete trigger "+triggerID+"? A webhook URL stops working at once"); err != nil { + return err + } + if err := env.client.DeleteTrigger(ctx, id, triggerID); err != nil { + return triggerWriteExit(err, ref, triggerID, env.cred) + } + fmt.Fprintf(env.stdout, "trigger %s deleted\n", triggerID) + return nil +} + +// triggerWriteExit names the trigger on a 404: the automation was found a +// moment ago, so it is the trigger that is missing. +func triggerWriteExit(err error, ref, triggerID string, cred credential) *ExitError { + var apiErr *api.APIError + if errors.As(err, &apiErr) && apiErr.Status == 404 { + return usageErrorf("trigger %s not found on automation %q; see: nan automations triggers ls %q", + triggerID, runs.Truncate(runs.SanitizeLine(ref), 64), runs.Truncate(runs.SanitizeLine(ref), 64)) + } + return writeExit(err, cred, "automations", scopeAutomationsWrite) +} diff --git a/cmd/exit.go b/cmd/exit.go index e194c9f..fe02077 100644 --- a/cmd/exit.go +++ b/cmd/exit.go @@ -130,8 +130,8 @@ func apiExit(err error, cred credential) *ExitError { // browserOnly explains the one class of refusal no credential of this CLI // can get past. var browserOnly = "this needs you signed in through the browser: do it in the portal at " + runs.PortalURL + - ". Approving a proposal, writing a secret and creating or editing an automation are portal-only, so that " + - "nothing that can read this machine's CLI session or tokens (an agent in a workspace, a CI job) can do them. " + + ". Approving a proposal is portal-only, so that nothing that can read this machine's CLI session or tokens " + + "(an agent in a workspace, a CI job) can approve what an agent proposed. " + "See https://nan.builders/docs/runs" // scopeHint is what to do about a platform token missing a scope. diff --git a/cmd/secrets.go b/cmd/secrets.go index 3286802..0516253 100644 --- a/cmd/secrets.go +++ b/cmd/secrets.go @@ -2,65 +2,88 @@ package cmd import ( "context" + "crypto/rand" + "encoding/hex" "errors" "fmt" + "io" "os" + "runtime" "strings" "text/tabwriter" + "unicode/utf8" "github.com/nxssie/nan-cli/internal/api" "github.com/nxssie/nan-cli/internal/runs" - "github.com/nxssie/nan-cli/internal/session" "github.com/spf13/cobra" ) -var secretsLsJSON bool +var ( + secretsLsJSON bool + secretsShowJSON bool -// secretsURL is where secrets are written: Settings > Secrets in the portal. + secretSetValueFile string + secretSetValueStdin bool + secretSetKind string + secretSetProvider string + secretSetBaseURL string + secretSetDescription string + secretSetJSON bool + + secretEditDescription string + secretEditBaseURL string + secretEditJSON bool + + secretRmYes bool +) + +// secretsURL is Settings > Secrets in the portal. const secretsURL = runs.PortalURL + "/settings" -// portalOnlySecretVerbs are the writes a member may reach for. Each one -// answers with where to do it instead of "unknown command". -var portalOnlySecretVerbs = map[string]bool{ - "set": true, "add": true, "create": true, "new": true, "put": true, "update": true, "edit": true, - "rotate": true, "replace": true, "rm": true, "delete": true, "remove": true, "get": true, "show": true, "reveal": true, -} +// maxSecretValue is the platform's limit on a secret's value. +const maxSecretValue = 16 * 1024 -const secretsPortalOnly = "secrets are written in the portal only: Settings > Secrets at " + secretsURL + - ". Writing or replacing a secret needs you signed in through the browser, so that nothing that can read this " + - "machine's CLI session or tokens (an agent in a workspace, a CI job) can swap one for its own. " + - "A secret's value is never shown again after it is saved, anywhere: nan secrets ls lists names and metadata" +// secretVerbHints answer the verbs a member may reach for that are not +// commands, with the one that is. +var secretVerbHints = map[string]string{ + "add": "set", "create": "set", "new": "set", "put": "set", "update": "set (the value) or nan secrets edit (description, base URL)", + "rotate": "set", "replace": "set", "delete": "rm", "remove": "rm", "get": "show", "reveal": "show", +} var secretsCmd = &cobra.Command{ Use: "secrets", - Short: "List your secrets (names and metadata only)", - Long: `List the secrets you keep for your automations: names, kinds, the last -characters as a hint, and which automations use each one. Values are never -shown, here or anywhere else, after they are saved. + Short: "Manage your secrets: values go in, only metadata comes out", + Long: `Manage the secrets you keep for your automations: list them, see one, add +one or replace its value, edit its description or base URL, delete it. +A secret's value is never shown again after it is saved, here or anywhere +else: you see its name, kind, last characters as a hint and which +automations use it. Kinds: generic (a value an automation hands to its runs) and provider_key (your own key for Anthropic, OpenAI, Google Gemini or an OpenAI-compatible endpoint, with its base URL). Run on a provider key with: nan run --byok-secret NAME --model ID "" -Portal-only, and why: adding, replacing and deleting secrets happen in the -portal, Settings > Secrets at ` + runs.PortalURL + `, signed in -through the browser. A secret written from a credential an agent or a CI job -could read (this machine's CLI session, a token) could be swapped for one -the attacker controls. +The value is read from a file or from stdin, never from the command line, +where it would stay in your shell history: + nan secrets set GITHUB_TOKEN --value-file ./token.txt + pbpaste | nan secrets set GITHUB_TOKEN --value-stdin + +` + writeCredentialHelp + ` -nan secrets ls needs your nan auth login session: platform tokens and API -keys cannot read secrets. +The portal does all of this too, Settings > Secrets at ` + runs.PortalURL + `, +signed in through the browser. -Docs: https://nan.builders/docs/runs`, +Docs: ` + writeDocs, Annotations: sysexits, Args: cobra.ArbitraryArgs, RunE: func(cmd *cobra.Command, args []string) error { - if len(args) > 0 && portalOnlySecretVerbs[args[0]] { - return usageErrorf("%s", secretsPortalOnly) - } if len(args) > 0 { - return usageErrorf("unknown command %q for \"nan secrets\"; try: nan secrets --help", runs.Truncate(runs.SanitizeLine(args[0]), 64)) + shown := runs.Truncate(runs.SanitizeLine(args[0]), 64) + if hint, ok := secretVerbHints[args[0]]; ok { + return usageErrorf("there is no nan secrets %s; try: nan secrets %s", shown, hint) + } + return usageErrorf("unknown command %q for \"nan secrets\"; try: nan secrets --help", shown) } return cmd.Help() }, @@ -72,50 +95,336 @@ var secretsLsCmd = &cobra.Command{ Args: cobra.NoArgs, Annotations: sysexits, RunE: func(cmd *cobra.Command, args []string) error { - env, err := newSessionEnv() + return withWriteEnv(func(env *runsEnv) error { return doSecretsLs(cmd.Context(), env) }) + }, +} + +var secretsShowCmd = &cobra.Command{ + Use: "show ", + Short: "Show one secret's metadata (never its value)", + Args: oneSecret, + Annotations: sysexits, + RunE: func(cmd *cobra.Command, args []string) error { + if err := checkSecretRef(args[0]); err != nil { + return err + } + return withWriteEnv(func(env *runsEnv) error { return doSecretsShow(cmd.Context(), env, args[0]) }) + }, +} + +var secretsSetCmd = &cobra.Command{ + Use: "set NAME (--value-file PATH | --value-stdin)", + Short: "Add a secret, or replace the value of one you have", + Long: `Add a secret, or replace the value of the one with that NAME. + +The value comes from --value-file PATH or from a pipe with --value-stdin, +never from the command line. One trailing newline is dropped (the one an +editor or echo adds); anything else is kept as it is. Up to 16 KiB. + +A new secret is generic unless you say otherwise. A provider key: + nan secrets set ANTHROPIC_KEY --kind provider_key --provider anthropic --value-file key.txt + nan secrets set LOCAL_LLM --provider openai_compatible --base-url https://llm.example.com/v1 --value-stdin +A secret's kind and provider are fixed when it is created: to change them, +delete it and add it again. + +NAME: capital letters, digits and _, starting with a letter (GITHUB_TOKEN). + +` + writeCredentialHelp + ` + +Docs: ` + writeDocs, + Example: ` nan secrets set GITHUB_TOKEN --value-file ./token.txt --description "fine-grained, repo acme/api" + op read op://dev/github/token | nan secrets set GITHUB_TOKEN --value-stdin`, + Args: oneSecretName, + Annotations: sysexits, + RunE: func(cmd *cobra.Command, args []string) error { + in, err := buildSecretSet(cmd, args[0]) if err != nil { return err } - return runtimeError(doSecretsLs(cmd.Context(), env)) + return withWriteEnv(func(env *runsEnv) error { return doSecretsSet(cmd.Context(), env, in) }) + }, +} + +var secretsEditCmd = &cobra.Command{ + Use: "edit [--description TEXT] [--base-url URL]", + Short: "Change a secret's description or base URL (not its value: see nan secrets set)", + Long: `Change a secret's description, or the base URL of an openai_compatible +provider key. To replace the value, use nan secrets set. + +` + writeCredentialHelp + ` + +Docs: ` + writeDocs, + Args: oneSecret, + Annotations: sysexits, + RunE: func(cmd *cobra.Command, args []string) error { + if err := checkSecretRef(args[0]); err != nil { + return err + } + var p api.SecretPatch + if cmd.Flags().Changed("description") { + p.Description = &secretEditDescription + } + if cmd.Flags().Changed("base-url") { + p.BaseURL = &secretEditBaseURL + } + if p.Description == nil && p.BaseURL == nil { + return usageErrorf("nothing to change: give --description, --base-url or both") + } + return withWriteEnv(func(env *runsEnv) error { return doSecretsEdit(cmd.Context(), env, args[0], p) }) + }, +} + +var secretsRmCmd = &cobra.Command{ + Use: "rm [--yes]", + Aliases: []string{"delete", "remove"}, + Short: "Delete a secret", + Long: `Delete a secret. A secret an automation uses cannot be deleted: remove it +from those automations first; the error names them. + +Asks before deleting on a terminal; elsewhere, confirm with --yes. + +` + writeCredentialHelp + ` + +Docs: ` + writeDocs, + Args: oneSecret, + Annotations: sysexits, + RunE: func(cmd *cobra.Command, args []string) error { + if err := checkSecretRef(args[0]); err != nil { + return err + } + return withWriteEnv(func(env *runsEnv) error { return doSecretsRm(cmd.Context(), env, args[0]) }) }, } func init() { rootCmd.AddCommand(secretsCmd) - secretsCmd.AddCommand(secretsLsCmd) + secretsCmd.AddCommand(secretsLsCmd, secretsShowCmd, secretsSetCmd, secretsEditCmd, secretsRmCmd) + secretsCmd.PersistentFlags().StringVar(&tokenFileFlag, "token-file", "", tokenFileUsage) + secretsLsCmd.Flags().BoolVar(&secretsLsJSON, "json", false, "Print the list as JSON (metadata only)") + secretsShowCmd.Flags().BoolVar(&secretsShowJSON, "json", false, "Print the secret's metadata as JSON") + + f := secretsSetCmd.Flags() + f.StringVar(&secretSetValueFile, "value-file", "", "Read the value from `PATH`") + f.BoolVar(&secretSetValueStdin, "value-stdin", false, "Read the value from stdin (a pipe, never a terminal)") + f.StringVar(&secretSetKind, "kind", "", "generic (default) or provider_key, for a new secret") + f.StringVar(&secretSetProvider, "provider", "", "anthropic, openai, google or openai_compatible (implies --kind provider_key)") + f.StringVar(&secretSetBaseURL, "base-url", "", "The endpoint of an openai_compatible provider key") + f.StringVar(&secretSetDescription, "description", "", "What the secret is for (never the value)") + f.BoolVar(&secretSetJSON, "json", false, "Print the secret's metadata as JSON") + + secretsEditCmd.Flags().StringVar(&secretEditDescription, "description", "", "The new description (\"\" clears it)") + secretsEditCmd.Flags().StringVar(&secretEditBaseURL, "base-url", "", "The new base URL of an openai_compatible provider key") + secretsEditCmd.Flags().BoolVar(&secretEditJSON, "json", false, "Print the secret's metadata as JSON") + + secretsRmCmd.Flags().BoolVar(&secretRmYes, "yes", false, "Delete without asking") } -// newSessionEnv is newRunsEnv for the commands only a signed-in session can -// use. NAN_TOKEN, --token-file and a saved token are skipped rather than -// sent: the platform refuses them there, and the saved session is the one -// credential that works. -func newSessionEnv() (*runsEnv, error) { - sess, err := session.Load() - if err != nil && !errors.Is(err, session.ErrNotLoggedIn) { - return nil, exitf(exitAuth, "could not read your session (%v): run nan auth login", err) +// oneSecret is cobra.ExactArgs(1) with a message that says what is missing. +func oneSecret(cmd *cobra.Command, args []string) error { + switch { + case len(args) == 0: + return usageErrorf("missing the secret's NAME or id; see: nan secrets ls") + case len(args) > 1: + return usageErrorf("one secret at a time; the value goes in --value-file or --value-stdin, never on the command line") } - if sess == nil || sess.Token == "" { - return nil, exitf(exitAuth, "nan secrets needs your session: run nan auth login (platform tokens and API keys cannot read secrets)") + return nil +} + +// oneSecretName is oneSecret for set: one NAME, and an extra argument is +// almost certainly the value, which is then in the shell history. +func oneSecretName(cmd *cobra.Command, args []string) error { + switch { + case len(args) == 0: + return usageErrorf("missing the secret's NAME, like GITHUB_TOKEN") + case len(args) > 1: + return usageErrorf("nan secrets set takes the NAME only: the value goes in --value-file or --value-stdin. " + + "If you typed the value on the command line, it is now in your shell history: revoke it where it was issued and set the new one from a file") } - cred := credential{kind: credSession, session: sess.Token} - return &runsEnv{client: cred.client(), cred: cred, stdout: os.Stdout, stderr: os.Stderr}, nil + return checkSecretName(args[0]) } -func doSecretsLs(ctx context.Context, env *runsEnv) error { - list, err := env.client.ListSecrets(ctx) +// pastedValueHint is what to do about a value typed on the command line. +const pastedValueHint = "If that was the value, it is now in your shell history: revoke it where it was issued, " + + "then save the new one with nan secrets set NAME --value-file PATH" + +// checkSecretName checks a NAME without ever repeating something that may +// be the value itself. +func checkSecretName(name string) error { + if secretNamePattern.MatchString(name) { + return nil + } + const what = "a secret NAME is capital letters, digits and _, starting with a letter (like GITHUB_TOKEN)" + switch upper := strings.ToUpper(name); { + case looksLikeAKey(name): + return usageErrorf("%s. %s", what, pastedValueHint) + case len(name) <= 24 && secretNamePattern.MatchString(upper): + return usageErrorf("%s. Did you mean %s?", what, upper) + } + return usageErrorf("%s", what) +} + +// checkSecretRef accepts a NAME or a secret id. +func checkSecretRef(ref string) error { + if runIDPattern.MatchString(ref) { + return nil + } + return checkSecretName(ref) +} + +// secretSet is what nan secrets set was asked to do. The value never +// appears in an error, a log line or the output. +type secretSet struct { + name string + value string + kind string + provider string + baseURL string + description *string +} + +func buildSecretSet(cmd *cobra.Command, name string) (*secretSet, error) { + in := &secretSet{name: name, kind: secretSetKind, provider: secretSetProvider, baseURL: secretSetBaseURL} + if cmd.Flags().Changed("description") { + d := secretSetDescription + in.description = &d + } + switch in.kind { + case "", api.SecretKindGeneric, api.SecretKindProviderKey: + default: + return nil, usageErrorf("--kind is generic or provider_key") + } + if in.provider != "" && in.kind == "" { + in.kind = api.SecretKindProviderKey + } + if in.provider != "" && in.kind != api.SecretKindProviderKey { + return nil, usageErrorf("--provider is for --kind provider_key") + } + if in.kind == api.SecretKindProviderKey && in.provider == "" { + return nil, usageErrorf("--kind provider_key needs --provider: anthropic, openai, google or openai_compatible") + } + value, err := readSecretValue(secretSetValueFile, secretSetValueStdin) if err != nil { - var apiErr *api.APIError - if errors.As(err, &apiErr) && apiErr.Status == 404 { - return exitf(exitAuth, "secrets are not enabled for your account") + return nil, err + } + in.value = value + return in, nil +} + +// readSecretValue reads the value from --value-file or a piped stdin, and +// drops one trailing newline. Errors never include the value. +func readSecretValue(path string, fromStdin bool) (string, error) { + switch { + case path != "" && fromStdin: + return "", usageErrorf("give --value-file or --value-stdin, not both") + case path == "" && !fromStdin: + return "", usageErrorf("the value comes from --value-file PATH or --value-stdin (a pipe), never from the command line") + } + var ( + raw []byte + err error + ) + if fromStdin { + if writeStdinTTY() { + return "", usageErrorf("--value-stdin reads a piped value, and stdin is a terminal, where typing it would show it on screen: " + + "pipe it (op read ... | nan secrets set NAME --value-stdin) or use --value-file PATH") + } + raw, err = io.ReadAll(io.LimitReader(writeStdin, maxSecretValue+3)) + if err != nil { + return "", usageErrorf("could not read the value from stdin") + } + } else { + shown := runs.SanitizeLine(path) + f, openErr := os.Open(path) + if openErr != nil { + return "", usageErrorf("could not read --value-file %s: %v", shown, unwrapPath(openErr)) + } + defer f.Close() + if info, statErr := f.Stat(); statErr == nil { + if info.IsDir() { + return "", usageErrorf("--value-file %s is a directory", shown) + } + if runtime.GOOS != "windows" && info.Mode().IsRegular() && info.Mode().Perm()&0o077 != 0 { + fmt.Fprintf(os.Stderr, "note: %s can be read by other users of this machine (chmod 600 %s)\n", shown, shown) + } + } + raw, err = io.ReadAll(io.LimitReader(f, maxSecretValue+3)) + if err != nil { + return "", usageErrorf("could not read --value-file %s: %v", shown, unwrapPath(err)) + } + } + value := string(raw) + if strings.HasSuffix(value, "\r\n") { + value = value[:len(value)-2] + } else if strings.HasSuffix(value, "\n") { + value = value[:len(value)-1] + } + switch { + case value == "": + return "", usageErrorf("the value is empty") + case len(value) > maxSecretValue: + return "", usageErrorf("the value is over %d KiB, the most a secret holds", maxSecretValue/1024) + case !utf8.ValidString(value): + return "", usageErrorf("the value is not UTF-8 text") + } + return value, nil +} + +// listSecrets lists the member's secrets: a session reads the portal's +// route (any session may, whatever the write API), a token /v1/secrets +// with secrets:read. +func listSecrets(ctx context.Context, env *runsEnv) (*api.SecretList, error) { + if env.cred.kind == credSession { + list, err := env.client.ListSecrets(ctx) + if err != nil { + var apiErr *api.APIError + if errors.As(err, &apiErr) && apiErr.Status == 404 { + return nil, exitf(exitAuth, "secrets are not enabled for your account") + } + return nil, apiExit(err, env.cred) + } + return list, nil + } + list, err := env.client.SecretsV1(ctx) + if err != nil { + return nil, secretsV1Exit(err, env.cred, scopeSecretsRead) + } + return list, nil +} + +// secretsV1Exit is writeExit for GET /v1/secrets, where a 404 means the +// platform has no /v1/secrets for the member (write API off). +func secretsV1Exit(err error, cred credential, scope string) *ExitError { + var apiErr *api.APIError + if errors.As(err, &apiErr) && apiErr.Status == 404 { + return writeAPIOff("secrets") + } + return writeExit(err, cred, "secrets", scope) +} + +func findSecret(list *api.SecretList, ref string) *api.Secret { + for i := range list.Data { + if list.Data[i].Name == ref || list.Data[i].ID == ref { + return &list.Data[i] } - return apiExit(err, env.cred) + } + return nil +} + +func secretNotFound(ref string) *ExitError { + return usageErrorf("no secret named %q; see: nan secrets ls", runs.Truncate(runs.SanitizeLine(ref), 64)) +} + +func doSecretsLs(ctx context.Context, env *runsEnv) error { + list, err := listSecrets(ctx, env) + if err != nil { + return err } if secretsLsJSON { return printJSON(env.stdout, list, true) } if len(list.Data) == 0 { - fmt.Fprintf(env.stderr, "No secrets yet. Add them in the portal: Settings > Secrets at %s\n", secretsURL) + fmt.Fprintln(env.stderr, "No secrets yet. Add one with: nan secrets set NAME --value-file PATH") return nil } // The base URL column only when there is one: it is set on @@ -127,47 +436,250 @@ func doSecretsLs(ctx context.Context, env *runsEnv) error { } } tw := tabwriter.NewWriter(env.stdout, 0, 0, 2, ' ', 0) - header := "NAME\tKIND\tPROVIDER\tHINT\tUSED BY\tLAST USED\tUPDATED" + header := "NAME\tKIND\tPROVIDER\tHINT\tUSED BY\tLAST USED\tUPDATED\tCHANGED FROM" if withURL { header += "\tBASE URL" } fmt.Fprintln(tw, header) for _, s := range list.Data { - hint := "-" - if s.Hint != nil && *s.Hint != "" { - hint = runs.Truncate(runs.SanitizeLine(*s.Hint), 12) - } - provider := "-" - if s.Provider != nil && *s.Provider != "" { - provider = runs.Truncate(runs.SanitizeLine(*s.Provider), 24) - } - used := make([]string, 0, len(s.UsedBy)) - for _, u := range s.UsedBy { - used = append(used, runs.SanitizeLine(u.Label())) - } - usedBy := "-" - if len(used) > 0 { - usedBy = runs.Truncate(strings.Join(used, ", "), 40) - } lastUsed := "never" if s.LastUsedAt != nil { lastUsed = localTime(*s.LastUsedAt) } - line := fmt.Sprintf("%s\t%s\t%s\t%s\t%s\t%s\t%s", + line := fmt.Sprintf("%s\t%s\t%s\t%s\t%s\t%s\t%s\t%s", runs.Truncate(runs.SanitizeLine(s.Name), 64), - runs.SanitizeLine(s.Kind), provider, hint, usedBy, lastUsed, localTime(s.UpdatedAt)) + runs.SanitizeLine(s.Kind), orDash(s.Provider, 24), orDash(s.Hint, 12), runs.Truncate(usedByList(s), 40), + lastUsed, localTime(s.UpdatedAt), dashIfEmpty(updatedByLabel(s.UpdatedBy))) if withURL { - baseURL := "-" - if s.BaseURL != nil && *s.BaseURL != "" { - baseURL = runs.Truncate(runs.SanitizeLine(*s.BaseURL), 60) - } - line += "\t" + baseURL + line += "\t" + orDash(s.BaseURL, 60) } fmt.Fprintln(tw, line) } - if err := tw.Flush(); err != nil { + return tw.Flush() +} + +func orDash(v *string, max int) string { + if v == nil || *v == "" { + return "-" + } + return runs.Truncate(runs.SanitizeLine(*v), max) +} + +func dashIfEmpty(v string) string { + if v == "" { + return "-" + } + return v +} + +func usedByList(s api.Secret) string { + used := make([]string, 0, len(s.UsedBy)) + for _, u := range s.UsedBy { + used = append(used, runs.SanitizeLine(u.Label())) + } + if len(used) == 0 { + return "-" + } + return strings.Join(used, ", ") +} + +func doSecretsShow(ctx context.Context, env *runsEnv, ref string) error { + list, err := listSecrets(ctx, env) + if err != nil { + return err + } + s := findSecret(list, ref) + if s == nil { + return secretNotFound(ref) + } + if secretsShowJSON { + return printJSON(env.stdout, s, true) + } + printSecret(env, s) + return nil +} + +func printSecret(env *runsEnv, s *api.Secret) { + tw := tabwriter.NewWriter(env.stdout, 0, 0, 2, ' ', 0) + row := func(k, v string) { + if v != "" && v != "-" { + fmt.Fprintf(tw, "%s\t%s\n", k, v) + } + } + row("name", runs.SanitizeLine(s.Name)) + row("id", runs.SanitizeLine(s.ID)) + row("kind", runs.SanitizeLine(s.Kind)) + row("provider", orDash(s.Provider, 40)) + row("base url", orDash(s.BaseURL, 200)) + row("hint", orDash(s.Hint, 12)) + if s.ValueBytes > 0 { + row("size", fmt.Sprintf("%d bytes", s.ValueBytes)) + } + row("description", runs.Truncate(runs.SanitizeLine(s.Description), 256)) + row("used by", usedByList(*s)) + row("last changed from", updatedByLabel(s.UpdatedBy)) + row("created", localTime(s.CreatedAt)) + row("updated", localTime(s.UpdatedAt)) + lastUsed := "never" + if s.LastUsedAt != nil { + lastUsed = localTime(*s.LastUsedAt) + } + row("last used", lastUsed) + _ = tw.Flush() +} + +// newIdempotencyKey is the Idempotency-Key of one nan secrets set: a retry +// of the same request after a lost answer returns the secret the first +// attempt created instead of 409 name_taken. +func newIdempotencyKey() string { + b := make([]byte, 16) + _, _ = rand.Read(b) + return "nan-cli-" + hex.EncodeToString(b) +} + +// createAttempts is how many times nan secrets set sends a create whose +// answer was lost on the way (never one the platform refused). +var createAttempts = 2 + +func doSecretsSet(ctx context.Context, env *runsEnv, in *secretSet) error { + if err := requireWriteAPI(ctx, env, "secrets"); err != nil { + return err + } + list, err := env.client.SecretsV1(ctx) + var existing *api.Secret + if err != nil { + var apiErr *api.APIError + // A token with secrets:write and not secrets:read cannot look the + // name up: it can still create. + if !(errors.As(err, &apiErr) && apiErr.Code == "token_scope_denied") { + return secretsV1Exit(err, env.cred, scopeSecretsRead) + } + } else { + existing = findSecret(list, in.name) + } + if existing == nil { + return createSecret(ctx, env, in, list != nil && err == nil) + } + return replaceSecret(ctx, env, in, existing) +} + +func createSecret(ctx context.Context, env *runsEnv, in *secretSet, looked bool) error { + req := api.CreateSecretRequest{Name: in.name, Value: in.value, Kind: in.kind, Provider: in.provider, BaseURL: in.baseURL, Description: in.description} + key := newIdempotencyKey() + var ( + s *api.Secret + replayed bool + err error + ) + for attempt := 0; attempt < createAttempts; attempt++ { + s, replayed, err = env.client.CreateSecret(ctx, req, key) + if err == nil || !lostAnswer(err) { + break + } + } + if err != nil { + var apiErr *api.APIError + if !looked && errors.As(err, &apiErr) && apiErr.Code == "name_taken" { + return usageErrorf("%s already exists, and %s cannot look it up to replace its value: that needs the %s scope too", + in.name, env.cred.describe(), scopeSecretsRead) + } + return writeExit(err, env.cred, "secrets", scopeSecretsWrite) + } + if secretSetJSON { + return printJSON(env.stdout, s, true) + } + verb := "created" + if replayed { + verb = "created (the platform had it from the first attempt)" + } + fmt.Fprintf(env.stdout, "secret %s %s: %s, %d bytes, hint %s\n", runs.SanitizeLine(s.Name), verb, + runs.SanitizeLine(s.Kind), s.ValueBytes, orDash(s.Hint, 12)) + return nil +} + +// lostAnswer is a request that may or may not have reached the platform: +// no answer came back. A refusal, a 5xx or an expired session is not one. +func lostAnswer(err error) bool { + var apiErr *api.APIError + return !errors.As(err, &apiErr) && !errors.Is(err, api.ErrSessionExpired) && !errors.Is(err, context.Canceled) +} + +func replaceSecret(ctx context.Context, env *runsEnv, in *secretSet, existing *api.Secret) error { + if in.kind != "" && in.kind != existing.Kind { + return usageErrorf("%s is a %s secret: the kind is fixed when a secret is created (delete it and add it again to change it)", + in.name, runs.SanitizeLine(existing.Kind)) + } + if in.provider != "" && (existing.Provider == nil || *existing.Provider != in.provider) { + return usageErrorf("%s has another provider: the provider is fixed when a secret is created (delete it and add it again to change it)", in.name) + } + s, err := env.client.ReplaceSecretValue(ctx, existing.ID, in.value) + if err != nil { + return writeExit(err, env.cred, "secrets", scopeSecretsWrite) + } + if in.description != nil || in.baseURL != "" { + p := api.SecretPatch{Description: in.description} + if in.baseURL != "" { + p.BaseURL = &in.baseURL + } + if s, err = env.client.UpdateSecret(ctx, existing.ID, p); err != nil { + fmt.Fprintln(env.stderr, "the value was replaced, but not the description or base URL:") + return writeExit(err, env.cred, "secrets", scopeSecretsWrite) + } + } + if secretSetJSON { + return printJSON(env.stdout, s, true) + } + fmt.Fprintf(env.stdout, "secret %s: value replaced, %d bytes, hint %s\n", runs.SanitizeLine(s.Name), s.ValueBytes, orDash(s.Hint, 12)) + if len(s.UsedBy) > 0 { + fmt.Fprintf(env.stderr, "the next runs of these automations get the new value: %s\n", usedByList(*s)) + } + return nil +} + +// resolveSecretForWrite finds a secret by NAME or id through /v1/secrets. +func resolveSecretForWrite(ctx context.Context, env *runsEnv, ref string) (*api.Secret, error) { + if err := requireWriteAPI(ctx, env, "secrets"); err != nil { + return nil, err + } + list, err := env.client.SecretsV1(ctx) + if err != nil { + return nil, secretsV1Exit(err, env.cred, scopeSecretsRead) + } + s := findSecret(list, ref) + if s == nil { + return nil, secretNotFound(ref) + } + return s, nil +} + +func doSecretsEdit(ctx context.Context, env *runsEnv, ref string, p api.SecretPatch) error { + existing, err := resolveSecretForWrite(ctx, env, ref) + if err != nil { return err } - fmt.Fprintf(env.stderr, "add, replace or delete secrets in the portal: Settings > Secrets at %s\n", secretsURL) + s, err := env.client.UpdateSecret(ctx, existing.ID, p) + if err != nil { + return writeExit(err, env.cred, "secrets", scopeSecretsWrite) + } + if secretEditJSON { + return printJSON(env.stdout, s, true) + } + fmt.Fprintf(env.stdout, "secret %s updated\n", runs.SanitizeLine(s.Name)) + return nil +} + +func doSecretsRm(ctx context.Context, env *runsEnv, ref string) error { + existing, err := resolveSecretForWrite(ctx, env, ref) + if err != nil { + return err + } + name := runs.SanitizeLine(existing.Name) + if err := confirm(env, secretRmYes, "delete secret "+name+"? Its value cannot be recovered"); err != nil { + return err + } + if err := env.client.DeleteSecret(ctx, existing.ID); err != nil { + return writeExit(err, env.cred, "secrets", scopeSecretsWrite) + } + fmt.Fprintf(env.stdout, "secret %s deleted\n", name) return nil } diff --git a/cmd/writeapi.go b/cmd/writeapi.go new file mode 100644 index 0000000..989afe0 --- /dev/null +++ b/cmd/writeapi.go @@ -0,0 +1,289 @@ +package cmd + +import ( + "bufio" + "bytes" + "context" + "encoding/json" + "errors" + "fmt" + "io" + "os" + "strings" + "unicode/utf8" + + "github.com/nxssie/nan-cli/internal/api" + "github.com/nxssie/nan-cli/internal/runs" + "github.com/nxssie/nan-cli/internal/session" + "go.yaml.in/yaml/v3" +) + +// The write API (helmcode/nan-devops AGENT-AUTOMATIONS-PHASE3-WRITE-API.md): +// secrets and automations written from this CLI. The platform accepts a CLI +// session (nan auth login) or a platform token with the route's scope +// (secrets:read, secrets:write, automations:write); it refuses API keys +// (sk-), workspace keys and sessions from older clients. Approving a +// proposal stays portal-only. + +// writeDocs is where the CLI's write commands are documented. +const writeDocs = "https://nan.builders/docs/nan-cli" + +// The scopes the write commands need, as the platform names them. +const ( + scopeSecretsRead = "secrets:read" + scopeSecretsWrite = "secrets:write" + scopeAutomationsWrite = "automations:write" +) + +// writeCredentialHelp is the part of every write command's help that says +// what it authenticates with and what that hands out. +const writeCredentialHelp = `Authentication, first match wins: --token-file, your session from +nan auth login, NAN_TOKEN, a platform token saved with nan auth login +--api-token. A platform token needs the scope of what it does: +secrets:read to list and show secrets, secrets:write to change them, +automations:write to change automations (automations:read to find one by +name). Those scopes are added to a token in the portal only, Settings > +Tokens, signed in through the browser. API keys (sk-) and workspace keys +never read or write secrets and never change automations. + +Whoever holds a credential that can write here can change your secrets and +automations: an automation receives the secrets you assign it, so a +credential that can write automations can reach every secret you have. Sign +in with nan auth login on your own machine, not inside a workspace (an +agent there could read the session file). In a workspace or CI job, use a +token with only the scopes the job needs.` + +// Test seams: where a confirmation or a piped value is read from, and +// whether that is a terminal. +var ( + writeStdin io.Reader = os.Stdin + writeStdinTTY = func() bool { return isTerminal(os.Stdin) } +) + +// newWriteEnv picks the credential of a secrets or write command: an +// explicit --token-file first, then the session (a CLI session can do +// everything here without any scope set up), then NAN_TOKEN, then a saved +// platform token. An API key is refused before it is sent: the platform +// never takes one here. +func newWriteEnv() (*runsEnv, error) { + if tokenFileFlag != "" { + tok, err := readTokenFile(tokenFileFlag) + if err != nil { + return nil, err + } + return writeEnvFor(credential{kind: credFile, path: tokenFileFlag, bearer: tok}) + } + sess, err := session.Load() + if err != nil && !errors.Is(err, session.ErrNotLoggedIn) { + return nil, exitf(exitAuth, "could not read your session (%v): run nan auth login", err) + } + if sess != nil && sess.Token != "" { + return writeEnvFor(credential{kind: credSession, session: sess.Token}) + } + if v := strings.TrimSpace(os.Getenv(tokenEnvVar)); v != "" { + if !validTokenShape(v) { + return nil, exitf(exitAuth, "%s is set but does not hold a valid token: %s", tokenEnvVar, tokenShapeHint) + } + return writeEnvFor(credential{kind: credEnv, bearer: v}) + } + if sess != nil && sess.PlatformToken != "" { + return writeEnvFor(credential{kind: credStoredToken, bearer: sess.PlatformToken}) + } + return nil, exitf(exitAuth, "not signed in: run nan auth login, or set %s to a platform token with the scope you need (see --help)", tokenEnvVar) +} + +func writeEnvFor(cred credential) (*runsEnv, error) { + if strings.HasPrefix(cred.bearer, "sk-") { + return nil, exitf(exitAuth, "%s is an API key (sk-): API keys never read or write secrets and never change automations. "+ + "Sign in with nan auth login, or use a platform token (nan_pat_) with the scope you need, created at %s/settings", cred.describe(), runs.PortalURL) + } + return &runsEnv{client: cred.client(), cred: cred, stdout: os.Stdout, stderr: os.Stderr}, nil +} + +// withWriteEnv is withRunsEnv for the secrets and write commands. +func withWriteEnv(fn func(*runsEnv) error) error { + env, err := newWriteEnv() + if err != nil { + return err + } + return runtimeError(fn(env)) +} + +// writeAPIOff is the answer when the platform does not take writes from +// this CLI (yet) for the member: the portal still does them. +func writeAPIOff(what string) *ExitError { + return usageErrorf("changing %s from the CLI is not available for your account yet: do it in the portal at %s, "+ + "signed in through the browser. See %s", what, runs.PortalURL, writeDocs) +} + +// requireWriteAPI asks the platform, for a session, whether it takes writes +// from the CLI (features.write_api). A token has no /api/auth/me: its +// request is sent and a refusal is mapped by writeExit instead. When the +// question itself fails for another reason than an expired session, the +// write goes ahead and the platform decides. +func requireWriteAPI(ctx context.Context, env *runsEnv, what string) error { + if env.cred.kind != credSession { + return nil + } + f, err := env.client.Features(ctx) + if errors.Is(err, api.ErrSessionExpired) { + return authError(env.cred) + } + if err == nil && !f.WriteAPI { + return writeAPIOff(what) + } + return nil +} + +// writeExit maps a refused write (or a read of the write API) onto an exit +// code and a message that says what to do. what is "secrets" or +// "automations"; scope is the platform-token scope of the request. +func writeExit(err error, cred credential, what, scope string) *ExitError { + var apiErr *api.APIError + if !errors.As(err, &apiErr) { + return apiExit(err, cred) + } + msg := runs.SanitizeLine(apiErr.Message) + switch apiErr.Code { + case "browser_session_required": + // A platform that predates the write API, or has it off. + return writeAPIOff(what) + case "credential_not_allowed": + if cred.kind == credSession { + return exitf(exitAuth, "the platform does not take this session for changing %s: it is from an older sign-in. "+ + "Sign in again with: nan auth login", what) + } + return exitf(exitAuth, "%s cannot do this: %s needs a CLI session or a platform token with the %s scope", cred.describe(), what, scope) + case "workspace_key_not_allowed": + return exitf(exitAuth, "a workspace key never reads or writes secrets and never changes automations: "+ + "use your own session (nan auth login) or a platform token with the %s scope", scope) + case "token_scope_denied": + return exitf(exitAuth, "%s lacks a scope: %s. Create a token with the %s scope in the portal, Settings > Tokens at %s, "+ + "signed in through the browser (secrets:read, secrets:write and automations:write can only be added there)", + cred.describe(), msg, scope, runs.PortalURL) + case "rate_limited": + return exitf(exitUnavailable, "too many changes to your %s recently: try again later", what) + case "name_taken": + return usageErrorf("that name is already taken by another of your %s", what) + case "too_many_secrets": + return usageErrorf("you have as many secrets as an account can keep: delete one first (nan secrets rm NAME)") + case "idempotency_key_reused": + return usageErrorf("that idempotency key was already used to create a different secret") + case "secret_in_use": + return usageErrorf("the secret is assigned to these automations; remove it from them first (nan automations edit):\n%s", + joinNames(refLabels(apiErr.Automations))) + case "secrets_too_large": + return usageErrorf("with the new value, the secrets these automations receive would be over the size a run takes:\n%s", + joinNames(refLabels(apiErr.Automations))) + case "platform_access_required": + return exitf(exitAuth, "your membership does not include this: see %s/membership", runs.PortalURL) + case "secrets_unavailable": + return exitf(exitUnavailable, "the secrets store is unavailable right now; try again in a few minutes") + } + if apiErr.Status == 404 { + return usageErrorf("not found (%s)", msg) + } + return apiExit(err, cred) +} + +func refLabels(refs []api.Ref) []string { + out := make([]string, 0, len(refs)) + for i := range refs { + out = append(out, refs[i].Label()) + } + return out +} + +// updatedByLabel is the updated_by credential kind, as a person reads it. +func updatedByLabel(v string) string { + switch v { + case "browser_session": + return "Browser" + case "cli_session": + return "CLI" + case "platform_token": + return "API token" + case "": + return "" + } + return runs.SanitizeLine(v) +} + +// maxBodyFile bounds an automation or trigger file. The platform's own +// limit is lower; this only keeps a wrong file from being read whole. +const maxBodyFile = 1 << 20 + +// readBodyFile reads -f: a YAML or JSON file ("-" for stdin) holding one +// object, and returns it as compact JSON, sent as written. +func readBodyFile(path string) (json.RawMessage, error) { + if path == "" { + return nil, usageErrorf("-f FILE is required: a YAML or JSON file (- for stdin); see --help") + } + var ( + raw []byte + err error + ) + shown := runs.SanitizeLine(path) + if path == "-" { + if writeStdinTTY() { + return nil, usageErrorf("-f - reads the file from stdin, and stdin is a terminal: pipe it, or give a path") + } + raw, err = io.ReadAll(io.LimitReader(writeStdin, maxBodyFile+1)) + } else { + var f *os.File + f, err = os.Open(path) + if err != nil { + return nil, usageErrorf("could not read %s: %v", shown, unwrapPath(err)) + } + raw, err = io.ReadAll(io.LimitReader(f, maxBodyFile+1)) + f.Close() + } + if err != nil { + return nil, usageErrorf("could not read %s: %v", shown, unwrapPath(err)) + } + if len(raw) > maxBodyFile { + return nil, usageErrorf("%s is over %d KiB", shown, maxBodyFile/1024) + } + if !utf8.Valid(raw) { + return nil, usageErrorf("%s is not UTF-8 text", shown) + } + trimmed := bytes.TrimSpace(raw) + if len(trimmed) > 0 && trimmed[0] == '{' { + var compact bytes.Buffer + if err := json.Compact(&compact, trimmed); err != nil { + return nil, usageErrorf("%s is not valid JSON: %v", shown, err) + } + return compact.Bytes(), nil + } + var doc any + if err := yaml.Unmarshal(raw, &doc); err != nil { + return nil, usageErrorf("%s is neither valid JSON nor valid YAML: %v", shown, err) + } + obj, ok := doc.(map[string]any) + if !ok { + return nil, usageErrorf("%s must hold one object (name: ..., workspace: ...), not a list or a value", shown) + } + out, err := json.Marshal(obj) + if err != nil { + return nil, usageErrorf("%s has a value JSON cannot carry: %v", shown, err) + } + return out, nil +} + +// confirm asks a yes/no question on a terminal. Without one, the answer has +// to be given with --yes: a script never deletes by accident. +func confirm(env *runsEnv, yes bool, question string) error { + if yes { + return nil + } + if !writeStdinTTY() { + return usageErrorf("%s: confirm with --yes (stdin is not a terminal)", question) + } + fmt.Fprintf(env.stderr, "%s [y/N] ", question) + line, _ := bufio.NewReader(writeStdin).ReadString('\n') + switch strings.ToLower(strings.TrimSpace(line)) { + case "y", "yes": + return nil + } + return exitf(exitFailed, "cancelled: nothing was changed") +} diff --git a/go.mod b/go.mod index 2cf9520..66f3047 100644 --- a/go.mod +++ b/go.mod @@ -31,6 +31,7 @@ require ( github.com/rivo/uniseg v0.4.7 // indirect github.com/spf13/pflag v1.0.9 // indirect github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e // indirect + go.yaml.in/yaml/v3 v3.0.4 // indirect golang.org/x/sys v0.44.0 // indirect golang.org/x/text v0.39.0 // indirect ) diff --git a/go.sum b/go.sum index c144769..a2f04d3 100644 --- a/go.sum +++ b/go.sum @@ -50,6 +50,7 @@ github.com/spf13/pflag v1.0.9 h1:9exaQaMOCwffKiiiYk6/BndUBv+iRViNW+4lEMi0PvY= github.com/spf13/pflag v1.0.9/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e h1:JVG44RsyaB9T2KIHavMF/ppJZNG9ZpyihvCd0w101no= github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e/go.mod h1:RbqR21r5mrJuqunuUZ/Dhy/avygyECGrLceyNeo4LiM= +go.yaml.in/yaml/v3 v3.0.4 h1:tfq32ie2Jv2UxXFdLJdh3jXuOzWiL1fo0bu/FbuKpbc= go.yaml.in/yaml/v3 v3.0.4/go.mod h1:DhzuOOF2ATzADvBadXxruRBLzYTpT36CKvDb3+aBEFg= golang.org/x/exp v0.0.0-20231006140011-7918f672742d h1:jtJma62tbqLibJ5sFQz8bKtEM8rJBtfilJ2qTU199MI= golang.org/x/exp v0.0.0-20231006140011-7918f672742d/go.mod h1:ldy0pHrwJyGW56pPQzzkH36rKxoZW1tw7ZJpeKx+hdo= diff --git a/internal/api/automations.go b/internal/api/automations.go index 3e94dcb..eb88c94 100644 --- a/internal/api/automations.go +++ b/internal/api/automations.go @@ -37,9 +37,9 @@ const ( // BYOK reports whether the inference is the member's own provider key. func (in *Inference) BYOK() bool { return in != nil && in.Source == InferenceBYOK } -// Automation mirrors the automation object of /v1/automations. It is the -// member's own configuration, read here and never written: creating and -// editing automations happens in the portal. +// Automation mirrors the automation object of /v1/automations: the +// member's own configuration. Writes send the member's own file as it is +// (see CreateAutomation), so this type only has to read what comes back. type Automation struct { ID string `json:"id"` Object string `json:"object,omitempty"` @@ -63,6 +63,7 @@ type Automation struct { MaxTriggerDepth int `json:"max_trigger_depth"` Enabled bool `json:"enabled"` DisabledReason *string `json:"disabled_reason"` + UpdatedBy string `json:"updated_by,omitempty"` CreatedAt string `json:"created_at"` UpdatedAt string `json:"updated_at"` } @@ -73,8 +74,8 @@ type AutomationList struct { } // Trigger is a webhook or a schedule of an automation. The webhook URL and -// its signing secret are shown once, in the portal, and never again: only -// the URL's prefix is here. +// its signing secret are shown once, when the trigger is created or rotated +// (TriggerSecrets), and never again: only the URL's prefix is here. type Trigger struct { ID string `json:"id"` Kind string `json:"kind"` @@ -88,6 +89,7 @@ type Trigger struct { LastOutcome *string `json:"last_outcome,omitempty"` PrevSecretUntil *string `json:"prev_secret_until,omitempty"` CreatedAt string `json:"created_at,omitempty"` + UpdatedBy string `json:"updated_by,omitempty"` } type TriggerList struct { diff --git a/internal/api/runs.go b/internal/api/runs.go index 1f5cda0..e54062b 100644 --- a/internal/api/runs.go +++ b/internal/api/runs.go @@ -227,6 +227,9 @@ type APIError struct { Code string Param string Message string + // Automations names the automations a 409 secret_in_use or + // secrets_too_large is about (write API). + Automations []Ref } func (e *APIError) Error() string { @@ -377,21 +380,24 @@ func (c *RunsClient) statusError(status int, body []byte) error { apiErr := &APIError{Status: status} var openai struct { Error struct { - Message string `json:"message"` - Type string `json:"type"` - Param *string `json:"param"` - Code any `json:"code"` + Message string `json:"message"` + Type string `json:"type"` + Param *string `json:"param"` + Code any `json:"code"` + Automations []Ref `json:"automations"` } `json:"error"` } var bare struct { - Error string `json:"error"` - Message string `json:"message"` - Param *string `json:"param"` + Error string `json:"error"` + Message string `json:"message"` + Param *string `json:"param"` + Automations []Ref `json:"automations"` } switch { case json.Unmarshal(body, &openai) == nil && (openai.Error.Message != "" || openai.Error.Code != nil): apiErr.Message = openai.Error.Message apiErr.Type = openai.Error.Type + apiErr.Automations = openai.Error.Automations if openai.Error.Param != nil { apiErr.Param = *openai.Error.Param } @@ -412,6 +418,7 @@ func (c *RunsClient) statusError(status int, body []byte) error { if bare.Param != nil { apiErr.Param = *bare.Param } + apiErr.Automations = bare.Automations } return apiErr } diff --git a/internal/api/secrets.go b/internal/api/secrets.go index aa50641..31481dd 100644 --- a/internal/api/secrets.go +++ b/internal/api/secrets.go @@ -27,9 +27,12 @@ type Secret struct { ValueBytes int `json:"value_bytes"` Description string `json:"description"` UsedBy []Ref `json:"used_by"` - CreatedAt string `json:"created_at"` - UpdatedAt string `json:"updated_at"` - LastUsedAt *string `json:"last_used_at"` + // UpdatedBy is the credential kind of the last write: browser_session, + // cli_session or platform_token (write API). + UpdatedBy string `json:"updated_by,omitempty"` + CreatedAt string `json:"created_at"` + UpdatedAt string `json:"updated_at"` + LastUsedAt *string `json:"last_used_at"` } type SecretList struct { diff --git a/internal/api/testdata/phase2/automation_get.json b/internal/api/testdata/phase2/automation_get.json index c265359..d4a3c4f 100644 --- a/internal/api/testdata/phase2/automation_get.json +++ b/internal/api/testdata/phase2/automation_get.json @@ -4,6 +4,8 @@ "github_pr_review" ], "approval_ttl_seconds": 259200, + "checkout": "workspace", + "checkout_untrusted_heads": false, "concurrency_policy": "skip_if_active", "created_at": "2026-10-11T09:00:00Z", "cwd": "/home/nan/projects/api", @@ -29,8 +31,10 @@ "secret": "GITHUB_WRITE_TOKEN" } ], + "sticky_pr_comment": false, "template": "pr_review", "timeout_seconds": 1800, "updated_at": "2026-10-11T09:00:00Z", + "updated_by": "browser_session", "workspace": "11111111-2222-4333-8444-555555555555" } diff --git a/internal/api/testdata/phase2/automation_triggers.json b/internal/api/testdata/phase2/automation_triggers.json index b9ce112..02668f7 100644 --- a/internal/api/testdata/phase2/automation_triggers.json +++ b/internal/api/testdata/phase2/automation_triggers.json @@ -27,7 +27,8 @@ "signature_scheme": "github", "timezone": null, "token_prefix": "nan_whk_AbC1", - "updated_at": "2026-10-11T09:00:00Z" + "updated_at": "2026-10-11T09:00:00Z", + "updated_by": "browser_session" } ], "object": "list" diff --git a/internal/api/testdata/phase2/automations_list.json b/internal/api/testdata/phase2/automations_list.json index 8c87409..226709e 100644 --- a/internal/api/testdata/phase2/automations_list.json +++ b/internal/api/testdata/phase2/automations_list.json @@ -6,6 +6,8 @@ "github_pr_review" ], "approval_ttl_seconds": 259200, + "checkout": "workspace", + "checkout_untrusted_heads": false, "concurrency_policy": "skip_if_active", "created_at": "2026-10-11T09:00:00Z", "cwd": "/home/nan/projects/api", @@ -31,9 +33,11 @@ "secret": "GITHUB_WRITE_TOKEN" } ], + "sticky_pr_comment": false, "template": "pr_review", "timeout_seconds": 1800, "updated_at": "2026-10-11T09:00:00Z", + "updated_by": "browser_session", "workspace": "11111111-2222-4333-8444-555555555555" } ], diff --git a/internal/api/testdata/phase2/trigger_create.json b/internal/api/testdata/phase2/trigger_create.json new file mode 100644 index 0000000..f0259fd --- /dev/null +++ b/internal/api/testdata/phase2/trigger_create.json @@ -0,0 +1,34 @@ +{ + "signing_secret": "whsec_shown_once", + "trigger": { + "actor_filter": { + "author_associations": [], + "exclude_bots": false, + "logins": [ + "octocat" + ] + }, + "automation_id": "a0000000-0000-4000-8000-000000000001", + "created_at": "2026-10-11T09:00:00Z", + "cron": null, + "enabled": true, + "event_filter": { + "events": [ + "pull_request" + ] + }, + "id": "b0000000-0000-4000-8000-000000000002", + "kind": "webhook", + "last_fired_at": null, + "last_outcome": null, + "next_fire_at": null, + "object": "trigger", + "prev_secret_until": null, + "signature_scheme": "github", + "timezone": null, + "token_prefix": "nan_whk_AbC1", + "updated_at": "2026-10-11T09:00:00Z", + "updated_by": "browser_session" + }, + "webhook_url": "https://api.nan.builders/hooks/nan_whk_AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA" +} diff --git a/internal/api/testdata/phase2/trigger_rotate.json b/internal/api/testdata/phase2/trigger_rotate.json new file mode 100644 index 0000000..30599cb --- /dev/null +++ b/internal/api/testdata/phase2/trigger_rotate.json @@ -0,0 +1,3 @@ +{ + "signing_secret": "new_secret_shown_once" +} diff --git a/internal/api/write.go b/internal/api/write.go new file mode 100644 index 0000000..389766d --- /dev/null +++ b/internal/api/write.go @@ -0,0 +1,190 @@ +package api + +import ( + "context" + "encoding/json" + "net/http" + "net/url" +) + +// The write API (AGENT-AUTOMATIONS-PHASE3-WRITE-API.md): secrets and +// automations written from a CLI session or a platform token with the +// secrets:write / automations:write scope. The server decides whether a +// credential may write; this client only sends the request and reads the +// answer. Secret values go out in request bodies and never come back. + +// Features is the part of GET /api/auth/me this CLI reads. +type Features struct { + WriteAPI bool `json:"write_api"` + Secrets bool `json:"secrets"` + Automations bool `json:"automations"` +} + +// Features is GET /api/auth/me, read for its feature flags. +func (c *RunsClient) Features(ctx context.Context) (*Features, error) { + var me struct { + Features Features `json:"features"` + } + if err := c.getJSON(ctx, "/api/auth/me", &me); err != nil { + return nil, err + } + return &me.Features, nil +} + +// SecretsV1 is GET /v1/secrets: metadata only, for a session or a token +// with secrets:read. +func (c *RunsClient) SecretsV1(ctx context.Context) (*SecretList, error) { + list := &SecretList{} + return list, c.getJSON(ctx, "/v1/secrets", list) +} + +// CreateSecretRequest is the body of POST /v1/secrets. Its value is never +// printed: String and GoString redact it, so a %v or %#v in an error or a +// debug line cannot leak it. +type CreateSecretRequest struct { + Name string `json:"name"` + Value string `json:"value"` + Kind string `json:"kind,omitempty"` + Provider string `json:"provider,omitempty"` + BaseURL string `json:"base_url,omitempty"` + Description *string `json:"description,omitempty"` +} + +func (r CreateSecretRequest) String() string { + return "CreateSecretRequest{name:" + r.Name + " value:[redacted]}" +} +func (r CreateSecretRequest) GoString() string { return r.String() } + +// CreateSecret is POST /v1/secrets. idempotencyKey, when set, makes a retry +// answer the secret the first attempt created (replayed is then true). +func (c *RunsClient) CreateSecret(ctx context.Context, in CreateSecretRequest, idempotencyKey string) (s *Secret, replayed bool, err error) { + req, err := c.newRequest(ctx, http.MethodPost, "/v1/secrets", in) + if err != nil { + return nil, false, err + } + if idempotencyKey != "" { + req.Header.Set("Idempotency-Key", idempotencyKey) + } + s = &Secret{} + status, err := c.do(req, s) + if err != nil { + return nil, false, err + } + return s, status == http.StatusOK, nil +} + +type secretValueBody struct { + Value string `json:"value"` +} + +func (secretValueBody) String() string { return "secretValueBody{[redacted]}" } + +// ReplaceSecretValue is PUT /v1/secrets/{id}/value. +func (c *RunsClient) ReplaceSecretValue(ctx context.Context, id, value string) (*Secret, error) { + s := &Secret{} + return s, c.sendJSON(ctx, http.MethodPut, "/v1/secrets/"+url.PathEscape(id)+"/value", secretValueBody{Value: value}, s) +} + +// SecretPatch is the body of PATCH /v1/secrets/{id}: nil fields are left +// as they are. +type SecretPatch struct { + Description *string `json:"description,omitempty"` + BaseURL *string `json:"base_url,omitempty"` +} + +// UpdateSecret is PATCH /v1/secrets/{id}. +func (c *RunsClient) UpdateSecret(ctx context.Context, id string, p SecretPatch) (*Secret, error) { + s := &Secret{} + return s, c.sendJSON(ctx, http.MethodPatch, "/v1/secrets/"+url.PathEscape(id), p, s) +} + +// DeleteSecret is DELETE /v1/secrets/{id}. A secret an automation uses is +// a 409 secret_in_use whose APIError names the automations. +func (c *RunsClient) DeleteSecret(ctx context.Context, id string) error { + return c.sendJSON(ctx, http.MethodDelete, "/v1/secrets/"+url.PathEscape(id), nil, nil) +} + +// CreateAutomation is POST /v1/automations with the member's own body, +// passed through as written: the server validates every field. +func (c *RunsClient) CreateAutomation(ctx context.Context, body json.RawMessage) (*Automation, json.RawMessage, error) { + return c.automationWrite(ctx, http.MethodPost, "/v1/automations", body) +} + +// UpdateAutomation is PATCH /v1/automations/{id}: only the fields in body +// change. +func (c *RunsClient) UpdateAutomation(ctx context.Context, id string, body json.RawMessage) (*Automation, json.RawMessage, error) { + return c.automationWrite(ctx, http.MethodPatch, "/v1/automations/"+url.PathEscape(id), body) +} + +func (c *RunsClient) automationWrite(ctx context.Context, method, path string, body json.RawMessage) (*Automation, json.RawMessage, error) { + var raw json.RawMessage + if err := c.sendJSON(ctx, method, path, body, &raw); err != nil { + return nil, nil, err + } + a := &Automation{} + if err := json.Unmarshal(raw, a); err != nil { + return nil, nil, err + } + return a, raw, nil +} + +// DeleteAutomation is DELETE /v1/automations/{id}. +func (c *RunsClient) DeleteAutomation(ctx context.Context, id string) error { + return c.sendJSON(ctx, http.MethodDelete, "/v1/automations/"+url.PathEscape(id), nil, nil) +} + +// TriggerSecrets is what creating or rotating a trigger answers once and +// never again: the webhook URL (it carries the token that authenticates a +// delivery) and the signing secret. Absent fields were not issued. +type TriggerSecrets struct { + Trigger *Trigger `json:"trigger,omitempty"` + WebhookURL *string `json:"webhook_url,omitempty"` + SigningSecret *string `json:"signing_secret,omitempty"` +} + +// CreateTrigger is POST /v1/automations/{id}/triggers with the member's own +// body. Not idempotent: after a lost answer, list the triggers and rotate or +// delete the one you cannot account for. +func (c *RunsClient) CreateTrigger(ctx context.Context, automationID string, body json.RawMessage) (*TriggerSecrets, error) { + out := &TriggerSecrets{} + return out, c.sendJSON(ctx, http.MethodPost, "/v1/automations/"+url.PathEscape(automationID)+"/triggers", body, out) +} + +// UpdateTrigger is PATCH /v1/automations/{id}/triggers/{tid}. +func (c *RunsClient) UpdateTrigger(ctx context.Context, automationID, triggerID string, body json.RawMessage) (*Trigger, error) { + t := &Trigger{} + return t, c.sendJSON(ctx, http.MethodPatch, triggerPath(automationID, triggerID), body, t) +} + +// RotateRequest is the body of POST .../rotate: what to rotate ("url", +// "secret") and whether the previous signing secret stays valid for 24 h. +type RotateRequest struct { + Rotate []string `json:"rotate"` + Grace bool `json:"grace"` +} + +// RotateTrigger is POST /v1/automations/{id}/triggers/{tid}/rotate. +func (c *RunsClient) RotateTrigger(ctx context.Context, automationID, triggerID string, in RotateRequest) (*TriggerSecrets, error) { + out := &TriggerSecrets{} + return out, c.sendJSON(ctx, http.MethodPost, triggerPath(automationID, triggerID)+"/rotate", in, out) +} + +// DeleteTrigger is DELETE /v1/automations/{id}/triggers/{tid}. +func (c *RunsClient) DeleteTrigger(ctx context.Context, automationID, triggerID string) error { + return c.sendJSON(ctx, http.MethodDelete, triggerPath(automationID, triggerID), nil, nil) +} + +func triggerPath(automationID, triggerID string) string { + return "/v1/automations/" + url.PathEscape(automationID) + "/triggers/" + url.PathEscape(triggerID) +} + +// sendJSON sends body (nil for none; a json.RawMessage goes as it is) and +// decodes a 2xx answer into out (nil to ignore it, as for a 204). +func (c *RunsClient) sendJSON(ctx context.Context, method, path string, body, out any) error { + req, err := c.newRequest(ctx, method, path, body) + if err != nil { + return err + } + _, err = c.do(req, out) + return err +} From 495cdb16ad77077deb3c661f55dc5d3cf4065b20 Mon Sep 17 00:00:00 2001 From: barckcode Date: Wed, 14 Oct 2026 15:57:56 +0200 Subject: [PATCH 2/7] feat: nan secrets set/show/edit/rm and automation writes from the CLI (0.1.33) Write API (AGENT-AUTOMATIONS-PHASE3-WRITE-API.md, nan-cloud-api#348): secrets and automations written from a CLI session or a platform token with secrets:read, secrets:write or automations:write. Values only from a file or a pipe; webhook URL and signing secret printed once. Exit 64 and the portal when the account does not have the write API. Co-Authored-By: Claude Opus 5.5 --- README.md | 50 +- cmd/automations_test.go | 49 +- cmd/byok_test.go | 9 +- cmd/secrets.go | 4 +- cmd/testdata/byok/automations_show.golden | 2 +- cmd/testdata/byok/secrets_ls.golden | 8 +- cmd/testdata/writeapi/secrets_show.golden | 11 + cmd/testdata/writeapi/triggers_add.golden | 2 + cmd/writeapi.go | 4 +- cmd/writeapi_test.go | 971 ++++++++++++++++++++++ internal/tui/tui.go | 2 +- 11 files changed, 1056 insertions(+), 56 deletions(-) create mode 100644 cmd/testdata/writeapi/secrets_show.golden create mode 100644 cmd/testdata/writeapi/triggers_add.golden create mode 100644 cmd/writeapi_test.go diff --git a/README.md b/README.md index 9cf72bb..18705d2 100644 --- a/README.md +++ b/README.md @@ -172,27 +172,55 @@ nan runs reject [--reason R] # turn the proposal ### Automations, approvals and secrets -Automations are created and edited in the portal. From the CLI you can list, -inspect and run them, read their webhook deliveries, see what waits for your -approval, and list your secrets (names and metadata, never values): +From the CLI you can create, change and run automations, manage their +triggers, read their webhook deliveries, see what waits for your approval, +and manage your secrets (values go in, only names and metadata come out): ```bash nan automations ls [--json] nan automations show [--json] +nan automations create -f automation.yaml # YAML or JSON, - for stdin +nan automations edit -f changes.yaml # only the fields in the file change +nan automations enable|disable +nan automations rm [--yes] +nan automations triggers ls [--json] +nan automations triggers add -f trigger.yaml [--json] # webhook URL + secret shown ONCE +nan automations triggers edit -f changes.yaml +nan automations triggers rotate [--url] [--secret] [--no-grace] [--json] +nan automations triggers rm [--yes] nan automations run [--input @payload.json | --input - | --input JSON] [--detach] [--json] nan automations deliveries [--limit 50] [--json] nan approvals ls [--state pending|approved|rejected|expired|cancelled|auto|all] [--limit 20] [--json] nan approvals show [--diff] # same as nan runs proposal -nan secrets ls [--json] # needs nan auth login +nan secrets ls [--json] +nan secrets show NAME [--json] +nan secrets set NAME (--value-file PATH | --value-stdin) [--kind provider_key --provider P --base-url URL] [--description D] +nan secrets edit NAME [--description D] [--base-url URL] +nan secrets rm NAME [--yes] ``` -Approving a proposal, writing a secret and creating or editing an automation -happen only in the portal at , signed in through -the browser. Each of them can hand a secret to an agent or let it act on your -repository without asking, so none is possible with a credential an agent or -a CI job could read: this machine's CLI session, a platform token or an API -key. A platform token needs the `automations:read` scope to list and inspect -automations and `automations:run` to start one. +A secret's value is never an argument (it would stay in your shell history): +it comes from `--value-file` or a pipe into `--value-stdin`, and is never +shown again. A webhook's URL and signing secret are printed once, to stdout, +when the trigger is added or rotated: store them in the sender right away. + +Writes need your session from `nan auth login`, or a platform token with the +scope of what it does: `secrets:read` to list secrets, `secrets:write` to +change them, `automations:write` to change automations (plus +`automations:read` to find one by name). Those scopes are added to a token in +the portal only, signed in through the browser. API keys (`sk-`) and +workspace keys never read or write secrets and never change automations. +Whoever holds a credential that can write automations can reach every secret +you have (an automation receives the secrets you assign it), so sign in on +your own machine, not inside a workspace, and give a workspace or CI job a +token with only the scopes it needs. If your account does not have the write +API yet, these commands exit 64 and point at the portal. + +Approving a proposal stays portal-only, at , +signed in through the browser: a credential an agent or a script can hold +never approves what an agent proposed. A platform token needs the +`automations:read` scope to list and inspect automations and +`automations:run` to start one. The text views show workspace and automation names, looked up once per command; detail views print `name (id)` so the id stays copyable. A token diff --git a/cmd/automations_test.go b/cmd/automations_test.go index 0a688fc..af5cdef 100644 --- a/cmd/automations_test.go +++ b/cmd/automations_test.go @@ -558,19 +558,12 @@ func TestRunsLsAcceptsAwaitingApproval(t *testing.T) { } } -// nan secrets ls goes with the session even when a token is set: the -// platform refuses tokens and API keys there (§3.4). -func TestSecretsUseTheSessionOnly(t *testing.T) { - fakeHome(t, `{"token":"","platformToken":"`+testPAT+`","apiKey":"`+testKey+`"}`) - t.Setenv(tokenEnvVar, testPAT) - _, err := newSessionEnv() - if exitCode(err) != exitAuth || !strings.Contains(err.Error(), "nan auth login") || strings.Contains(err.Error(), testPAT) { - t.Fatalf("exit %d: %v", exitCode(err), err) - } - +// nan secrets ls goes with the session even when a token is set, on the +// portal's route, which any session may read whatever the write API. +func TestSecretsLsPrefersTheSession(t *testing.T) { fakeHome(t, `{"token":"sess-1","platformToken":"`+testPAT+`"}`) t.Setenv(tokenEnvVar, testPAT) - env, err := newSessionEnv() + env, err := newWriteEnv() if err != nil { t.Fatal(err) } @@ -593,9 +586,6 @@ func TestSecretsUseTheSessionOnly(t *testing.T) { } } assertNoControls(t, "secrets ls", out.String()) - if !strings.Contains(errOut.String(), "Settings > Secrets") { - t.Errorf("no portal hint: %q", errOut.String()) - } } func TestSecretsLsJSONHasNoValues(t *testing.T) { @@ -642,20 +632,13 @@ func execute(t *testing.T, args ...string) (int, string) { return code, msg } -// Approving, writing secrets and writing automations are portal-only in -// Phase 2 (§9.4, §11.2, owner decision 10): no command does them, and -// reaching for one says where to go and why. -func TestPortalOnlyWritesPointToThePortal(t *testing.T) { +// Approving stays portal-only (write API decision 4): no command does it, +// and reaching for one says where to go and why. +func TestApprovingPointsToThePortal(t *testing.T) { fakeHome(t, "") for _, args := range [][]string{ {"runs", "approve", testRunID}, {"approvals", "approve", testRunID}, - {"secrets", "set", "GITHUB_TOKEN"}, - {"secrets", "rm", "GITHUB_TOKEN"}, - {"automations", "create"}, - {"automations", "edit", "PR review"}, - {"automations", "rm", "PR review"}, - {"automations", "triggers", "add"}, } { code, msg := execute(t, args...) if code != exitUsage || !strings.Contains(msg, "https://cloud.nan.builders") || !strings.Contains(msg, "browser") { @@ -664,16 +647,12 @@ func TestPortalOnlyWritesPointToThePortal(t *testing.T) { } } -func TestNoCommandWrites(t *testing.T) { +func TestNoCommandApproves(t *testing.T) { var walk func(c *cobra.Command) walk = func(c *cobra.Command) { for _, sub := range c.Commands() { - path := sub.CommandPath() - switch sub.Name() { - case "approve", "set", "create", "edit", "rm", "delete", "triggers": - if !strings.HasPrefix(path, "nan auth") { - t.Errorf("%q exists", path) - } + if sub.Name() == "approve" || sub.HasAlias("approve") { + t.Errorf("%q exists", sub.CommandPath()) } walk(sub) } @@ -716,14 +695,14 @@ func TestPhase2Help(t *testing.T) { if !strings.Contains(runCmd.Long, "10 awaiting approval") || !strings.Contains(runsLogsCmd.Long, "10 awaiting approval") { t.Error("exit code 10 is not documented") } - for _, c := range append(automationsCmd.Commands(), approvalsCmd.Commands()...) { + cmds := append(automationsCmd.Commands(), approvalsCmd.Commands()...) + cmds = append(cmds, secretsCmd.Commands()...) + cmds = append(cmds, automationsTriggersCmd.Commands()...) + for _, c := range cmds { if c.InheritedFlags().Lookup("token-file") == nil { t.Errorf("%s has no --token-file", c.CommandPath()) } } - if secretsLsCmd.Flags().Lookup("token-file") != nil || secretsLsCmd.InheritedFlags().Lookup("token-file") != nil { - t.Error("nan secrets ls takes --token-file, which the platform would refuse") - } } // nan runs logs -f exits 10 on a run that stops for approval, like nan run. diff --git a/cmd/byok_test.go b/cmd/byok_test.go index 88c2db7..d5cfa86 100644 --- a/cmd/byok_test.go +++ b/cmd/byok_test.go @@ -206,7 +206,14 @@ func byokGoldenEnv(t *testing.T) (*runsEnv, *bytes.Buffer, *bytes.Buffer) { func checkTextGolden(t *testing.T, name, got string) { t.Helper() - path := filepath.Join("testdata", "byok", name+".golden") + checkGolden(t, "byok/"+name+".golden", got) +} + +// checkGolden compares got with cmd/testdata/; -update rewrites it. +func checkGolden(t *testing.T, rel, got string) { + t.Helper() + name := rel + path := filepath.Join("testdata", filepath.FromSlash(rel)) if *updateGolden { if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { t.Fatal(err) diff --git a/cmd/secrets.go b/cmd/secrets.go index 0516253..0b03492 100644 --- a/cmd/secrets.go +++ b/cmd/secrets.go @@ -74,7 +74,7 @@ where it would stay in your shell history: The portal does all of this too, Settings > Secrets at ` + runs.PortalURL + `, signed in through the browser. -Docs: ` + writeDocs, +Docs: ` + writeDocs + ` and https://nan.builders/docs/runs`, Annotations: sysexits, Args: cobra.ArbitraryArgs, RunE: func(cmd *cobra.Command, args []string) error { @@ -345,7 +345,7 @@ func readSecretValue(path string, fromStdin bool) (string, error) { return "", usageErrorf("--value-file %s is a directory", shown) } if runtime.GOOS != "windows" && info.Mode().IsRegular() && info.Mode().Perm()&0o077 != 0 { - fmt.Fprintf(os.Stderr, "note: %s can be read by other users of this machine (chmod 600 %s)\n", shown, shown) + fmt.Fprintf(writeStderr, "note: %s can be read by other users of this machine (chmod 600 %s)\n", shown, shown) } } raw, err = io.ReadAll(io.LimitReader(f, maxSecretValue+3)) diff --git a/cmd/testdata/byok/automations_show.golden b/cmd/testdata/byok/automations_show.golden index ad49535..217e456 100644 --- a/cmd/testdata/byok/automations_show.golden +++ b/cmd/testdata/byok/automations_show.golden @@ -24,4 +24,4 @@ instructions: Review for correctness. run it: nan automations run a0000000-0000-4000-8000-000000000001 -edit it in the portal: https://cloud.nan.builders/automations/a0000000-0000-4000-8000-000000000001 +change it: nan automations edit a0000000-0000-4000-8000-000000000001 -f FILE, or in the portal: https://cloud.nan.builders/automations/a0000000-0000-4000-8000-000000000001 diff --git a/cmd/testdata/byok/secrets_ls.golden b/cmd/testdata/byok/secrets_ls.golden index 3c88b3a..27a945f 100644 --- a/cmd/testdata/byok/secrets_ls.golden +++ b/cmd/testdata/byok/secrets_ls.golden @@ -1,4 +1,4 @@ -NAME KIND PROVIDER HINT USED BY LAST USED UPDATED BASE URL -GITHUB_WRITE_TOKEN generic - …6789 - never 2026-10-11 12:00 - -TOGETHER_KEY provider_key openai_compatible …6789 - never 2026-10-11 12:00 https://api.together.xyz/v1 -ANTHROPIC_KEY provider_key anthropic …6789 - never 2026-10-11 12:00 - +NAME KIND PROVIDER HINT USED BY LAST USED UPDATED CHANGED FROM BASE URL +GITHUB_WRITE_TOKEN generic - …6789 - never 2026-10-11 12:00 - - +TOGETHER_KEY provider_key openai_compatible …6789 - never 2026-10-11 12:00 - https://api.together.xyz/v1 +ANTHROPIC_KEY provider_key anthropic …6789 - never 2026-10-11 12:00 - - diff --git a/cmd/testdata/writeapi/secrets_show.golden b/cmd/testdata/writeapi/secrets_show.golden new file mode 100644 index 0000000..6333d0c --- /dev/null +++ b/cmd/testdata/writeapi/secrets_show.golden @@ -0,0 +1,11 @@ +name GITHUB_TOKEN +id 5e000000-0000-4000-8000-000000000001 +kind generic +hint …a1b2 +size 93 bytes +description fine-grained +used by PR review api +last changed from CLI +created 2026-10-10 12:00 +updated 2026-10-10 12:00 +last used never diff --git a/cmd/testdata/writeapi/triggers_add.golden b/cmd/testdata/writeapi/triggers_add.golden new file mode 100644 index 0000000..29b3e47 --- /dev/null +++ b/cmd/testdata/writeapi/triggers_add.golden @@ -0,0 +1,2 @@ +webhook URL: https://api.nan.builders/hooks/nan_whk_AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA +signing secret: whsec_shown_once diff --git a/cmd/writeapi.go b/cmd/writeapi.go index 989afe0..b8e0792 100644 --- a/cmd/writeapi.go +++ b/cmd/writeapi.go @@ -58,6 +58,8 @@ token with only the scopes the job needs.` var ( writeStdin io.Reader = os.Stdin writeStdinTTY = func() bool { return isTerminal(os.Stdin) } + writeStdout io.Writer = os.Stdout + writeStderr io.Writer = os.Stderr ) // newWriteEnv picks the credential of a secrets or write command: an @@ -97,7 +99,7 @@ func writeEnvFor(cred credential) (*runsEnv, error) { return nil, exitf(exitAuth, "%s is an API key (sk-): API keys never read or write secrets and never change automations. "+ "Sign in with nan auth login, or use a platform token (nan_pat_) with the scope you need, created at %s/settings", cred.describe(), runs.PortalURL) } - return &runsEnv{client: cred.client(), cred: cred, stdout: os.Stdout, stderr: os.Stderr}, nil + return &runsEnv{client: cred.client(), cred: cred, stdout: writeStdout, stderr: writeStderr}, nil } // withWriteEnv is withRunsEnv for the secrets and write commands. diff --git a/cmd/writeapi_test.go b/cmd/writeapi_test.go new file mode 100644 index 0000000..381c944 --- /dev/null +++ b/cmd/writeapi_test.go @@ -0,0 +1,971 @@ +package cmd + +import ( + "bytes" + "encoding/json" + "fmt" + "io" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "regexp" + "strings" + "sync" + "testing" + "time" + + "github.com/nxssie/nan-cli/internal/api" + + "github.com/spf13/cobra" + "github.com/spf13/pflag" +) + +// The write API (helmcode/nan-devops AGENT-AUTOMATIONS-PHASE3-WRITE-API.md, +// helmcode/nan-cloud-api#348) as a test server: /api/auth/me for the flag, +// /v1/secrets, the automation and trigger writes, with cloud-api's own +// golden responses where it has them. + +// testValue is a secret value that must never be printed or sent anywhere +// but the body of a create or a value replace. +const testValue = "ghp_TESTVALUEdoNotLeak0123456789abcdef" + +type writeReq struct { + Method, Path, Cookie, Authz, IdemKey, Body string +} + +type fakeWrite struct { + mu sync.Mutex + writeAPI bool + secrets []map[string]any + reqs []writeReq + idem map[string]map[string]any + // fail answers "METHOD /path" with status and body. + fail map[string]struct { + status int + body string + } + // dropFirstCreate closes the connection of the first POST /v1/secrets + // after storing the secret: an answer lost on the way. + dropFirstCreate bool + dropped bool + automationPatch map[string]any +} + +func newFakeWrite() *fakeWrite { + return &fakeWrite{ + writeAPI: true, + idem: map[string]map[string]any{}, + secrets: []map[string]any{{ + "id": "5e000000-0000-4000-8000-000000000001", "object": "secret", "name": "GITHUB_TOKEN", "kind": "generic", + "provider": nil, "base_url": nil, "hint": "…a1b2", "value_bytes": 93, "description": "fine-grained", + "used_by": []any{map[string]any{"id": testAutoID, "name": "PR review api"}}, "updated_by": "cli_session", + "created_at": "2026-10-10T12:00:00Z", "updated_at": "2026-10-10T12:00:00Z", "last_used_at": nil, + }}, + } +} + +func (f *fakeWrite) failOn(route string, status int, body string) { + if f.fail == nil { + f.fail = map[string]struct { + status int + body string + }{} + } + f.fail[route] = struct { + status int + body string + }{status, body} +} + +func phase2File(t testing.TB, name string) []byte { + b, err := os.ReadFile(filepath.Join("..", "internal", "api", "testdata", "phase2", name)) + if err != nil { + t.Fatal(err) + } + return b +} + +func (f *fakeWrite) handler(t *testing.T) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + body, _ := io.ReadAll(r.Body) + f.mu.Lock() + defer f.mu.Unlock() + f.reqs = append(f.reqs, writeReq{r.Method, r.URL.Path, r.Header.Get("Cookie"), r.Header.Get("Authorization"), r.Header.Get("Idempotency-Key"), string(body)}) + w.Header().Set("Content-Type", "application/json") + route := r.Method + " " + r.URL.Path + if fe, ok := f.fail[route]; ok { + w.WriteHeader(fe.status) + _, _ = io.WriteString(w, fe.body) + return + } + writeJSON := func(status int, v any) { + w.WriteHeader(status) + _ = json.NewEncoder(w).Encode(v) + } + const triggerID = "b0000000-0000-4000-8000-000000000002" + switch { + case route == "GET /api/auth/me": + writeJSON(200, map[string]any{"features": map[string]bool{"write_api": f.writeAPI, "secrets": true, "automations": true}}) + case route == "GET /api/account/secrets", route == "GET /v1/secrets": + if route == "GET /v1/secrets" && !f.writeAPI { + writeJSON(404, map[string]any{"error": map[string]any{"message": "Not found", "type": "invalid_request_error", "code": "not_found"}}) + return + } + writeJSON(200, map[string]any{"object": "list", "data": f.secrets}) + case route == "POST /v1/secrets": + var in map[string]any + _ = json.Unmarshal(body, &in) + key := r.Header.Get("Idempotency-Key") + if s, ok := f.idem[key]; ok && key != "" { + writeJSON(200, s) + return + } + s := map[string]any{"id": "5e000000-0000-4000-8000-000000000002", "object": "secret", "name": in["name"], "kind": "generic", + "hint": "…cdef", "value_bytes": len(in["value"].(string)), "description": "", "used_by": []any{}, "updated_by": "cli_session", + "created_at": "2026-10-14T10:00:00Z", "updated_at": "2026-10-14T10:00:00Z", "last_used_at": nil} + if k, ok := in["kind"].(string); ok { + s["kind"] = k + } + f.secrets = append(f.secrets, s) + f.idem[key] = s + if f.dropFirstCreate && !f.dropped { + f.dropped = true + hj, _ := w.(http.Hijacker) + conn, _, _ := hj.Hijack() + conn.Close() + return + } + writeJSON(201, s) + case strings.HasPrefix(r.URL.Path, "/v1/secrets/"): + id := strings.TrimSuffix(strings.TrimPrefix(r.URL.Path, "/v1/secrets/"), "/value") + var s map[string]any + idx := -1 + for i, x := range f.secrets { + if x["id"] == id { + s, idx = x, i + } + } + if s == nil { + writeJSON(404, map[string]any{"error": map[string]any{"message": "Not found", "type": "invalid_request_error", "code": "not_found"}}) + return + } + switch r.Method { + case http.MethodPut: + var in map[string]any + _ = json.Unmarshal(body, &in) + s["value_bytes"] = len(in["value"].(string)) + writeJSON(200, s) + case http.MethodPatch: + var in map[string]any + _ = json.Unmarshal(body, &in) + for k, v := range in { + s[k] = v + } + writeJSON(200, s) + case http.MethodDelete: + f.secrets = append(f.secrets[:idx], f.secrets[idx+1:]...) + w.WriteHeader(204) + } + case route == "GET /v1/automations": + _, _ = w.Write(phase2File(t, "automations_list.json")) + case route == "GET /v1/automations/"+testAutoID: + _, _ = w.Write(phase2File(t, "automation_get.json")) + case route == "POST /v1/automations", route == "PATCH /v1/automations/"+testAutoID: + var a map[string]any + _ = json.Unmarshal(phase2File(t, "automation_get.json"), &a) + var in map[string]any + _ = json.Unmarshal(body, &in) + for k, v := range in { + a[k] = v + } + a["updated_by"] = "cli_session" + status := 200 + if r.Method == http.MethodPost { + status = 201 + } + writeJSON(status, a) + case route == "DELETE /v1/automations/"+testAutoID, route == "DELETE /v1/automations/"+testAutoID+"/triggers/"+triggerID: + w.WriteHeader(204) + case route == "GET /v1/automations/"+testAutoID+"/triggers": + _, _ = w.Write(phase2File(t, "automation_triggers.json")) + case route == "POST /v1/automations/"+testAutoID+"/triggers": + w.Header().Set("Cache-Control", "no-store") + w.WriteHeader(201) + _, _ = w.Write(phase2File(t, "trigger_create.json")) + case route == "PATCH /v1/automations/"+testAutoID+"/triggers/"+triggerID: + var tc struct { + Trigger map[string]any `json:"trigger"` + } + _ = json.Unmarshal(phase2File(t, "trigger_create.json"), &tc) + var in map[string]any + _ = json.Unmarshal(body, &in) + for k, v := range in { + tc.Trigger[k] = v + } + writeJSON(200, tc.Trigger) + case route == "POST /v1/automations/"+testAutoID+"/triggers/"+triggerID+"/rotate": + w.Header().Set("Cache-Control", "no-store") + _, _ = w.Write(phase2File(t, "trigger_rotate.json")) + default: + writeJSON(404, map[string]any{"error": map[string]any{"message": "no route " + route, "type": "invalid_request_error", "code": "not_found"}}) + } + }) +} + +func (f *fakeWrite) requests() []writeReq { + f.mu.Lock() + defer f.mu.Unlock() + return append([]writeReq(nil), f.reqs...) +} + +func (f *fakeWrite) sent(method, path string) []writeReq { + var out []writeReq + for _, r := range f.requests() { + if r.Method == method && r.Path == path { + out = append(out, r) + } + } + return out +} + +// writeRun is one nan invocation against a fakeWrite. +type writeRun struct { + code int + msg string + stdout, stderr string + home string +} + +type writeSetup struct { + session string // session.json; "" signs nobody in + env string // NAN_TOKEN + stdin string + tty bool +} + +const triggerUUID = "b0000000-0000-4000-8000-000000000002" + +// resetCommandFlags puts every flag of the tree back to its default and +// unchanged, so one execute does not leak into the next. +func resetCommandFlags(c *cobra.Command) { + reset := func(f *pflag.Flag) { + _ = f.Value.Set(f.DefValue) + f.Changed = false + } + c.Flags().VisitAll(reset) + c.PersistentFlags().VisitAll(reset) + for _, sub := range c.Commands() { + resetCommandFlags(sub) + } +} + +func runWrite(t *testing.T, fake *fakeWrite, setup writeSetup, args ...string) writeRun { + t.Helper() + srv := httptest.NewServer(fake.handler(t)) + t.Cleanup(srv.Close) + home := fakeHome(t, setup.session) + t.Setenv(tokenEnvVar, setup.env) + + oldBase, oldIn, oldTTY, oldOut, oldErr, oldAttempts := runsBaseURL, writeStdin, writeStdinTTY, writeStdout, writeStderr, createAttempts + t.Cleanup(func() { + runsBaseURL, writeStdin, writeStdinTTY, writeStdout, writeStderr, createAttempts = oldBase, oldIn, oldTTY, oldOut, oldErr, oldAttempts + resetCommandFlags(rootCmd) + }) + runsBaseURL = srv.URL + loc := time.Local + time.Local = time.UTC + t.Cleanup(func() { time.Local = loc }) + writeStdin = strings.NewReader(setup.stdin) + tty := setup.tty + writeStdinTTY = func() bool { return tty } + var out, errOut bytes.Buffer + writeStdout, writeStderr = &out, &errOut + resetCommandFlags(rootCmd) + + code, msg := execute(t, args...) + return writeRun{code: code, msg: msg, stdout: out.String(), stderr: errOut.String(), home: home} +} + +const cliSession = `{"token":"sess-cli"}` + +func writeValueFile(t *testing.T, content string) string { + t.Helper() + p := filepath.Join(t.TempDir(), "value") + if err := os.WriteFile(p, []byte(content), 0o600); err != nil { + t.Fatal(err) + } + return p +} + +// noLeak checks the value is nowhere but where it has to be. +func noLeak(t *testing.T, r writeRun, fake *fakeWrite) { + t.Helper() + for what, text := range map[string]string{"stdout": r.stdout, "stderr": r.stderr, "message": r.msg} { + if strings.Contains(text, testValue) || strings.Contains(text, "TESTVALUE") { + t.Errorf("%s carries the value: %q", what, text) + } + } + for _, req := range fake.requests() { + writesValue := (req.Method == http.MethodPost && req.Path == "/v1/secrets") || + (req.Method == http.MethodPut && strings.HasSuffix(req.Path, "/value")) + if !writesValue && strings.Contains(req.Body+req.Path+req.IdemKey, "TESTVALUE") { + t.Errorf("%s %s carries the value", req.Method, req.Path) + } + } + _ = filepath.Walk(r.home, func(path string, info os.FileInfo, err error) error { + if err == nil && !info.IsDir() { + if b, _ := os.ReadFile(path); bytes.Contains(b, []byte("TESTVALUE")) { + t.Errorf("%s holds the value", path) + } + } + return nil + }) +} + +var idemKeyPattern = regexp.MustCompile(`^nan-cli-[0-9a-f]{32}$`) + +func TestSecretsSetCreatesFromAFile(t *testing.T) { + fake := newFakeWrite() + file := writeValueFile(t, testValue+"\n") + r := runWrite(t, fake, writeSetup{session: cliSession}, "secrets", "set", "NEW_TOKEN", "--value-file", file, "--description", "for ci") + if r.code != 0 { + t.Fatalf("exit %d: %s", r.code, r.msg) + } + posts := fake.sent("POST", "/v1/secrets") + if len(posts) != 1 { + t.Fatalf("posts: %v", fake.requests()) + } + var body map[string]any + _ = json.Unmarshal([]byte(posts[0].Body), &body) + if body["value"] != testValue || body["name"] != "NEW_TOKEN" || body["description"] != "for ci" { + t.Errorf("body %v", body) + } + if _, ok := body["kind"]; ok { + t.Errorf("sent a kind nobody asked for: %v", body) + } + if !idemKeyPattern.MatchString(posts[0].IdemKey) { + t.Errorf("Idempotency-Key %q", posts[0].IdemKey) + } + if posts[0].Cookie != "nan_session=sess-cli" { + t.Errorf("cookie %q", posts[0].Cookie) + } + if len(fake.sent("GET", "/api/auth/me")) != 1 { + t.Error("did not ask the platform whether it takes CLI writes") + } + wantAll(t, "stdout", r.stdout, "NEW_TOKEN created", "hint …cdef") + noLeak(t, r, fake) +} + +func TestSecretsSetReplacesTheValueOfAnExistingSecret(t *testing.T) { + fake := newFakeWrite() + r := runWrite(t, fake, writeSetup{session: cliSession, stdin: testValue + "\r\n"}, + "secrets", "set", "GITHUB_TOKEN", "--value-stdin", "--description", "rotated") + if r.code != 0 { + t.Fatalf("exit %d: %s", r.code, r.msg) + } + const path = "/v1/secrets/5e000000-0000-4000-8000-000000000001" + puts := fake.sent("PUT", path+"/value") + if len(puts) != 1 || puts[0].Body != `{"value":"`+testValue+`"}` { + t.Fatalf("puts %v", puts) + } + if p := fake.sent("PATCH", path); len(p) != 1 || p[0].Body != `{"description":"rotated"}` { + t.Errorf("patch %v", p) + } + if len(fake.sent("POST", "/v1/secrets")) != 0 { + t.Error("created a second secret") + } + wantAll(t, "stdout", r.stdout, "GITHUB_TOKEN: value replaced") + wantAll(t, "stderr", r.stderr, "PR review api") + noLeak(t, r, fake) +} + +func TestSecretsSetReadsTheValueExactly(t *testing.T) { + for in, want := range map[string]string{ + "abc\n": "abc", + "abc\r\n": "abc", + "abc\n\n": "abc\n", + "abc": "abc", + " a b c \n": " a b c ", + } { + writeStdin = strings.NewReader(in) + writeStdinTTY = func() bool { return false } + got, err := readSecretValue("", true) + if err != nil || got != want { + t.Errorf("%q: got %q, %v", in, got, err) + } + } + writeStdin, writeStdinTTY = os.Stdin, func() bool { return isTerminal(os.Stdin) } +} + +func TestSecretsSetRefusals(t *testing.T) { + file := writeValueFile(t, testValue) + for name, tc := range map[string]struct { + setup writeSetup + args []string + wants []string + }{ + "stdin is a terminal": {writeSetup{session: cliSession, tty: true}, []string{"secrets", "set", "X_TOKEN", "--value-stdin"}, []string{"terminal", "--value-file"}}, + "no value source": {writeSetup{session: cliSession}, []string{"secrets", "set", "X_TOKEN"}, []string{"--value-file", "--value-stdin"}}, + "both value sources": {writeSetup{session: cliSession}, []string{"secrets", "set", "X_TOKEN", "--value-stdin", "--value-file", file}, []string{"not both"}}, + "value on the command": {writeSetup{session: cliSession}, []string{"secrets", "set", "X_TOKEN", testValue}, []string{"shell history", "revoke"}}, + "value as the name": {writeSetup{session: cliSession}, []string{"secrets", "set", testValue, "--value-file", file}, []string{"shell history"}}, + "lower case name": {writeSetup{session: cliSession}, []string{"secrets", "set", "x_token", "--value-file", file}, []string{"X_TOKEN"}}, + "empty value": {writeSetup{session: cliSession, stdin: "\n"}, []string{"secrets", "set", "X_TOKEN", "--value-stdin"}, []string{"empty"}}, + "unknown kind": {writeSetup{session: cliSession}, []string{"secrets", "set", "X_TOKEN", "--value-file", file, "--kind", "other"}, []string{"generic or provider_key"}}, + "provider_key no prov": {writeSetup{session: cliSession}, []string{"secrets", "set", "X_TOKEN", "--value-file", file, "--kind", "provider_key"}, []string{"--provider"}}, + "provider with generic": {writeSetup{session: cliSession}, []string{"secrets", "set", "X_TOKEN", "--value-file", file, "--kind", "generic", "--provider", "openai"}, []string{"provider_key"}}, + "missing file": {writeSetup{session: cliSession}, []string{"secrets", "set", "X_TOKEN", "--value-file", "/nonexistent/v"}, []string{"could not read"}}, + } { + fake := newFakeWrite() + r := runWrite(t, fake, tc.setup, tc.args...) + if r.code != exitUsage { + t.Errorf("%s: exit %d: %s", name, r.code, r.msg) + } + for _, w := range tc.wants { + if !strings.Contains(r.msg, w) { + t.Errorf("%s: message lacks %q: %s", name, w, r.msg) + } + } + if len(fake.requests()) != 0 { + t.Errorf("%s: sent %v", name, fake.requests()) + } + noLeak(t, r, fake) + } +} + +func TestSecretsSetKindAndProviderAreFixed(t *testing.T) { + file := writeValueFile(t, testValue) + for _, args := range [][]string{ + {"--kind", "provider_key", "--provider", "openai"}, + {"--provider", "anthropic"}, + } { + fake := newFakeWrite() + r := runWrite(t, fake, writeSetup{session: cliSession}, append([]string{"secrets", "set", "GITHUB_TOKEN", "--value-file", file}, args...)...) + if r.code != exitUsage || !strings.Contains(r.msg, "fixed when a secret is created") { + t.Errorf("%v: exit %d: %s", args, r.code, r.msg) + } + for _, req := range fake.requests() { + if req.Method != http.MethodGet { + t.Errorf("%v: wrote %s %s", args, req.Method, req.Path) + } + } + } +} + +func TestSecretsSetProviderKey(t *testing.T) { + fake := newFakeWrite() + file := writeValueFile(t, testValue) + r := runWrite(t, fake, writeSetup{session: cliSession}, "secrets", "set", "LOCAL_LLM", "--value-file", file, + "--provider", "openai_compatible", "--base-url", "https://llm.example.com/v1") + if r.code != 0 { + t.Fatalf("exit %d: %s", r.code, r.msg) + } + var body map[string]any + _ = json.Unmarshal([]byte(fake.sent("POST", "/v1/secrets")[0].Body), &body) + if body["kind"] != "provider_key" || body["provider"] != "openai_compatible" || body["base_url"] != "https://llm.example.com/v1" { + t.Errorf("body %v", body) + } +} + +// A create whose answer is lost is sent again with the same key, and the +// platform answers the secret the first one created. +func TestSecretsSetRetriesWithTheSameIdempotencyKey(t *testing.T) { + fake := newFakeWrite() + fake.dropFirstCreate = true + file := writeValueFile(t, testValue) + r := runWrite(t, fake, writeSetup{session: cliSession}, "secrets", "set", "NEW_TOKEN", "--value-file", file) + if r.code != 0 { + t.Fatalf("exit %d: %s", r.code, r.msg) + } + posts := fake.sent("POST", "/v1/secrets") + if len(posts) != 2 || posts[0].IdemKey == "" || posts[0].IdemKey != posts[1].IdemKey { + t.Fatalf("posts %v", posts) + } + if n := len(fake.secrets); n != 2 { + t.Errorf("%d secrets: a retry created a second one", n) + } + wantAll(t, "stdout", r.stdout, "NEW_TOKEN created") +} + +// Two invocations never share a key: a later set of the same name is a new +// request, not a replay. +func TestIdempotencyKeysAreFresh(t *testing.T) { + a, b := newIdempotencyKey(), newIdempotencyKey() + if a == b || !idemKeyPattern.MatchString(a) { + t.Errorf("%q %q", a, b) + } +} + +func TestSecretsWriteAPIOffKeepsTheExplanation(t *testing.T) { + file := writeValueFile(t, testValue) + for name, args := range map[string][]string{ + "set": {"secrets", "set", "NEW_TOKEN", "--value-file", file}, + "edit": {"secrets", "edit", "GITHUB_TOKEN", "--description", "x"}, + "rm": {"secrets", "rm", "GITHUB_TOKEN", "--yes"}, + } { + // A session: features.write_api is false. + fake := newFakeWrite() + fake.writeAPI = false + r := runWrite(t, fake, writeSetup{session: cliSession}, args...) + if r.code != exitUsage || !strings.Contains(r.msg, "https://cloud.nan.builders") || !strings.Contains(r.msg, "browser") { + t.Errorf("%s (session): exit %d: %s", name, r.code, r.msg) + } + for _, req := range fake.requests() { + if req.Path != "/api/auth/me" { + t.Errorf("%s (session): sent %s %s", name, req.Method, req.Path) + } + } + // A token: /v1/secrets is a 404. + fake = newFakeWrite() + fake.writeAPI = false + r = runWrite(t, fake, writeSetup{env: testPAT}, args...) + if r.code != exitUsage || !strings.Contains(r.msg, "https://cloud.nan.builders") { + t.Errorf("%s (token): exit %d: %s", name, r.code, r.msg) + } + if strings.Contains(r.msg, testPAT) { + t.Errorf("%s: the token is in the message", name) + } + } +} + +// The CLI session goes first; NAN_TOKEN when there is none; an API key is +// never sent. +func TestSecretsCredentialOrder(t *testing.T) { + fake := newFakeWrite() + r := runWrite(t, fake, writeSetup{session: `{"token":"sess-cli","platformToken":"` + testPAT + `"}`, env: testPAT}, "secrets", "ls") + if r.code != 0 { + t.Fatalf("exit %d: %s", r.code, r.msg) + } + if got := fake.sent("GET", "/api/account/secrets"); len(got) != 1 || got[0].Cookie != "nan_session=sess-cli" || got[0].Authz != "" { + t.Errorf("session ls: %v", fake.requests()) + } + + fake = newFakeWrite() + r = runWrite(t, fake, writeSetup{env: testPAT}, "secrets", "ls") + if r.code != 0 { + t.Fatalf("exit %d: %s", r.code, r.msg) + } + if got := fake.sent("GET", "/v1/secrets"); len(got) != 1 || got[0].Authz != "Bearer "+testPAT || got[0].Cookie != "" { + t.Errorf("token ls: %v", fake.requests()) + } + wantAll(t, "ls", r.stdout, "GITHUB_TOKEN", "CHANGED FROM", "CLI") + + fake = newFakeWrite() + r = runWrite(t, fake, writeSetup{session: `{"platformToken":"` + testPAT + `"}`}, "secrets", "ls") + if r.code != 0 || len(fake.sent("GET", "/v1/secrets")) != 1 { + t.Errorf("saved token: exit %d %s %v", r.code, r.msg, fake.requests()) + } + + for _, setup := range []writeSetup{{env: testKey}, {session: `{"apiKey":"` + testKey + `"}`}, {}} { + fake = newFakeWrite() + r = runWrite(t, fake, setup, "secrets", "ls") + if r.code != exitAuth || len(fake.requests()) != 0 || strings.Contains(r.msg, testKey) { + t.Errorf("%+v: exit %d, sent %v: %s", setup, r.code, fake.requests(), r.msg) + } + } + fake = newFakeWrite() + r = runWrite(t, fake, writeSetup{env: testKey}, "secrets", "ls") + wantAll(t, "sk- refusal", r.msg, "API key", "nan auth login") +} + +func TestSecretsShow(t *testing.T) { + fake := newFakeWrite() + r := runWrite(t, fake, writeSetup{session: cliSession}, "secrets", "show", "GITHUB_TOKEN") + if r.code != 0 { + t.Fatalf("exit %d: %s", r.code, r.msg) + } + checkGolden(t, "writeapi/secrets_show.golden", r.stdout) + r = runWrite(t, fake, writeSetup{session: cliSession}, "secrets", "show", "GITHUB_TOKEN", "--json") + if r.code != 0 || !strings.Contains(r.stdout, `"updated_by": "cli_session"`) || strings.Contains(r.stdout, `"value"`) { + t.Errorf("exit %d: %s", r.code, r.stdout) + } + r = runWrite(t, fake, writeSetup{session: cliSession}, "secrets", "show", "NOPE") + if r.code != exitUsage || !strings.Contains(r.msg, "nan secrets ls") { + t.Errorf("exit %d: %s", r.code, r.msg) + } +} + +func TestSecretsEdit(t *testing.T) { + const path = "/v1/secrets/5e000000-0000-4000-8000-000000000001" + fake := newFakeWrite() + r := runWrite(t, fake, writeSetup{session: cliSession}, "secrets", "edit", "GITHUB_TOKEN", "--description", "") + if r.code != 0 { + t.Fatalf("exit %d: %s", r.code, r.msg) + } + if p := fake.sent("PATCH", path); len(p) != 1 || p[0].Body != `{"description":""}` { + t.Errorf("patch %v", p) + } + fake = newFakeWrite() + r = runWrite(t, fake, writeSetup{session: cliSession}, "secrets", "edit", "GITHUB_TOKEN") + if r.code != exitUsage || len(fake.requests()) != 0 { + t.Errorf("no flags: exit %d %v", r.code, fake.requests()) + } + fake = newFakeWrite() + r = runWrite(t, fake, writeSetup{session: cliSession}, "secrets", "edit", "GITHUB_TOKEN", "--base-url", "https://x.example/v1") + if p := fake.sent("PATCH", path); r.code != 0 || len(p) != 1 || p[0].Body != `{"base_url":"https://x.example/v1"}` { + t.Errorf("exit %d patch %v", r.code, p) + } +} + +func TestSecretsRm(t *testing.T) { + const path = "/v1/secrets/5e000000-0000-4000-8000-000000000001" + fake := newFakeWrite() + r := runWrite(t, fake, writeSetup{session: cliSession}, "secrets", "rm", "GITHUB_TOKEN") + if r.code != exitUsage || !strings.Contains(r.msg, "--yes") || len(fake.sent("DELETE", path)) != 0 { + t.Errorf("no tty, no --yes: exit %d %s", r.code, r.msg) + } + fake = newFakeWrite() + r = runWrite(t, fake, writeSetup{session: cliSession, tty: true, stdin: "n\n"}, "secrets", "rm", "GITHUB_TOKEN") + if r.code != exitFailed || len(fake.sent("DELETE", path)) != 0 { + t.Errorf("answered no: exit %d %s", r.code, r.msg) + } + fake = newFakeWrite() + r = runWrite(t, fake, writeSetup{session: cliSession, tty: true, stdin: "y\n"}, "secrets", "rm", "GITHUB_TOKEN") + if r.code != 0 || len(fake.sent("DELETE", path)) != 1 || !strings.Contains(r.stdout, "deleted") { + t.Errorf("answered yes: exit %d %s", r.code, r.msg) + } + fake = newFakeWrite() + fake.failOn("DELETE "+path, 409, `{"error":{"message":"the secret is assigned to automations","type":"invalid_request_error","code":"secret_in_use", + "automations":[{"id":"`+testAutoID+`","name":"PR review api"}]}}`) + r = runWrite(t, fake, writeSetup{session: cliSession}, "secrets", "delete", "GITHUB_TOKEN", "--yes") + if r.code != exitUsage || !strings.Contains(r.msg, "PR review api") { + t.Errorf("in use: exit %d %s", r.code, r.msg) + } +} + +func TestWriteErrorsSayWhatToDo(t *testing.T) { + tokenCred := credential{kind: credEnv, bearer: testPAT} + sessCred := credential{kind: credSession, session: "s"} + for name, tc := range map[string]struct { + cred credential + body string + code int + wants []string + }{ + "scope denied": {tokenCred, `{"error":{"message":"This token lacks the \"secrets:write\" scope.","type":"permission_error","code":"token_scope_denied"}}`, exitAuth, []string{"secrets:write", "Settings > Tokens", "browser"}}, + "session not cli": {sessCred, `{"error":{"message":"x","type":"permission_error","code":"credential_not_allowed"}}`, exitAuth, []string{"nan auth login"}}, + "token not allowed": {tokenCred, `{"error":{"message":"x","type":"permission_error","code":"credential_not_allowed"}}`, exitAuth, []string{"secrets:write", tokenEnvVar}}, + "workspace key": {tokenCred, `{"error":{"message":"x","type":"permission_error","code":"workspace_key_not_allowed"}}`, exitAuth, []string{"workspace key"}}, + "rate limited": {sessCred, `{"error":{"message":"x","type":"rate_limit_exceeded","code":"rate_limited"}}`, exitUnavailable, []string{"try again later"}}, + "name taken": {sessCred, `{"error":{"message":"x","type":"invalid_request_error","code":"name_taken"}}`, exitUsage, []string{"already taken"}}, + "too many": {sessCred, `{"error":{"message":"x","type":"invalid_request_error","code":"too_many_secrets"}}`, exitUsage, []string{"nan secrets rm"}}, + "idempotency reused": {sessCred, `{"error":{"message":"x","type":"invalid_request_error","code":"idempotency_key_reused"}}`, exitUsage, []string{"idempotency key"}}, + "too large": {sessCred, `{"error":{"message":"x","type":"invalid_request_error","code":"secrets_too_large","automations":[{"id":"a","name":"Nightly"}]}}`, exitUsage, []string{"Nightly", "size"}}, + "platform access": {sessCred, `{"error":{"message":"x","type":"permission_error","code":"platform_access_required"}}`, exitAuth, []string{"membership"}}, + "browser only": {sessCred, `{"error":{"message":"x","type":"permission_error","code":"browser_session_required"}}`, exitUsage, []string{"portal", "browser"}}, + "unavailable": {sessCred, `{"error":{"message":"x","type":"api_error","code":"secrets_unavailable"}}`, exitUnavailable, []string{"try again"}}, + "bad param": {sessCred, `{"error":{"message":"base_url must be https","type":"invalid_request_error","param":"base_url","code":"invalid_request"}}`, exitUsage, []string{"base_url"}}, + } { + fake := newFakeWrite() + fake.failOn("POST /v1/secrets", 400, tc.body) + srv := httptest.NewServer(fake.handler(t)) + client := tc.cred.client().WithBaseURL(srv.URL) + _, _, err := client.CreateSecret(t.Context(), apiCreate("X_TOKEN"), "") + srv.Close() + e := writeExit(err, tc.cred, "secrets", scopeSecretsWrite) + if e.Code != tc.code { + t.Errorf("%s: exit %d: %v", name, e.Code, e) + } + for _, w := range tc.wants { + if !strings.Contains(e.Error(), w) { + t.Errorf("%s: %q lacks %q", name, e.Error(), w) + } + } + if strings.Contains(e.Error(), testPAT) || strings.Contains(e.Error(), "\x1b") { + t.Errorf("%s: %q", name, e.Error()) + } + } +} + +func TestAutomationsCreateFromYAML(t *testing.T) { + fake := newFakeWrite() + file := filepath.Join(t.TempDir(), "a.yaml") + _ = os.WriteFile(file, []byte("name: PR review api\nworkspace: "+testWSID+"\nagent: pi\napproval_kinds: []\nsecrets:\n - secret: GITHUB_TOKEN\n env: GITHUB_TOKEN\n exposure: actions\nmax_runs_per_hour: 30\n"), 0o600) + r := runWrite(t, fake, writeSetup{session: cliSession}, "automations", "create", "-f", file) + if r.code != 0 { + t.Fatalf("exit %d: %s", r.code, r.msg) + } + posts := fake.sent("POST", "/v1/automations") + if len(posts) != 1 { + t.Fatalf("%v", fake.requests()) + } + var body map[string]any + _ = json.Unmarshal([]byte(posts[0].Body), &body) + if body["name"] != "PR review api" || body["max_runs_per_hour"] != float64(30) || fmt.Sprint(body["approval_kinds"]) != "[]" { + t.Errorf("body %v", body) + } + if s, _ := body["secrets"].([]any); len(s) != 1 { + t.Errorf("secrets %v", body["secrets"]) + } + wantAll(t, "stdout", r.stdout, "PR review api created", testAutoID) + wantAll(t, "stderr", r.stderr, "applies without asking", "triggers add") + + // JSON from stdin, sent compacted as written; --json prints the answer. + fake = newFakeWrite() + r = runWrite(t, fake, writeSetup{session: cliSession, stdin: "{\n \"name\": \"x\",\n \"approval_kinds\": [\"github_push\"]\n}\n"}, "automations", "create", "-f", "-", "--json") + if r.code != 0 || fake.sent("POST", "/v1/automations")[0].Body != `{"name":"x","approval_kinds":["github_push"]}` { + t.Fatalf("exit %d %s %v", r.code, r.msg, fake.requests()) + } + if !strings.Contains(r.stdout, `"object": "automation"`) || !strings.Contains(r.stdout, `"updated_by": "cli_session"`) { + t.Errorf("--json: %s", r.stdout) + } +} + +func TestReadBodyFile(t *testing.T) { + dir := t.TempDir() + write := func(name, content string) string { + p := filepath.Join(dir, name) + _ = os.WriteFile(p, []byte(content), 0o600) + return p + } + for name, tc := range map[string]struct { + path string + want string + }{ + "list": {write("l.yaml", "- a\n- b\n"), "one object"}, + "scalar": {write("s.yaml", "hello\n"), "one object"}, + "bad json": {write("b.json", "{\"a\": }"), "not valid JSON"}, + "bad yaml": {write("b.yaml", "a: [1,\n"), "neither"}, + "missing": {filepath.Join(dir, "nope.yaml"), "could not read"}, + "no file": {"", "-f FILE"}, + } { + _, err := readBodyFile(tc.path) + if exitCode(err) != exitUsage || !strings.Contains(err.Error(), tc.want) { + t.Errorf("%s: %v", name, err) + } + } + writeStdinTTY = func() bool { return true } + defer func() { writeStdinTTY = func() bool { return isTerminal(os.Stdin) } }() + if _, err := readBodyFile("-"); exitCode(err) != exitUsage || !strings.Contains(err.Error(), "terminal") { + t.Errorf("tty stdin: %v", err) + } +} + +func TestAutomationsEditEnableDisableRm(t *testing.T) { + path := "/v1/automations/" + testAutoID + file := filepath.Join(t.TempDir(), "patch.yaml") + _ = os.WriteFile(file, []byte("approval_kinds: []\n"), 0o600) + for name, tc := range map[string]struct { + args []string + method string + body string + out string + }{ + "edit by name": {[]string{"automations", "edit", "PR review api", "-f", file}, "PATCH", `{"approval_kinds":[]}`, "updated"}, + "enable": {[]string{"automations", "enable", testAutoID}, "PATCH", `{"enabled":true}`, "enabled"}, + "disable": {[]string{"automations", "disable", "PR review api"}, "PATCH", `{"enabled":false}`, "disabled"}, + "rm": {[]string{"automations", "rm", "PR review api", "--yes"}, "DELETE", "", "deleted"}, + } { + fake := newFakeWrite() + r := runWrite(t, fake, writeSetup{session: cliSession}, tc.args...) + if r.code != 0 { + t.Errorf("%s: exit %d: %s", name, r.code, r.msg) + continue + } + got := fake.sent(tc.method, path) + if len(got) != 1 || got[0].Body != tc.body { + t.Errorf("%s: %v", name, fake.requests()) + } + if !strings.Contains(r.stdout, tc.out) { + t.Errorf("%s: %q", name, r.stdout) + } + } + fake := newFakeWrite() + r := runWrite(t, fake, writeSetup{session: cliSession}, "automations", "rm", "PR review api") + if r.code != exitUsage || len(fake.sent("DELETE", path)) != 0 { + t.Errorf("rm without --yes: exit %d", r.code) + } +} + +func TestTriggersAddShowsTheSecretsOnce(t *testing.T) { + file := filepath.Join(t.TempDir(), "t.yaml") + _ = os.WriteFile(file, []byte("kind: webhook\nsignature_scheme: github\nevent_filter:\n events: [pull_request]\n"), 0o600) + fake := newFakeWrite() + r := runWrite(t, fake, writeSetup{session: cliSession}, "automations", "triggers", "add", "PR review api", "-f", file) + if r.code != 0 { + t.Fatalf("exit %d: %s", r.code, r.msg) + } + posts := fake.sent("POST", "/v1/automations/"+testAutoID+"/triggers") + if len(posts) != 1 || posts[0].Body != `{"event_filter":{"events":["pull_request"]},"kind":"webhook","signature_scheme":"github"}` { + t.Fatalf("%v", fake.requests()) + } + checkGolden(t, "writeapi/triggers_add.golden", r.stdout) + for _, s := range []string{"whsec_shown_once", "https://api.nan.builders/hooks/nan_whk_"} { + if n := strings.Count(r.stdout, s); n != 1 { + t.Errorf("%q printed %d times", s, n) + } + if strings.Contains(r.stderr, s) { + t.Errorf("%q on stderr", s) + } + } + wantAll(t, "stderr", r.stderr, "Store these now", "never shown again", "triggers rotate") + assertNothingWritten(t, r.home, "whsec_shown_once") + + fake = newFakeWrite() + r = runWrite(t, fake, writeSetup{session: cliSession}, "automations", "triggers", "add", testAutoID, "-f", file, "--json") + var got map[string]any + if err := json.Unmarshal([]byte(r.stdout), &got); err != nil || got["signing_secret"] != "whsec_shown_once" || got["webhook_url"] == nil || got["trigger"] == nil { + t.Errorf("--json: %v %s", err, r.stdout) + } + wantAll(t, "stderr", r.stderr, "Store these now") +} + +func assertNothingWritten(t *testing.T, home, needle string) { + t.Helper() + _ = filepath.Walk(home, func(path string, info os.FileInfo, err error) error { + if err == nil && !info.IsDir() { + if b, _ := os.ReadFile(path); bytes.Contains(b, []byte(needle)) { + t.Errorf("%s holds %q", path, needle) + } + } + return nil + }) +} + +func TestTriggersRotate(t *testing.T) { + path := "/v1/automations/" + testAutoID + "/triggers/" + triggerUUID + "/rotate" + for name, tc := range map[string]struct { + flags []string + body string + note string + }{ + "default both": {nil, `{"rotate":["url","secret"],"grace":true}`, "24 hours"}, + "secret, no grace": {[]string{"--secret", "--no-grace"}, `{"rotate":["secret"],"grace":false}`, "no longer works"}, + "url": {[]string{"--url"}, `{"rotate":["url"],"grace":true}`, "Store these now"}, + } { + fake := newFakeWrite() + r := runWrite(t, fake, writeSetup{session: cliSession}, append([]string{"automations", "triggers", "rotate", "PR review api", triggerUUID}, tc.flags...)...) + if r.code != 0 { + t.Errorf("%s: exit %d %s", name, r.code, r.msg) + continue + } + if got := fake.sent("POST", path); len(got) != 1 || got[0].Body != tc.body { + t.Errorf("%s: %v", name, fake.requests()) + } + if strings.Count(r.stdout, "new_secret_shown_once") != 1 || !strings.Contains(r.stderr, tc.note) { + t.Errorf("%s: stdout %q stderr %q", name, r.stdout, r.stderr) + } + } +} + +func TestTriggersLsEditRm(t *testing.T) { + fake := newFakeWrite() + r := runWrite(t, fake, writeSetup{session: cliSession}, "automations", "triggers", "ls", "PR review api") + if r.code != 0 { + t.Fatalf("exit %d %s", r.code, r.msg) + } + wantAll(t, "triggers ls", r.stdout, triggerUUID, "webhook github", "CHANGED FROM", "Browser") + + file := filepath.Join(t.TempDir(), "t.json") + _ = os.WriteFile(file, []byte(`{"enabled": false}`), 0o600) + fake = newFakeWrite() + r = runWrite(t, fake, writeSetup{session: cliSession}, "automations", "triggers", "edit", "PR review api", triggerUUID, "-f", file) + path := "/v1/automations/" + testAutoID + "/triggers/" + triggerUUID + if got := fake.sent("PATCH", path); r.code != 0 || len(got) != 1 || got[0].Body != `{"enabled":false}` || !strings.Contains(r.stdout, "disabled") { + t.Errorf("edit: exit %d %s %v %q", r.code, r.msg, fake.requests(), r.stdout) + } + + fake = newFakeWrite() + r = runWrite(t, fake, writeSetup{session: cliSession}, "automations", "triggers", "rm", "PR review api", triggerUUID, "--yes") + if r.code != 0 || len(fake.sent("DELETE", path)) != 1 { + t.Errorf("rm: exit %d %s", r.code, r.msg) + } + + fake = newFakeWrite() + r = runWrite(t, fake, writeSetup{session: cliSession}, "automations", "triggers", "rm", "PR review api", "nan_whk_AbC1", "--yes") + if r.code != exitUsage || len(fake.requests()) != 0 { + t.Errorf("bad trigger id: exit %d %v", r.code, fake.requests()) + } + + other := "c0000000-0000-4000-8000-000000000009" + fake = newFakeWrite() + r = runWrite(t, fake, writeSetup{session: cliSession}, "automations", "triggers", "rm", "PR review api", other, "--yes") + if r.code != exitUsage || !strings.Contains(r.msg, "trigger "+other+" not found") { + t.Errorf("unknown trigger: exit %d %s", r.code, r.msg) + } +} + +func TestAutomationWritesWhenTheServerSaysNo(t *testing.T) { + // A session, write API off: one question, no write. + fake := newFakeWrite() + fake.writeAPI = false + r := runWrite(t, fake, writeSetup{session: cliSession}, "automations", "disable", "PR review api") + if r.code != exitUsage || !strings.Contains(r.msg, "portal") { + t.Errorf("flag off: exit %d %s", r.code, r.msg) + } + if len(fake.sent("PATCH", "/v1/automations/"+testAutoID)) != 0 { + t.Error("wrote with the write API off") + } + + // A token on a Phase 2 platform: browser_session_required. + fake = newFakeWrite() + fake.failOn("PATCH /v1/automations/"+testAutoID, 403, `{"error":{"message":"needs a browser","type":"permission_error","code":"browser_session_required"}}`) + r = runWrite(t, fake, writeSetup{env: testPAT}, "automations", "disable", "PR review api") + if r.code != exitUsage || !strings.Contains(r.msg, "portal") || len(fake.sent("GET", "/api/auth/me")) != 0 { + t.Errorf("phase 2: exit %d %s", r.code, r.msg) + } + + // A token without automations:write. + fake = newFakeWrite() + fake.failOn("PATCH /v1/automations/"+testAutoID, 403, `{"error":{"message":"This token lacks the \"automations:write\" scope.","type":"permission_error","code":"token_scope_denied"}}`) + r = runWrite(t, fake, writeSetup{env: testPAT}, "automations", "enable", "PR review api") + if r.code != exitAuth || !strings.Contains(r.msg, "automations:write") || !strings.Contains(r.msg, "Settings > Tokens") || strings.Contains(r.msg, testPAT) { + t.Errorf("scope: exit %d %s", r.code, r.msg) + } + if got := fake.sent("PATCH", "/v1/automations/"+testAutoID); len(got) != 1 || got[0].Authz != "Bearer "+testPAT { + t.Errorf("token not sent as a bearer: %v", got) + } + + // Rate limited. + fake = newFakeWrite() + fake.failOn("DELETE /v1/automations/"+testAutoID, 429, `{"error":{"message":"Too many","type":"rate_limit_exceeded","code":"rate_limited"}}`) + r = runWrite(t, fake, writeSetup{session: cliSession}, "automations", "rm", "PR review api", "--yes") + if r.code != exitUnavailable || !strings.Contains(r.msg, "automations") { + t.Errorf("rate limited: exit %d %s", r.code, r.msg) + } + + // An expired session is an auth error, not "not available". + fake = newFakeWrite() + fake.failOn("GET /api/auth/me", 401, `{"error":"invalid session"}`) + r = runWrite(t, fake, writeSetup{session: cliSession}, "automations", "enable", "PR review api") + if r.code != exitAuth || !strings.Contains(r.msg, "nan auth login") { + t.Errorf("expired: exit %d %s", r.code, r.msg) + } +} + +func TestShowSaysWhereTheLastChangeCameFrom(t *testing.T) { + resetPhase2Flags() + env, out, _ := goldenEnv(t) + if err := doAutomationsShow(t.Context(), env, testAutoID); err != nil { + t.Fatal(err) + } + wantAll(t, "show", out.String(), "last changed from Browser", "nan automations edit "+testAutoID) +} + +func TestUpdatedByLabel(t *testing.T) { + for in, want := range map[string]string{"browser_session": "Browser", "cli_session": "CLI", "platform_token": "API token", "": "", "x\x1b": "x"} { + if got := updatedByLabel(in); got != want { + t.Errorf("%q: %q", in, got) + } + } +} + +func TestVerbHintsPointAtTheCommand(t *testing.T) { + fakeHome(t, "") + for args, want := range map[string]string{ + "secrets add": "nan secrets set", + "secrets reveal": "nan secrets show", + "automations new": "nan automations create", + "automations rotate": "nan automations triggers rotate", + } { + code, msg := execute(t, strings.Fields(args)...) + if code != exitUsage || !strings.Contains(msg, want) { + t.Errorf("nan %s: exit %d: %s", args, code, msg) + } + } +} + +func apiCreate(name string) api.CreateSecretRequest { + return api.CreateSecretRequest{Name: name, Value: testValue} +} diff --git a/internal/tui/tui.go b/internal/tui/tui.go index 3ee0842..71c30dd 100644 --- a/internal/tui/tui.go +++ b/internal/tui/tui.go @@ -3661,7 +3661,7 @@ func (m model) renderSetup(l layout) string { // ── about renderer ─────────────────────────────────────────────────────────── -const Version = "0.1.32" +const Version = "0.1.33" func renderAbout(l layout) string { var b strings.Builder From 13d02edcb233714beb757e23bde64db53e06f9e2 Mon Sep 17 00:00:00 2001 From: barckcode Date: Wed, 14 Oct 2026 16:03:17 +0200 Subject: [PATCH 3/7] test: the CLI's own create retry, write-only tokens, partial set, not_found, size edges; QA notes - the retry test now forces a fresh connection per request, so the second POST is the CLI's (net/http retries only on a reused one); lostAnswer unit test - a token with secrets:write and no secrets:read creates, and names the scope it lacks - set on an existing secret with --base-url, and a PATCH that fails after the PUT - not_found on PUT/PATCH/DELETE; 16 KiB boundary; non-UTF-8 - a pasted value as a subcommand is not echoed; reads with the write API off say reading - rotate --json prints the store-now note only when something was issued; --json and token tests Co-Authored-By: Claude Opus 5.5 --- cmd/automations_write.go | 4 +- cmd/secrets.go | 9 +- cmd/writeapi.go | 10 +- cmd/writeapi_test.go | 233 ++++++++++++++++++++++++++++++++++++++- 4 files changed, 250 insertions(+), 6 deletions(-) diff --git a/cmd/automations_write.go b/cmd/automations_write.go index b8a8882..3dca1b6 100644 --- a/cmd/automations_write.go +++ b/cmd/automations_write.go @@ -467,7 +467,9 @@ func doTriggersRotate(ctx context.Context, env *runsEnv, ref, triggerID string, if err := printJSON(env.stdout, out, true); err != nil { return err } - fmt.Fprintln(env.stderr, oneTimeNote) + if out.WebhookURL != nil || out.SigningSecret != nil { + fmt.Fprintln(env.stderr, oneTimeNote) + } return nil } printTriggerSecrets(env, out) diff --git a/cmd/secrets.go b/cmd/secrets.go index 0b03492..cec8c34 100644 --- a/cmd/secrets.go +++ b/cmd/secrets.go @@ -79,6 +79,9 @@ Docs: ` + writeDocs + ` and https://nan.builders/docs/runs`, Args: cobra.ArbitraryArgs, RunE: func(cmd *cobra.Command, args []string) error { if len(args) > 0 { + if looksLikeAKey(args[0]) { + return usageErrorf("unknown command for \"nan secrets\". %s", pastedValueHint) + } shown := runs.Truncate(runs.SanitizeLine(args[0]), 64) if hint, ok := secretVerbHints[args[0]]; ok { return usageErrorf("there is no nan secrets %s; try: nan secrets %s", shown, hint) @@ -387,7 +390,11 @@ func listSecrets(ctx context.Context, env *runsEnv) (*api.SecretList, error) { } list, err := env.client.SecretsV1(ctx) if err != nil { - return nil, secretsV1Exit(err, env.cred, scopeSecretsRead) + var apiErr *api.APIError + if errors.As(err, &apiErr) && apiErr.Status == 404 { + return nil, apiOff("reading secrets with a platform token") + } + return nil, writeExit(err, env.cred, "secrets", scopeSecretsRead) } return list, nil } diff --git a/cmd/writeapi.go b/cmd/writeapi.go index b8e0792..be63acd 100644 --- a/cmd/writeapi.go +++ b/cmd/writeapi.go @@ -114,8 +114,14 @@ func withWriteEnv(fn func(*runsEnv) error) error { // writeAPIOff is the answer when the platform does not take writes from // this CLI (yet) for the member: the portal still does them. func writeAPIOff(what string) *ExitError { - return usageErrorf("changing %s from the CLI is not available for your account yet: do it in the portal at %s, "+ - "signed in through the browser. See %s", what, runs.PortalURL, writeDocs) + return apiOff("changing " + what) +} + +// apiOff is writeAPIOff for any action ("reading secrets", "changing +// automations"). +func apiOff(action string) *ExitError { + return usageErrorf("%s from the CLI is not available for your account yet: do it in the portal at %s, "+ + "signed in through the browser. See %s", action, runs.PortalURL, writeDocs) } // requireWriteAPI asks the platform, for a session, whether it takes writes diff --git a/cmd/writeapi_test.go b/cmd/writeapi_test.go index 381c944..0f1c974 100644 --- a/cmd/writeapi_test.go +++ b/cmd/writeapi_test.go @@ -2,6 +2,7 @@ package cmd import ( "bytes" + "context" "encoding/json" "fmt" "io" @@ -93,6 +94,12 @@ func (f *fakeWrite) handler(t *testing.T) http.Handler { defer f.mu.Unlock() f.reqs = append(f.reqs, writeReq{r.Method, r.URL.Path, r.Header.Get("Cookie"), r.Header.Get("Authorization"), r.Header.Get("Idempotency-Key"), string(body)}) w.Header().Set("Content-Type", "application/json") + if f.dropFirstCreate { + // A fresh connection per request: net/http only retries a + // request itself on a reused connection, so what follows a + // dropped answer is the CLI's own retry. + w.Header().Set("Connection", "close") + } route := r.Method + " " + r.URL.Path if fe, ok := f.fail[route]; ok { w.WriteHeader(fe.status) @@ -206,7 +213,22 @@ func (f *fakeWrite) handler(t *testing.T) http.Handler { writeJSON(200, tc.Trigger) case route == "POST /v1/automations/"+testAutoID+"/triggers/"+triggerID+"/rotate": w.Header().Set("Cache-Control", "no-store") - _, _ = w.Write(phase2File(t, "trigger_rotate.json")) + var in struct { + Rotate []string `json:"rotate"` + } + _ = json.Unmarshal(body, &in) + out := map[string]any{} + for _, x := range in.Rotate { + switch x { + case "url": + out["webhook_url"] = "https://api.nan.builders/hooks/nan_whk_ROTATEDROTATEDROTATED" + case "secret": + var rot map[string]any + _ = json.Unmarshal(phase2File(t, "trigger_rotate.json"), &rot) + out["signing_secret"] = rot["signing_secret"] + } + } + writeJSON(200, out) default: writeJSON(404, map[string]any{"error": map[string]any{"message": "no route " + route, "type": "invalid_request_error", "code": "not_found"}}) } @@ -482,6 +504,9 @@ func TestSecretsSetRetriesWithTheSameIdempotencyKey(t *testing.T) { if len(posts) != 2 || posts[0].IdemKey == "" || posts[0].IdemKey != posts[1].IdemKey { t.Fatalf("posts %v", posts) } + if r.stderr != "" && strings.Contains(r.stderr, "could not reach") { + t.Errorf("stderr %q", r.stderr) + } if n := len(fake.secrets); n != 2 { t.Errorf("%d secrets: a retry created a second one", n) } @@ -843,7 +868,14 @@ func TestTriggersRotate(t *testing.T) { if got := fake.sent("POST", path); len(got) != 1 || got[0].Body != tc.body { t.Errorf("%s: %v", name, fake.requests()) } - if strings.Count(r.stdout, "new_secret_shown_once") != 1 || !strings.Contains(r.stderr, tc.note) { + wantSecret := 1 + if strings.Join(tc.flags, " ") == "--url" { + wantSecret = 0 + if strings.Count(r.stdout, "nan_whk_ROTATED") != 1 { + t.Errorf("%s: no new URL: %q", name, r.stdout) + } + } + if strings.Count(r.stdout, "new_secret_shown_once") != wantSecret || !strings.Contains(r.stderr, tc.note) { t.Errorf("%s: stdout %q stderr %q", name, r.stdout, r.stderr) } } @@ -969,3 +1001,200 @@ func TestVerbHintsPointAtTheCommand(t *testing.T) { func apiCreate(name string) api.CreateSecretRequest { return api.CreateSecretRequest{Name: name, Value: testValue} } + +// Without the CLI's own retry, the lost answer is an error: the second POST +// of the test above is this CLI's, not net/http's. +func TestSecretsSetWithoutRetryFailsOnALostAnswer(t *testing.T) { + fake := newFakeWrite() + fake.dropFirstCreate = true + file := writeValueFile(t, testValue) + createAttempts = 1 + r := runWrite(t, fake, writeSetup{session: cliSession}, "secrets", "set", "NEW_TOKEN", "--value-file", file) + if r.code == 0 || len(fake.sent("POST", "/v1/secrets")) != 1 { + t.Fatalf("exit %d, posts %v", r.code, fake.sent("POST", "/v1/secrets")) + } + noLeak(t, r, fake) +} + +func TestLostAnswer(t *testing.T) { + for name, tc := range map[string]struct { + err error + want bool + }{ + "network": {fmt.Errorf("could not reach nan.builders: %w", io.ErrUnexpectedEOF), true}, + "refusal": {&api.APIError{Status: 409, Code: "name_taken"}, false}, + "5xx": {&api.APIError{Status: 502}, false}, + "expired": {api.ErrSessionExpired, false}, + "cancelled": {fmt.Errorf("x: %w", context.Canceled), false}, + } { + if got := lostAnswer(tc.err); got != tc.want { + t.Errorf("%s: %v", name, got) + } + } +} + +const readScopeDenied = `{"error":{"message":"This token lacks the \"secrets:read\" scope.","type":"permission_error","code":"token_scope_denied"}}` + +// A token with secrets:write and not secrets:read cannot look the name up: +// it still creates, and says what it needs when the name exists. +func TestSecretsSetWithAWriteOnlyToken(t *testing.T) { + file := writeValueFile(t, testValue) + fake := newFakeWrite() + fake.failOn("GET /v1/secrets", 403, readScopeDenied) + r := runWrite(t, fake, writeSetup{env: testPAT}, "secrets", "set", "NEW_TOKEN", "--value-file", file) + if r.code != 0 || len(fake.sent("POST", "/v1/secrets")) != 1 { + t.Fatalf("exit %d %s %v", r.code, r.msg, fake.requests()) + } + if got := fake.sent("POST", "/v1/secrets")[0]; got.Authz != "Bearer "+testPAT || len(fake.sent("GET", "/api/auth/me")) != 0 { + t.Errorf("token request %v", fake.requests()) + } + + fake = newFakeWrite() + fake.failOn("GET /v1/secrets", 403, readScopeDenied) + fake.failOn("POST /v1/secrets", 409, `{"error":{"message":"taken","type":"invalid_request_error","param":"name","code":"name_taken"}}`) + r = runWrite(t, fake, writeSetup{env: testPAT}, "secrets", "set", "GITHUB_TOKEN", "--value-file", file) + if r.code != exitUsage || !strings.Contains(r.msg, "secrets:read") || !strings.Contains(r.msg, "GITHUB_TOKEN already exists") || strings.Contains(r.msg, testPAT) { + t.Errorf("exit %d %s", r.code, r.msg) + } + noLeak(t, r, fake) +} + +func TestSecretsSetExistingWithBaseURLAndAFailedPatch(t *testing.T) { + const path = "/v1/secrets/5e000000-0000-4000-8000-000000000001" + file := writeValueFile(t, testValue) + fake := newFakeWrite() + r := runWrite(t, fake, writeSetup{session: cliSession}, "secrets", "set", "GITHUB_TOKEN", "--value-file", file, "--base-url", "https://x.example/v1") + if p := fake.sent("PATCH", path); r.code != 0 || len(p) != 1 || p[0].Body != `{"base_url":"https://x.example/v1"}` { + t.Errorf("exit %d %s %v", r.code, r.msg, p) + } + + fake = newFakeWrite() + fake.failOn("PATCH "+path, 400, `{"error":{"message":"base_url must be an https URL","type":"invalid_request_error","param":"base_url","code":"invalid_request"}}`) + r = runWrite(t, fake, writeSetup{session: cliSession}, "secrets", "set", "GITHUB_TOKEN", "--value-file", file, "--base-url", "http://x") + if r.code != exitUsage || !strings.Contains(r.msg, "base_url") || len(fake.sent("PUT", path+"/value")) != 1 { + t.Errorf("exit %d %s", r.code, r.msg) + } + wantAll(t, "stderr", r.stderr, "the value was replaced, but not the description or base URL") + noLeak(t, r, fake) +} + +// A secret deleted between the lookup and the write. +func TestSecretWriteNotFound(t *testing.T) { + const path = "/v1/secrets/5e000000-0000-4000-8000-000000000001" + notFound := `{"error":{"message":"Not found","type":"invalid_request_error","code":"not_found"}}` + file := writeValueFile(t, testValue) + for name, tc := range map[string]struct { + route string + args []string + }{ + "set": {"PUT " + path + "/value", []string{"secrets", "set", "GITHUB_TOKEN", "--value-file", file}}, + "edit": {"PATCH " + path, []string{"secrets", "edit", "GITHUB_TOKEN", "--description", "x"}}, + "rm": {"DELETE " + path, []string{"secrets", "rm", "GITHUB_TOKEN", "--yes"}}, + } { + fake := newFakeWrite() + fake.failOn(tc.route, 404, notFound) + r := runWrite(t, fake, writeSetup{session: cliSession}, tc.args...) + if r.code != exitUsage || !strings.Contains(r.msg, "not found") { + t.Errorf("%s: exit %d %s", name, r.code, r.msg) + } + } +} + +func TestSecretValueSizeLimit(t *testing.T) { + defer func() { writeStdin, writeStdinTTY = os.Stdin, func() bool { return isTerminal(os.Stdin) } }() + writeStdinTTY = func() bool { return false } + writeStdin = strings.NewReader(strings.Repeat("a", maxSecretValue) + "\n") + if v, err := readSecretValue("", true); err != nil || len(v) != maxSecretValue { + t.Errorf("16 KiB: %d %v", len(v), err) + } + writeStdin = strings.NewReader(strings.Repeat("a", maxSecretValue) + "\r\n") + if v, err := readSecretValue("", true); err != nil || len(v) != maxSecretValue { + t.Errorf("16 KiB + CRLF: %d %v", len(v), err) + } + writeStdin = strings.NewReader(strings.Repeat("a", maxSecretValue+1)) + if _, err := readSecretValue("", true); exitCode(err) != exitUsage || !strings.Contains(err.Error(), "16 KiB") { + t.Errorf("16 KiB + 1: %v", err) + } + writeStdin = strings.NewReader("\xff\xfe") + if _, err := readSecretValue("", true); exitCode(err) != exitUsage || strings.Contains(err.Error(), "\xff") { + t.Errorf("not UTF-8: %v", err) + } +} + +func TestPastedValueIsNotEchoedAsACommand(t *testing.T) { + fakeHome(t, "") + code, msg := execute(t, "secrets", testValue) + if code != exitUsage || strings.Contains(msg, "TESTVALUE") || !strings.Contains(msg, "shell history") { + t.Errorf("exit %d: %s", code, msg) + } +} + +func TestSecretsReadWithTheWriteAPIOff(t *testing.T) { + fake := newFakeWrite() + fake.writeAPI = false + r := runWrite(t, fake, writeSetup{env: testPAT}, "secrets", "ls") + if r.code != exitUsage || !strings.Contains(r.msg, "reading secrets") { + t.Errorf("exit %d %s", r.code, r.msg) + } +} + +func TestTriggersJSONAndTokens(t *testing.T) { + fake := newFakeWrite() + r := runWrite(t, fake, writeSetup{env: testPAT}, "automations", "triggers", "ls", "PR review api", "--json") + var list map[string]any + if err := json.Unmarshal([]byte(r.stdout), &list); r.code != 0 || err != nil || list["data"] == nil { + t.Errorf("ls --json: exit %d %v %s", r.code, err, r.stdout) + } + + file := filepath.Join(t.TempDir(), "t.yaml") + _ = os.WriteFile(file, []byte("kind: webhook\nsignature_scheme: github\n"), 0o600) + fake = newFakeWrite() + r = runWrite(t, fake, writeSetup{env: testPAT}, "automations", "triggers", "add", testAutoID, "-f", file) + if got := fake.sent("POST", "/v1/automations/"+testAutoID+"/triggers"); r.code != 0 || len(got) != 1 || got[0].Authz != "Bearer "+testPAT { + t.Errorf("add with a token: exit %d %s %v", r.code, r.msg, fake.requests()) + } + if strings.Count(r.stdout, "whsec_shown_once") != 1 { + t.Errorf("add with a token: %q", r.stdout) + } + + fake = newFakeWrite() + r = runWrite(t, fake, writeSetup{env: testPAT}, "automations", "triggers", "rotate", testAutoID, triggerUUID, "--secret", "--json") + var rot map[string]any + if err := json.Unmarshal([]byte(r.stdout), &rot); r.code != 0 || err != nil || rot["signing_secret"] != "new_secret_shown_once" || rot["webhook_url"] != nil { + t.Errorf("rotate --json: exit %d %v %s", r.code, err, r.stdout) + } + wantAll(t, "stderr", r.stderr, "Store these now") +} + +func TestWriteArgumentErrors(t *testing.T) { + for _, args := range [][]string{ + {"secrets", "show"}, + {"secrets", "rm", "A_TOKEN", "B_TOKEN"}, + {"secrets", "set"}, + {"secrets", "edit", "lower"}, + {"automations", "triggers", "rm", "PR review api"}, + {"automations", "triggers", "rm", "PR review api", triggerUUID, "extra"}, + {"automations", "create", "-f", ""}, + {"automations", "triggers", "nope"}, + } { + fake := newFakeWrite() + r := runWrite(t, fake, writeSetup{session: cliSession}, args...) + if r.code != exitUsage || len(fake.requests()) != 0 { + t.Errorf("nan %s: exit %d %s %v", strings.Join(args, " "), r.code, r.msg, fake.requests()) + } + } +} + +func TestWriteTokenFileAndBadEnv(t *testing.T) { + path := writeTokenFile(t, testPAT+"\n", 0o600) + fake := newFakeWrite() + r := runWrite(t, fake, writeSetup{session: cliSession}, "secrets", "ls", "--token-file", path) + if got := fake.sent("GET", "/v1/secrets"); r.code != 0 || len(got) != 1 || got[0].Authz != "Bearer "+testPAT || got[0].Cookie != "" { + t.Errorf("--token-file wins over the session: exit %d %s %v", r.code, r.msg, fake.requests()) + } + fake = newFakeWrite() + r = runWrite(t, fake, writeSetup{env: "not a token"}, "secrets", "ls") + if r.code != exitAuth || len(fake.requests()) != 0 || strings.Contains(r.msg, "not a token") { + t.Errorf("bad NAN_TOKEN: exit %d %s", r.code, r.msg) + } +} From 18feb7698ac0e025148b5cebdc9c2bdb0ca7b91a Mon Sep 17 00:00:00 2001 From: barckcode Date: Wed, 14 Oct 2026 16:06:28 +0200 Subject: [PATCH 4/7] fix(secrets): provider is gemini, not google; refuse unknown providers before reading the value Co-Authored-By: Claude Opus 5.5 --- cmd/secrets.go | 18 ++++++++++++++++-- cmd/writeapi_test.go | 2 ++ 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/cmd/secrets.go b/cmd/secrets.go index cec8c34..2e0a9a6 100644 --- a/cmd/secrets.go +++ b/cmd/secrets.go @@ -211,7 +211,7 @@ func init() { f.StringVar(&secretSetValueFile, "value-file", "", "Read the value from `PATH`") f.BoolVar(&secretSetValueStdin, "value-stdin", false, "Read the value from stdin (a pipe, never a terminal)") f.StringVar(&secretSetKind, "kind", "", "generic (default) or provider_key, for a new secret") - f.StringVar(&secretSetProvider, "provider", "", "anthropic, openai, google or openai_compatible (implies --kind provider_key)") + f.StringVar(&secretSetProvider, "provider", "", "anthropic, openai, gemini or openai_compatible (implies --kind provider_key)") f.StringVar(&secretSetBaseURL, "base-url", "", "The endpoint of an openai_compatible provider key") f.StringVar(&secretSetDescription, "description", "", "What the secret is for (never the value)") f.BoolVar(&secretSetJSON, "json", false, "Print the secret's metadata as JSON") @@ -304,7 +304,14 @@ func buildSecretSet(cmd *cobra.Command, name string) (*secretSet, error) { return nil, usageErrorf("--provider is for --kind provider_key") } if in.kind == api.SecretKindProviderKey && in.provider == "" { - return nil, usageErrorf("--kind provider_key needs --provider: anthropic, openai, google or openai_compatible") + return nil, usageErrorf("--kind provider_key needs --provider: anthropic, openai, gemini or openai_compatible") + } + if in.provider != "" && !secretProviders[in.provider] { + hint := "" + if in.provider == "google" { + hint = " (Google's API is gemini)" + } + return nil, usageErrorf("--provider is anthropic, openai, gemini or openai_compatible%s", hint) } value, err := readSecretValue(secretSetValueFile, secretSetValueStdin) if err != nil { @@ -314,6 +321,13 @@ func buildSecretSet(cmd *cobra.Command, name string) (*secretSet, error) { return in, nil } +// secretProviders are the provider names cloud-api accepts on a provider_key +// secret (internal/secrets/service.go). Anything else would be a 400 after the +// value was already read and sent. +var secretProviders = map[string]bool{ + "anthropic": true, "openai": true, "gemini": true, "openai_compatible": true, +} + // readSecretValue reads the value from --value-file or a piped stdin, and // drops one trailing newline. Errors never include the value. func readSecretValue(path string, fromStdin bool) (string, error) { diff --git a/cmd/writeapi_test.go b/cmd/writeapi_test.go index 0f1c974..a52fc92 100644 --- a/cmd/writeapi_test.go +++ b/cmd/writeapi_test.go @@ -436,6 +436,8 @@ func TestSecretsSetRefusals(t *testing.T) { "empty value": {writeSetup{session: cliSession, stdin: "\n"}, []string{"secrets", "set", "X_TOKEN", "--value-stdin"}, []string{"empty"}}, "unknown kind": {writeSetup{session: cliSession}, []string{"secrets", "set", "X_TOKEN", "--value-file", file, "--kind", "other"}, []string{"generic or provider_key"}}, "provider_key no prov": {writeSetup{session: cliSession}, []string{"secrets", "set", "X_TOKEN", "--value-file", file, "--kind", "provider_key"}, []string{"--provider"}}, + "provider google": {writeSetup{session: cliSession}, []string{"secrets", "set", "X_TOKEN", "--value-file", file, "--provider", "google"}, []string{"gemini"}}, + "provider unknown": {writeSetup{session: cliSession}, []string{"secrets", "set", "X_TOKEN", "--value-file", file, "--provider", "mistral"}, []string{"anthropic, openai, gemini or openai_compatible"}}, "provider with generic": {writeSetup{session: cliSession}, []string{"secrets", "set", "X_TOKEN", "--value-file", file, "--kind", "generic", "--provider", "openai"}, []string{"provider_key"}}, "missing file": {writeSetup{session: cliSession}, []string{"secrets", "set", "X_TOKEN", "--value-file", "/nonexistent/v"}, []string{"could not read"}}, } { From d5a1dbe711bd4c08568c04d7b8a85ea60555f689 Mon Sep 17 00:00:00 2001 From: barckcode Date: Wed, 14 Oct 2026 16:07:39 +0200 Subject: [PATCH 5/7] test(secrets): every accepted provider reaches the create body; the google hint is asserted Co-Authored-By: Claude Opus 5.5 --- cmd/secrets.go | 2 +- cmd/writeapi_test.go | 19 ++++++++++++++++++- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/cmd/secrets.go b/cmd/secrets.go index 2e0a9a6..b9e46cc 100644 --- a/cmd/secrets.go +++ b/cmd/secrets.go @@ -322,7 +322,7 @@ func buildSecretSet(cmd *cobra.Command, name string) (*secretSet, error) { } // secretProviders are the provider names cloud-api accepts on a provider_key -// secret (internal/secrets/service.go). Anything else would be a 400 after the +// secret (protocol.Providers in cloud-api's internal/runs/protocol/byok.go). Anything else would be a 400 after the // value was already read and sent. var secretProviders = map[string]bool{ "anthropic": true, "openai": true, "gemini": true, "openai_compatible": true, diff --git a/cmd/writeapi_test.go b/cmd/writeapi_test.go index a52fc92..d9eb31c 100644 --- a/cmd/writeapi_test.go +++ b/cmd/writeapi_test.go @@ -436,7 +436,7 @@ func TestSecretsSetRefusals(t *testing.T) { "empty value": {writeSetup{session: cliSession, stdin: "\n"}, []string{"secrets", "set", "X_TOKEN", "--value-stdin"}, []string{"empty"}}, "unknown kind": {writeSetup{session: cliSession}, []string{"secrets", "set", "X_TOKEN", "--value-file", file, "--kind", "other"}, []string{"generic or provider_key"}}, "provider_key no prov": {writeSetup{session: cliSession}, []string{"secrets", "set", "X_TOKEN", "--value-file", file, "--kind", "provider_key"}, []string{"--provider"}}, - "provider google": {writeSetup{session: cliSession}, []string{"secrets", "set", "X_TOKEN", "--value-file", file, "--provider", "google"}, []string{"gemini"}}, + "provider google": {writeSetup{session: cliSession}, []string{"secrets", "set", "X_TOKEN", "--value-file", file, "--provider", "google"}, []string{"Google's API is gemini"}}, "provider unknown": {writeSetup{session: cliSession}, []string{"secrets", "set", "X_TOKEN", "--value-file", file, "--provider", "mistral"}, []string{"anthropic, openai, gemini or openai_compatible"}}, "provider with generic": {writeSetup{session: cliSession}, []string{"secrets", "set", "X_TOKEN", "--value-file", file, "--kind", "generic", "--provider", "openai"}, []string{"provider_key"}}, "missing file": {writeSetup{session: cliSession}, []string{"secrets", "set", "X_TOKEN", "--value-file", "/nonexistent/v"}, []string{"could not read"}}, @@ -492,6 +492,23 @@ func TestSecretsSetProviderKey(t *testing.T) { } } +// Every provider cloud-api accepts goes through as sent. +func TestSecretsSetEachProvider(t *testing.T) { + file := writeValueFile(t, testValue) + for _, p := range []string{"anthropic", "openai", "gemini"} { + fake := newFakeWrite() + r := runWrite(t, fake, writeSetup{session: cliSession}, "secrets", "set", "LLM_KEY", "--value-file", file, "--provider", p) + if r.code != 0 { + t.Fatalf("%s: exit %d: %s", p, r.code, r.msg) + } + var body map[string]any + _ = json.Unmarshal([]byte(fake.sent("POST", "/v1/secrets")[0].Body), &body) + if body["kind"] != "provider_key" || body["provider"] != p { + t.Errorf("%s: body %v", p, body) + } + } +} + // A create whose answer is lost is sent again with the same key, and the // platform answers the secret the first one created. func TestSecretsSetRetriesWithTheSameIdempotencyKey(t *testing.T) { From dc0714e7781909d4ff0cba6699984a551d185031 Mon Sep 17 00:00:00 2001 From: barckcode Date: Wed, 14 Oct 2026 16:12:06 +0200 Subject: [PATCH 6/7] feat(triggers): rotate a Stripe webhook's signing secret with --signing-secret-file/--signing-secret-stdin Stripe issues the whsec_, so cloud-api requires it in the rotate body and the CLI had no way to send it: the default rotate and --secret always failed for Stripe. The secret is read like a secret value (file or pipe, never a terminal or an argument), sent once, never printed, and a 400 on signing_secret now explains what to do. Co-Authored-By: Claude Opus 5.5 --- README.md | 7 ++- cmd/automations_write.go | 59 +++++++++++++++++++------ cmd/secrets.go | 25 ++++++++--- cmd/writeapi_test.go | 93 +++++++++++++++++++++++++++++++++++++++- internal/api/write.go | 19 ++++++++ 5 files changed, 184 insertions(+), 19 deletions(-) diff --git a/README.md b/README.md index 18705d2..dc867d9 100644 --- a/README.md +++ b/README.md @@ -186,7 +186,8 @@ nan automations rm [--yes] nan automations triggers ls [--json] nan automations triggers add -f trigger.yaml [--json] # webhook URL + secret shown ONCE nan automations triggers edit -f changes.yaml -nan automations triggers rotate [--url] [--secret] [--no-grace] [--json] +nan automations triggers rotate [--url] [--secret] [--no-grace] + [--signing-secret-file PATH | --signing-secret-stdin] [--json] nan automations triggers rm [--yes] nan automations run [--input @payload.json | --input - | --input JSON] [--detach] [--json] nan automations deliveries [--limit 50] [--json] @@ -203,6 +204,10 @@ A secret's value is never an argument (it would stay in your shell history): it comes from `--value-file` or a pipe into `--value-stdin`, and is never shown again. A webhook's URL and signing secret are printed once, to stdout, when the trigger is added or rotated: store them in the sender right away. +A Stripe webhook is the exception: Stripe issues its signing secret, so it +goes in the `-f` file when you add the trigger (pipe the file with `-f -`), +and on rotate you roll it in the Stripe dashboard and pass the new `whsec_` +with `--signing-secret-file` or `--signing-secret-stdin`. Writes need your session from `nan auth login`, or a platform token with the scope of what it does: `secrets:read` to list secrets, `secrets:write` to diff --git a/cmd/automations_write.go b/cmd/automations_write.go index 3dca1b6..b0478f7 100644 --- a/cmd/automations_write.go +++ b/cmd/automations_write.go @@ -18,14 +18,16 @@ import ( // every field, exactly as for the portal. var ( - autoWriteFile string - autoWriteJSON bool - autoRmYes bool - trigLsJSON bool - trigRmYes bool - trigRotateURL bool - trigRotateSec bool - trigRotateHard bool + autoWriteFile string + autoWriteJSON bool + autoRmYes bool + trigLsJSON bool + trigRmYes bool + trigRotateURL bool + trigRotateSec bool + trigRotateHard bool + trigRotateWhsecFile string + trigRotateWhsecStdin bool ) const automationFileHelp = `The file is YAML or JSON (- reads it from stdin) with the fields of the @@ -139,6 +141,8 @@ Docs: ` + writeDocs, }, } +var signingSecretFlags = secretInputFlags{"--signing-secret-file", "--signing-secret-stdin", "pbpaste | nan automations triggers rotate NAME ID --signing-secret-stdin"} + const oneTimeNote = "Store these now: they are never shown again. Lost them? Issue new ones with: nan automations triggers rotate" var automationsTriggersCmd = &cobra.Command{ @@ -229,7 +233,7 @@ Docs: https://nan.builders/docs/webhooks`, } var triggersRotateCmd = &cobra.Command{ - Use: "rotate [--url] [--secret] [--no-grace]", + Use: "rotate [--url] [--secret] [--no-grace] [--signing-secret-file PATH | --signing-secret-stdin]", Short: "Issue a new webhook URL and/or signing secret, shown once", Long: `Issue a new webhook URL, a new signing secret, or both (the default). The new values are printed once, to stdout. ` + oneTimeNote + `. @@ -238,17 +242,33 @@ With --secret, the previous signing secret keeps working for 24 hours so the sender can be updated; --no-grace ends it at once (for a leaked secret). A new URL replaces the old one at once. +A Stripe webhook's signing secret is issued by Stripe, not by NaN: roll it +in the Stripe dashboard, then hand the new whsec_ over with +--signing-secret-file PATH or --signing-secret-stdin (a pipe, never typed +or passed as an argument). It replaces the stored one and is not printed. + Docs: https://nan.builders/docs/webhooks`, Args: automationAndTrigger, Annotations: sysexits, RunE: func(cmd *cobra.Command, args []string) error { in := api.RotateRequest{Grace: !trigRotateHard} - if trigRotateURL || !trigRotateSec { + rotateSecret := trigRotateSec || trigRotateWhsecFile != "" || trigRotateWhsecStdin + if trigRotateURL || !rotateSecret { in.Rotate = append(in.Rotate, "url") } - if trigRotateSec || !trigRotateURL { + if rotateSecret || !trigRotateURL { in.Rotate = append(in.Rotate, "secret") } + if trigRotateWhsecFile != "" || trigRotateWhsecStdin { + if trigRotateWhsecFile != "" && trigRotateWhsecStdin { + return usageErrorf("give --signing-secret-file or --signing-secret-stdin, not both") + } + whsec, err := readSecretInput(signingSecretFlags, trigRotateWhsecFile, trigRotateWhsecStdin) + if err != nil { + return err + } + in.SigningSecret = &whsec + } return withWriteEnv(func(env *runsEnv) error { return doTriggersRotate(cmd.Context(), env, args[0], args[1], in) }) }, } @@ -282,6 +302,8 @@ func init() { triggersRotateCmd.Flags().BoolVar(&trigRotateURL, "url", false, "Rotate the webhook URL") triggersRotateCmd.Flags().BoolVar(&trigRotateSec, "secret", false, "Rotate the signing secret") triggersRotateCmd.Flags().BoolVar(&trigRotateHard, "no-grace", false, "End the previous signing secret at once (a leaked secret)") + triggersRotateCmd.Flags().StringVar(&trigRotateWhsecFile, "signing-secret-file", "", "Stripe only: read the new whsec_ from this file") + triggersRotateCmd.Flags().BoolVar(&trigRotateWhsecStdin, "signing-secret-stdin", false, "Stripe only: read the new whsec_ from a pipe") } func automationAndTrigger(cmd *cobra.Command, args []string) error { @@ -461,6 +483,16 @@ func doTriggersRotate(ctx context.Context, env *runsEnv, ref, triggerID string, } out, err := env.client.RotateTrigger(ctx, id, triggerID, in) if err != nil { + var apiErr *api.APIError + if errors.As(err, &apiErr) && apiErr.Status == 400 && apiErr.Param == "signing_secret" { + if in.SigningSecret == nil { + return usageErrorf("this is a Stripe webhook: its signing secret comes from the Stripe dashboard. " + + "Roll it there and pass the new whsec_ with --signing-secret-file PATH or --signing-secret-stdin, " + + "or rotate only the URL with --url") + } + return usageErrorf("the server refused the signing secret: %s. NaN generates the signing secret of GitHub and NaN webhooks; "+ + "--signing-secret-file and --signing-secret-stdin are for Stripe's whsec_", runs.Truncate(runs.SanitizeLine(apiErr.Message), 200)) + } return triggerWriteExit(err, ref, triggerID, env.cred) } if autoWriteJSON { @@ -473,7 +505,10 @@ func doTriggersRotate(ctx context.Context, env *runsEnv, ref, triggerID string, return nil } printTriggerSecrets(env, out) - if out.SigningSecret != nil { + if in.SigningSecret != nil { + fmt.Fprintln(env.stderr, "the signing secret you gave (from Stripe) is now the one checked") + } + if out.SigningSecret != nil || in.SigningSecret != nil { if in.Grace { fmt.Fprintln(env.stderr, "the previous signing secret keeps working for 24 hours") } else { diff --git a/cmd/secrets.go b/cmd/secrets.go index b9e46cc..dba08a2 100644 --- a/cmd/secrets.go +++ b/cmd/secrets.go @@ -337,14 +337,29 @@ func readSecretValue(path string, fromStdin bool) (string, error) { case path == "" && !fromStdin: return "", usageErrorf("the value comes from --value-file PATH or --value-stdin (a pipe), never from the command line") } + return readSecretInput(secretValueFlags, path, fromStdin) +} + +// secretInputFlags names the pair of flags a secret is read through, for +// the messages of readSecretInput. +type secretInputFlags struct { + file, stdin, example string +} + +var secretValueFlags = secretInputFlags{"--value-file", "--value-stdin", "op read ... | nan secrets set NAME --value-stdin"} + +// readSecretInput reads a secret from the file at path or from a piped +// stdin (never a terminal) and drops one trailing newline. Errors never +// include what was read. +func readSecretInput(flags secretInputFlags, path string, fromStdin bool) (string, error) { var ( raw []byte err error ) if fromStdin { if writeStdinTTY() { - return "", usageErrorf("--value-stdin reads a piped value, and stdin is a terminal, where typing it would show it on screen: " + - "pipe it (op read ... | nan secrets set NAME --value-stdin) or use --value-file PATH") + return "", usageErrorf("%s reads a piped value, and stdin is a terminal, where typing it would show it on screen: "+ + "pipe it (%s) or use %s PATH", flags.stdin, flags.example, flags.file) } raw, err = io.ReadAll(io.LimitReader(writeStdin, maxSecretValue+3)) if err != nil { @@ -354,12 +369,12 @@ func readSecretValue(path string, fromStdin bool) (string, error) { shown := runs.SanitizeLine(path) f, openErr := os.Open(path) if openErr != nil { - return "", usageErrorf("could not read --value-file %s: %v", shown, unwrapPath(openErr)) + return "", usageErrorf("could not read %s %s: %v", flags.file, shown, unwrapPath(openErr)) } defer f.Close() if info, statErr := f.Stat(); statErr == nil { if info.IsDir() { - return "", usageErrorf("--value-file %s is a directory", shown) + return "", usageErrorf("%s %s is a directory", flags.file, shown) } if runtime.GOOS != "windows" && info.Mode().IsRegular() && info.Mode().Perm()&0o077 != 0 { fmt.Fprintf(writeStderr, "note: %s can be read by other users of this machine (chmod 600 %s)\n", shown, shown) @@ -367,7 +382,7 @@ func readSecretValue(path string, fromStdin bool) (string, error) { } raw, err = io.ReadAll(io.LimitReader(f, maxSecretValue+3)) if err != nil { - return "", usageErrorf("could not read --value-file %s: %v", shown, unwrapPath(err)) + return "", usageErrorf("could not read %s %s: %v", flags.file, shown, unwrapPath(err)) } } value := string(raw) diff --git a/cmd/writeapi_test.go b/cmd/writeapi_test.go index d9eb31c..70f331e 100644 --- a/cmd/writeapi_test.go +++ b/cmd/writeapi_test.go @@ -214,7 +214,8 @@ func (f *fakeWrite) handler(t *testing.T) http.Handler { case route == "POST /v1/automations/"+testAutoID+"/triggers/"+triggerID+"/rotate": w.Header().Set("Cache-Control", "no-store") var in struct { - Rotate []string `json:"rotate"` + Rotate []string `json:"rotate"` + SigningSecret *string `json:"signing_secret"` } _ = json.Unmarshal(body, &in) out := map[string]any{} @@ -223,6 +224,9 @@ func (f *fakeWrite) handler(t *testing.T) http.Handler { case "url": out["webhook_url"] = "https://api.nan.builders/hooks/nan_whk_ROTATEDROTATEDROTATED" case "secret": + if in.SigningSecret != nil { + continue // Stripe's whsec_ is stored, never echoed + } var rot map[string]any _ = json.Unmarshal(phase2File(t, "trigger_rotate.json"), &rot) out["signing_secret"] = rot["signing_secret"] @@ -1217,3 +1221,90 @@ func TestWriteTokenFileAndBadEnv(t *testing.T) { t.Errorf("bad NAN_TOKEN: exit %d %s", r.code, r.msg) } } + +const testWhsec = "whsec_STRIPEROLLEDSECRET" + +// A Stripe trigger's new signing secret comes from Stripe: read from a file +// or a pipe, sent once, never printed or stored by the CLI. +func TestTriggersRotateStripeSigningSecret(t *testing.T) { + path := "/v1/automations/" + testAutoID + "/triggers/" + triggerUUID + "/rotate" + file := writeValueFile(t, testWhsec) + for name, tc := range map[string]struct { + setup writeSetup + flags []string + body string + }{ + "file": {writeSetup{session: cliSession}, []string{"--signing-secret-file", file}, `{"rotate":["secret"],"grace":true,"signing_secret":"` + testWhsec + `"}`}, + "stdin": {writeSetup{session: cliSession, stdin: testWhsec + "\n"}, []string{"--signing-secret-stdin", "--no-grace"}, `{"rotate":["secret"],"grace":false,"signing_secret":"` + testWhsec + `"}`}, + "url and file": {writeSetup{session: cliSession}, []string{"--url", "--signing-secret-file", file}, `{"rotate":["url","secret"],"grace":true,"signing_secret":"` + testWhsec + `"}`}, + } { + fake := newFakeWrite() + r := runWrite(t, fake, tc.setup, append([]string{"automations", "triggers", "rotate", "PR review api", triggerUUID}, tc.flags...)...) + if r.code != 0 { + t.Errorf("%s: exit %d %s", name, r.code, r.msg) + continue + } + if got := fake.sent("POST", path); len(got) != 1 || got[0].Body != tc.body { + t.Errorf("%s: %v", name, fake.requests()) + } + if !strings.Contains(r.stderr, "from Stripe") { + t.Errorf("%s: stderr %q", name, r.stderr) + } + for what, text := range map[string]string{"stdout": r.stdout, "stderr": r.stderr, "message": r.msg} { + if strings.Contains(text, testWhsec) { + t.Errorf("%s: %s echoes the signing secret", name, what) + } + } + assertNothingWritten(t, r.home, testWhsec) + } +} + +func TestTriggersRotateStripeRefusals(t *testing.T) { + path := "/v1/automations/" + testAutoID + "/triggers/" + triggerUUID + "/rotate" + file := writeValueFile(t, testWhsec) + stripe400 := `{"error":{"message":"paste the endpoint's signing secret from the Stripe dashboard (whsec_…)","type":"invalid_request_error","param":"signing_secret","code":"invalid_request"}}` + generated400 := `{"error":{"message":"NaN generates the signing secret for this scheme","type":"invalid_request_error","param":"signing_secret","code":"invalid_request"}}` + for name, tc := range map[string]struct { + setup writeSetup + flags []string + fail string + sends int + wants []string + }{ + "stripe without the secret": {writeSetup{session: cliSession}, nil, stripe400, 1, []string{"Stripe dashboard", "--signing-secret-file", "--url"}}, + "secret on a github hook": {writeSetup{session: cliSession}, []string{"--signing-secret-file", file}, generated400, 1, []string{"NaN generates", "Stripe"}}, + "both sources": {writeSetup{session: cliSession}, []string{"--signing-secret-file", file, "--signing-secret-stdin"}, "", 0, []string{"not both"}}, + "terminal": {writeSetup{session: cliSession, tty: true}, []string{"--signing-secret-stdin"}, "", 0, []string{"--signing-secret-stdin", "terminal", "--signing-secret-file"}}, + "empty": {writeSetup{session: cliSession, stdin: "\n"}, []string{"--signing-secret-stdin"}, "", 0, []string{"empty"}}, + } { + fake := newFakeWrite() + if tc.fail != "" { + fake.failOn("POST "+path, 400, tc.fail) + } + r := runWrite(t, fake, tc.setup, append([]string{"automations", "triggers", "rotate", "PR review api", triggerUUID}, tc.flags...)...) + if r.code != exitUsage { + t.Errorf("%s: exit %d %s", name, r.code, r.msg) + } + for _, w := range tc.wants { + if !strings.Contains(r.msg, w) { + t.Errorf("%s: message lacks %q: %s", name, w, r.msg) + } + } + if got := fake.sent("POST", path); len(got) != tc.sends { + t.Errorf("%s: sent %d rotate requests", name, len(got)) + } + if strings.Contains(r.stdout+r.stderr+r.msg, testWhsec) { + t.Errorf("%s: output echoes the signing secret", name) + } + } +} + +func TestRotateRequestRedactsSigningSecret(t *testing.T) { + s := testWhsec + in := api.RotateRequest{Rotate: []string{"secret"}, Grace: true, SigningSecret: &s} + for _, f := range []string{"%v", "%+v", "%#v", "%s"} { + if out := fmt.Sprintf(f, in); strings.Contains(out, testWhsec) { + t.Errorf("%s prints the signing secret: %s", f, out) + } + } +} diff --git a/internal/api/write.go b/internal/api/write.go index 389766d..7c07db2 100644 --- a/internal/api/write.go +++ b/internal/api/write.go @@ -3,6 +3,7 @@ package api import ( "context" "encoding/json" + "fmt" "net/http" "net/url" ) @@ -161,6 +162,24 @@ func (c *RunsClient) UpdateTrigger(ctx context.Context, automationID, triggerID type RotateRequest struct { Rotate []string `json:"rotate"` Grace bool `json:"grace"` + // SigningSecret is the new whsec_ of a Stripe trigger (Stripe issues + // it; NaN generates the others). Never printed: see String. + SigningSecret *string `json:"signing_secret,omitempty"` +} + +// String keeps the signing secret out of any %v or %+v. +func (r RotateRequest) String() string { + return fmt.Sprintf("RotateRequest{Rotate:%v Grace:%v SigningSecret:%s}", r.Rotate, r.Grace, redactedIf(r.SigningSecret != nil)) +} + +// GoString keeps the signing secret out of %#v. +func (r RotateRequest) GoString() string { return r.String() } + +func redactedIf(set bool) string { + if set { + return "[redacted]" + } + return "" } // RotateTrigger is POST /v1/automations/{id}/triggers/{tid}/rotate. From d96cac8d6044d2a738c57e673380b2a16a91a0af Mon Sep 17 00:00:00 2001 From: barckcode Date: Wed, 14 Oct 2026 16:13:24 +0200 Subject: [PATCH 7/7] fix(triggers): a malformed Stripe whsec_ gets a Stripe hint, not the GitHub/NaN one Co-Authored-By: Claude Opus 5.5 --- cmd/automations_write.go | 5 +++++ cmd/writeapi_test.go | 1 + 2 files changed, 6 insertions(+) diff --git a/cmd/automations_write.go b/cmd/automations_write.go index b0478f7..ec971b7 100644 --- a/cmd/automations_write.go +++ b/cmd/automations_write.go @@ -5,6 +5,7 @@ import ( "encoding/json" "errors" "fmt" + "strings" "text/tabwriter" "github.com/nxssie/nan-cli/internal/api" @@ -490,6 +491,10 @@ func doTriggersRotate(ctx context.Context, env *runsEnv, ref, triggerID string, "Roll it there and pass the new whsec_ with --signing-secret-file PATH or --signing-secret-stdin, " + "or rotate only the URL with --url") } + if strings.Contains(apiErr.Message, "Stripe") { + return usageErrorf("the server refused the signing secret: %s. Copy the endpoint's whsec_ from the Stripe dashboard again", + runs.Truncate(runs.SanitizeLine(apiErr.Message), 200)) + } return usageErrorf("the server refused the signing secret: %s. NaN generates the signing secret of GitHub and NaN webhooks; "+ "--signing-secret-file and --signing-secret-stdin are for Stripe's whsec_", runs.Truncate(runs.SanitizeLine(apiErr.Message), 200)) } diff --git a/cmd/writeapi_test.go b/cmd/writeapi_test.go index 70f331e..e304422 100644 --- a/cmd/writeapi_test.go +++ b/cmd/writeapi_test.go @@ -1272,6 +1272,7 @@ func TestTriggersRotateStripeRefusals(t *testing.T) { wants []string }{ "stripe without the secret": {writeSetup{session: cliSession}, nil, stripe400, 1, []string{"Stripe dashboard", "--signing-secret-file", "--url"}}, + "malformed whsec": {writeSetup{session: cliSession}, []string{"--signing-secret-file", file}, stripe400, 1, []string{"Stripe dashboard again"}}, "secret on a github hook": {writeSetup{session: cliSession}, []string{"--signing-secret-file", file}, generated400, 1, []string{"NaN generates", "Stripe"}}, "both sources": {writeSetup{session: cliSession}, []string{"--signing-secret-file", file, "--signing-secret-stdin"}, "", 0, []string{"not both"}}, "terminal": {writeSetup{session: cliSession, tty: true}, []string{"--signing-secret-stdin"}, "", 0, []string{"--signing-secret-stdin", "terminal", "--signing-secret-file"}},