From 9e062505dc72aa489b7f7cbab4da6f0d8b797a73 Mon Sep 17 00:00:00 2001 From: BatteryShark Date: Fri, 24 Apr 2026 13:28:27 -0400 Subject: [PATCH] Simplify show-password behavior --- README.md | 4 +- SECURITY_REPORT.md | 19 ++++---- internal/cli/cli.go | 44 ++++++++++--------- internal/cli/cli_test.go | 36 ++++++++++++--- skills/opencode-agent-operator/SKILL.md | 4 +- .../references/operations.md | 6 +-- .../references/security-defaults.md | 2 +- .../references/tailscale-diagnostics.md | 2 +- 8 files changed, 73 insertions(+), 44 deletions(-) diff --git a/README.md b/README.md index 6e6870b..b4646d8 100644 --- a/README.md +++ b/README.md @@ -159,7 +159,7 @@ opencode-agent list opencode-agent status api opencode-agent logs api opencode-agent restart api -opencode-agent show-password api --reveal +opencode-agent show-password api opencode-agent rotate-password api --reveal opencode-agent uninstall api --purge ``` @@ -195,7 +195,7 @@ opencode-agent status api --json - `stop [name]` - `restart [name]` - `logs [name] [--lines 120]` -- `show-password [name] --reveal` +- `show-password [name]` - `rotate-password [name] [--restart=false] [--reveal]` - `uninstall [name] [--purge]` diff --git a/SECURITY_REPORT.md b/SECURITY_REPORT.md index 13e9435..86266ee 100644 --- a/SECURITY_REPORT.md +++ b/SECURITY_REPORT.md @@ -26,7 +26,7 @@ The highest priority remaining risks are: | F-002 Tailnet bind fallback | Remediated | Automatic `0.0.0.0` fallback is removed; fallback now requires `--allow-all-interfaces-fallback`. | | F-003 Missing secure project config | Remediated | Missing `/opencode.json` is seeded with ask-by-default permissions at `0600` unless disabled. | | F-004 Unsafe existing project config | Mitigated | Existing configs are preserved and audited with visible warnings for dangerous permissions and MCP definitions. | -| F-005 Passwords printed to stdout | Remediated | Secrets are hidden by default; reveal requires explicit `--reveal`; dry-run does not generate or print passwords. | +| F-005 Passwords printed to stdout | Mitigated | Install and rotate hide secrets by default; `show-password` is an explicit credential reveal command; dry-run does not generate or print passwords. | | F-006 Password in process environment | Still outstanding | Parent env is filtered, but `OPENCODE_SERVER_PASSWORD` is still required in the child environment. | | F-007 Shared Basic Auth only | Still outstanding | Credential metadata and warnings were added, but auth is still one shared credential per instance. | | F-008 At-rest logs/metadata | Mitigated | Restrictive permissions remain, log rotation and broader redaction were added; no encryption-at-rest was added. | @@ -389,15 +389,16 @@ Recommended remediation: Re-test entry (2026-04-24): -Status: Remediated. +Status: Mitigated. Current-state notes: -- `install`, `show-password`, and `rotate-password` hide passwords by default. -- Password output now requires explicit `--reveal`. +- `install` and `rotate-password` hide passwords by default. +- `show-password` prints existing credentials by design; use `status` for instance metadata without revealing credentials. +- Install and rotation password output requires explicit `--reveal`. - `install --dry-run` does not generate a password and prints only placeholder text such as `[generated at install time; not printed during dry-run]`. -- `TestInstallDryRunShowsNamedPlan` and `TestShowAndRotatePassword` cover the no-reveal default behavior. -- Remaining risk: explicit `--reveal` still prints the secret by design and should be treated as a deliberate operator action. +- `TestInstallDryRunShowsNamedPlan` and `TestShowAndRotatePassword` cover dry-run behavior, default rotation hiding, and explicit `show-password` reveal behavior. +- Remaining risk: `show-password` and explicit `--reveal` on install/rotate print secrets to stdout by design and should be treated as deliberate operator actions. ### F-006: OpenCode Password Is Passed Through Process Environment @@ -791,13 +792,13 @@ Dangerous project config examples to flag: - OS keychain storage is used for credentials. - Config, state, service unit, and log files are generally created with restrictive permissions. - Password rotation is available and records creation/rotation metadata. -- Passwords are hidden by default in install, dry-run, show-password, and rotate-password output. +- Passwords are hidden by default in install, dry-run, and rotate-password output; `show-password` intentionally reveals existing credentials. - Child process environment inheritance is filtered by default. - Logs rotate and redact common secret shapes in addition to exact password values. ### Gaps -- Secrets can still be printed to stdout when an operator explicitly passes `--reveal`. +- Secrets can still be printed to stdout through `show-password` or when an operator explicitly passes `--reveal` to install/rotate. - The OpenCode server password is still passed to OpenCode through process environment variables. - Rotation has metadata, but no schedule enforcement or per-client revocation model. - Logs have broader redaction, but transformed, partial, or context-specific secrets may still evade redaction. @@ -805,7 +806,7 @@ Dangerous project config examples to flag: ### Recommendations -- Keep the explicit `--reveal` model and avoid adding secret output to JSON responses. +- Keep secret output limited to explicit credential workflows and avoid adding secret output to JSON responses. - Replace environment-variable password handoff if OpenCode gains a protected secret-file, stdin, or keychain integration. - Add credential rotation guidance, age warnings, or an optional rotation policy. - Keep the child process environment filtered by default. diff --git a/internal/cli/cli.go b/internal/cli/cli.go index cabedcc..20642a0 100644 --- a/internal/cli/cli.go +++ b/internal/cli/cli.go @@ -94,7 +94,7 @@ Usage: opencode-agent expose status|off [name] opencode-agent start|stop|restart [name] opencode-agent logs [name] [--lines 120] - opencode-agent show-password [name] --reveal + opencode-agent show-password [name] opencode-agent rotate-password [name] [--reveal] opencode-agent uninstall [name] [--purge] @@ -219,7 +219,7 @@ func install(args []string, stdout, stderr io.Writer) error { if *reveal { fmt.Fprintf(stdout, "Password: %s\n", plainPassword) } else { - fmt.Fprintln(stdout, "Password: stored in the OS keychain (use `opencode-agent show-password --reveal` to print it).") + fmt.Fprintln(stdout, "Password: stored in the OS keychain (use `opencode-agent show-password` to print it).") } printProjectConfigSummary(stdout, projectReport) printExposureSummary(stdout, exposurePlan) @@ -271,10 +271,11 @@ func status(args []string, stdout, stderr io.Writer) error { fs.SetOutput(stderr) nameFlag := fs.String("name", "", "instance name") jsonOutput := fs.Bool("json", false, "print JSON") - if err := fs.Parse(args); err != nil { + split := splitInterspersedFlags(args, map[string]bool{"name": true}) + if err := fs.Parse(split.Flags); err != nil { return err } - name := pickName(*nameFlag, fs.Args()) + name := pickName(*nameFlag, split.Positionals) cfg, paths, err := instance.LoadConfig(name) if err != nil { return err @@ -470,10 +471,11 @@ func serviceAction(args []string, action string, stdout, stderr io.Writer) error fs := flag.NewFlagSet(action, flag.ContinueOnError) fs.SetOutput(stderr) nameFlag := fs.String("name", "", "instance name") - if err := fs.Parse(args); err != nil { + split := splitInterspersedFlags(args, map[string]bool{"name": true}) + if err := fs.Parse(split.Flags); err != nil { return err } - name := pickName(*nameFlag, fs.Args()) + name := pickName(*nameFlag, split.Positionals) cfg, paths, err := instance.LoadConfig(name) if err != nil { return err @@ -503,10 +505,14 @@ func logs(args []string, stdout, stderr io.Writer) error { fs.SetOutput(stderr) nameFlag := fs.String("name", "", "instance name") lines := fs.Int("lines", 120, "lines to print") - if err := fs.Parse(args); err != nil { + split := splitInterspersedFlags(args, map[string]bool{ + "name": true, + "lines": true, + }) + if err := fs.Parse(split.Flags); err != nil { return err } - name := pickName(*nameFlag, fs.Args()) + name := pickName(*nameFlag, split.Positionals) _, paths, err := instance.LoadConfig(name) if err != nil { return err @@ -519,11 +525,11 @@ func showPassword(args []string, stdout, stderr io.Writer) error { fs := flag.NewFlagSet("show-password", flag.ContinueOnError) fs.SetOutput(stderr) nameFlag := fs.String("name", "", "instance name") - reveal := fs.Bool("reveal", false, "print the password") - if err := fs.Parse(args); err != nil { + split := splitInterspersedFlags(args, map[string]bool{"name": true}) + if err := fs.Parse(split.Flags); err != nil { return err } - name := pickName(*nameFlag, fs.Args()) + name := pickName(*nameFlag, split.Positionals) cfg, _, err := instance.LoadConfig(name) if err != nil { return err @@ -533,11 +539,7 @@ func showPassword(args []string, stdout, stderr io.Writer) error { return err } fmt.Fprintf(stdout, "URL: %s\nUsername: %s\n", cfg.AdvertiseURL, creds.Username) - if *reveal { - fmt.Fprintf(stdout, "Password: %s\n", creds.Password) - } else { - fmt.Fprintln(stdout, "Password: stored in the OS keychain (rerun with --reveal to print it).") - } + fmt.Fprintf(stdout, "Password: %s\n", creds.Password) return nil } @@ -547,10 +549,11 @@ func rotatePassword(args []string, stdout, stderr io.Writer) error { nameFlag := fs.String("name", "", "instance name") restart := fs.Bool("restart", true, "restart the service after rotating") reveal := fs.Bool("reveal", false, "print the new password") - if err := fs.Parse(args); err != nil { + split := splitInterspersedFlags(args, map[string]bool{"name": true}) + if err := fs.Parse(split.Flags); err != nil { return err } - name := pickName(*nameFlag, fs.Args()) + name := pickName(*nameFlag, split.Positionals) cfg, _, err := instance.LoadConfig(name) if err != nil { return err @@ -588,10 +591,11 @@ func uninstall(args []string, stdout, stderr io.Writer) error { fs.SetOutput(stderr) nameFlag := fs.String("name", "", "instance name") purge := fs.Bool("purge", false, "remove config, state, and keychain password") - if err := fs.Parse(args); err != nil { + split := splitInterspersedFlags(args, map[string]bool{"name": true}) + if err := fs.Parse(split.Flags); err != nil { return err } - name := pickName(*nameFlag, fs.Args()) + name := pickName(*nameFlag, split.Positionals) cfg, paths, err := instance.LoadConfig(name) if err != nil { return err diff --git a/internal/cli/cli_test.go b/internal/cli/cli_test.go index 25938d8..02a0d03 100644 --- a/internal/cli/cli_test.go +++ b/internal/cli/cli_test.go @@ -264,22 +264,37 @@ func TestShowAndRotatePassword(t *testing.T) { if err := keychain.Store("default", keychain.Credentials{Username: "opencode", Password: "old"}); err != nil { t.Fatal(err) } + namedCfg := cfg + namedCfg.Name = "brain" + namedCfg.Port = 4100 + if _, err := instance.SaveConfig(namedCfg); err != nil { + t.Fatal(err) + } + if err := keychain.Store("brain", keychain.Credentials{Username: "opencode", Password: "brain-secret"}); err != nil { + t.Fatal(err) + } var stdout, stderr bytes.Buffer if code := Run([]string{"show-password"}, &stdout, &stderr); code != 0 { t.Fatalf("show-password code=%d stderr=%s", code, stderr.String()) } - if strings.Contains(stdout.String(), "old") { - t.Fatalf("show-password should hide secret without --reveal: %s", stdout.String()) + if !strings.Contains(stdout.String(), "old") { + t.Fatalf("show-password should reveal the secret: %s", stdout.String()) } stdout.Reset() stderr.Reset() - if code := Run([]string{"show-password", "--reveal"}, &stdout, &stderr); code != 0 { - t.Fatalf("show-password --reveal code=%d stderr=%s", code, stderr.String()) + if code := Run([]string{"show-password", "brain"}, &stdout, &stderr); code != 0 { + t.Fatalf("show-password brain code=%d stderr=%s", code, stderr.String()) } - if !strings.Contains(stdout.String(), "old") { - t.Fatalf("show-password --reveal output = %s", stdout.String()) + if !strings.Contains(stdout.String(), "brain-secret") { + t.Fatalf("show-password should reveal the named instance secret: %s", stdout.String()) + } + + stdout.Reset() + stderr.Reset() + if code := Run([]string{"show-password", "brain", "--reveal"}, &stdout, &stderr); code == 0 { + t.Fatalf("show-password should reject the removed --reveal flag: %s", stdout.String()) } stdout.Reset() @@ -300,6 +315,15 @@ func TestShowAndRotatePassword(t *testing.T) { if creds.CreatedAt.IsZero() || creds.RotatedAt.IsZero() { t.Fatalf("credential metadata was not recorded: %#v", creds) } + + stdout.Reset() + stderr.Reset() + if code := Run([]string{"rotate-password", "brain", "--restart=false", "--reveal"}, &stdout, &stderr); code != 0 { + t.Fatalf("rotate-password brain --restart=false --reveal code=%d stderr=%s", code, stderr.String()) + } + if !strings.Contains(stdout.String(), "Password: ") || strings.Contains(stdout.String(), "stored in the OS keychain") { + t.Fatalf("rotate-password should parse flags after the instance name and reveal when requested: %s", stdout.String()) + } } func resetCLIEnv(t *testing.T) { diff --git a/skills/opencode-agent-operator/SKILL.md b/skills/opencode-agent-operator/SKILL.md index c414cef..ae3332a 100644 --- a/skills/opencode-agent-operator/SKILL.md +++ b/skills/opencode-agent-operator/SKILL.md @@ -10,7 +10,7 @@ Use this skill to operate `opencode-agent` safely. Treat the repository source a ## Core Rules - Prefer loopback installs: keep OpenCode bound to `127.0.0.1` unless the user explicitly accepts broader exposure. -- Do not print passwords unless the user explicitly asks or a documented workflow requires `--reveal`. +- Do not print passwords unless the user explicitly asks, uses `show-password`, or a documented workflow requires `--reveal`. - Treat remote plain HTTP with Basic Auth as unsafe. Use HTTPS termination through Tailscale Serve/Funnel, Cloudflare Tunnel, Caddy, Nginx, or another identity-aware proxy. - Use Tailscale Serve for private tailnet/VPN access. Use Tailscale Funnel only when the user explicitly requests public internet exposure. - Use `--dry-run` before risky install or exposure changes when the user wants a preview, or when you need to explain planned files and commands. @@ -47,7 +47,7 @@ opencode-agent list opencode-agent status api opencode-agent logs api opencode-agent restart api -opencode-agent show-password --reveal api +opencode-agent show-password api opencode-agent rotate-password --reveal api opencode-agent uninstall --purge api ``` diff --git a/skills/opencode-agent-operator/references/operations.md b/skills/opencode-agent-operator/references/operations.md index 7e086be..09c057e 100644 --- a/skills/opencode-agent-operator/references/operations.md +++ b/skills/opencode-agent-operator/references/operations.md @@ -71,7 +71,7 @@ Useful install flags: --dry-run ``` -For lifecycle commands implemented with Go's standard flag parser, put flags before the optional instance name. For example, use `opencode-agent status --json api`, not `opencode-agent status api --json`. +For lifecycle commands with an optional instance name, flags may be placed before or after the name. For example, both `opencode-agent status --json api` and `opencode-agent status api --json` are valid. Do not use `--advertise-host` or `--advertise-url` together with `--expose tailscale`; the CLI rejects that combination. Do not use `--advertise-host` and `--advertise-url` together; choose one. @@ -94,10 +94,10 @@ opencode-agent install --workdir /path/to/project --port 4096 --reveal Show credentials for an existing instance: ```bash -opencode-agent show-password --reveal api +opencode-agent show-password api ``` -Without `--reveal`, `show-password` prints the URL and username and confirms that the password is stored in the OS keychain. +`show-password` prints the URL, username, and password. Use `status` when you only need instance metadata without revealing credentials. Rotate a password and restart by default: diff --git a/skills/opencode-agent-operator/references/security-defaults.md b/skills/opencode-agent-operator/references/security-defaults.md index a48ba1c..1ff9d32 100644 --- a/skills/opencode-agent-operator/references/security-defaults.md +++ b/skills/opencode-agent-operator/references/security-defaults.md @@ -40,7 +40,7 @@ Defaults: - Password is generated with cryptographic randomness when omitted. - Password is stored in the OS keychain service `opencode-agent`, account `instance:`. - Password is not written to the agent config. -- Password is not printed unless `--reveal` is used. +- Password is not printed during install or rotation unless `--reveal` is used. The explicit `show-password` command prints existing credentials. Security implications: diff --git a/skills/opencode-agent-operator/references/tailscale-diagnostics.md b/skills/opencode-agent-operator/references/tailscale-diagnostics.md index c046c65..6580150 100644 --- a/skills/opencode-agent-operator/references/tailscale-diagnostics.md +++ b/skills/opencode-agent-operator/references/tailscale-diagnostics.md @@ -169,7 +169,7 @@ http://127.0.0.1: 6. Confirm credentials: ```bash -opencode-agent show-password --reveal +opencode-agent show-password ``` Use this only when the user needs to log in or test manually. Rotate if credentials may have leaked: