fix: post-merge audit remediation — §1.12 positional leak + §1.8 keychain fail-loud [INT-443]

CLAUDE.md

@@ -4,7 +4,7 @@ This file provides guidance for AI agents working with the newrelic-cli codebase
 
 ## Project Overview
 
-newrelic-cli is a command-line interface for New Relic written in Go. It uses the Cobra framework for commands and provides a public `api/` package that can be imported as a Go library. The CLI supports multiple output formats (table, JSON, plain). The API key is stored only in the OS keyring via the shared `cli-common/credstore` (macOS Keychain / Windows Credential Manager / Linux Secret Service); non-secret `account_id`/`region` live in `~/.config/newrelic-cli/config.yml`. See "Credentials" below.
+newrelic-cli is a command-line interface for New Relic written in Go. It uses the Cobra framework for commands and provides a public `api/` package that can be imported as a Go library. The CLI supports multiple output formats (table, JSON, plain). The API key is stored in the OS keyring via the shared `cli-common/credstore` (macOS Keychain / Windows Credential Manager / Linux Secret Service), or an encrypted file with the explicit file-backend opt-in — never in plaintext and never in `config.yml`; non-secret `account_id`/`region` live in `~/.config/newrelic-cli/config.yml`. See "Credentials" below.
 
 ## Quick Commands
 
@@ -308,10 +308,12 @@ The View struct handles all output formatting. To add a new format:
 
 ## Credentials (Secret-Handling Standard §2.5)
 
-The API key is stored **only** in the OS keyring via `cli-common/credstore`
-under ref `newrelic-cli/default`, key `api_key` (one logical credential, one
-key — §1.3). It is **never** on disk and **never** read from the environment
-at runtime. `account_id`/`region` are non-secret and live in
+The API key is stored in the OS keyring via `cli-common/credstore` under ref
+`newrelic-cli/default`, key `api_key` (one logical credential, one key —
+§1.3), or — with the explicit `keyring.backend: file` opt-in — an encrypted
+file. It is **never** stored in plaintext, **never** in `config.yml`, and
+**never** read from the environment at runtime. `account_id`/`region` are
+non-secret and live in
 `~/.config/newrelic-cli/config.yml` alongside `credential_ref` and the
 optional `keyring.backend: file` opt-in.
 

README.md

@@ -15,7 +15,7 @@ A command-line interface for interacting with New Relic APIs.
 - **Synthetic Monitors**: List and inspect synthetic monitoring configurations
 - **Users**: List and view user details
 - **Multiple Output Formats**: Table, JSON, and plain (scriptable) output
-- **Secure Credential Storage**: API key stored in the OS keyring (macOS Keychain / Windows Credential Manager / Linux Secret Service), never on disk
+- **Secure Credential Storage**: API key stored in the OS keyring (macOS Keychain / Windows Credential Manager / Linux Secret Service), or an encrypted file with the explicit file-backend opt-in — never in plaintext and never in `config.yml`
 
 ## Installation
 
@@ -117,7 +117,7 @@ go install github.com/open-cli-collective/newrelic-cli/cmd/nrq@latest
 ## Quick Start
 
 ```bash
-# 1. First-time setup (API key stored in the OS keyring, never on disk)
+# 1. First-time setup (API key stored in the OS keyring — never in plaintext, never in config.yml)
 nrq init
 
 # 2. Verify configuration

api/client.go

@@ -3,12 +3,33 @@ package api
 import (
 	"bytes"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"io"
 	"net/http"
 	"time"
 )
 
+// ErrNewRemoved is returned by the deprecated New shim. It exists so an
+// external importer of this pre-1.0 package gets an actionable runtime
+// error instead of a bare compile break, and so the credential-resolution
+// move is self-documenting at the old call site.
+var ErrNewRemoved = errors.New(
+	"api.New() has been removed: the API key now resolves from the OS keyring " +
+		"via `nrq init` / `nrq set-credential`, not from this package. " +
+		"Construct the client with api.NewWithConfig(api.ClientConfig{...}) " +
+		"using values you resolve yourself")
+
+// New is deprecated and non-functional.
+//
+// Deprecated: New previously resolved credentials from the environment and
+// config file. Per Secret-Handling Standard §2.5/§1.11 the api/ package no
+// longer reads the keyring, environment, or config, nor runs the §1.8
+// migration (that is a command-layer side effect — see NewWithConfig). This
+// shim only returns ErrNewRemoved so existing importers fail clearly. Use
+// NewWithConfig.
+func New() (*Client, error) { return nil, ErrNewRemoved }
+
 // Region represents a New Relic region
 type Region string
 

internal/cmd/configcmd/config.go

@@ -10,6 +10,7 @@ import (
 	"fmt"
 	"io"
 	"os"
+	"runtime"
 	"strings"
 
 	"github.com/spf13/cobra"
@@ -67,7 +68,7 @@ Examples:
   op read "op://Vault/New Relic/api key" | nrq set-credential --key api_key --stdin
   nrq set-credential --key api_key --from-env NEWRELIC_API_KEY
   nrq set-credential --ref newrelic-cli/default --key api_key --stdin`,
-		Args: cobra.NoArgs,
+		Args: root.NoPositionalArgs, // never echo a fat-fingered secret (§1.12)
 		RunE: func(cmd *cobra.Command, _ []string) error {
 			return runSetCredential(o)
 		},
@@ -477,6 +478,9 @@ func runClear(o *clearOptions) error {
 			if _, statErr := os.Stat(config.LegacyCredentialsPath()); statErr == nil {
 				v.Println("would remove: " + config.LegacyCredentialsPath() + " (legacy plaintext)")
 			}
+			if runtime.GOOS == "darwin" {
+				v.Println("would remove: legacy macOS Keychain accounts for service \"newrelic-cli\" (if present)")
+			}
 		}
 		return nil
 	}
@@ -513,6 +517,12 @@ func runClear(o *clearOptions) error {
 				v.Success("Removed %s (legacy plaintext)", lp)
 			}
 		}
+		// Also scrub the legacy macOS Keychain accounts: otherwise a
+		// pre-migration `clear --all` on macOS is silently undone by the
+		// next Open() re-migrating the surviving Keychain item.
+		if err := keychain.ScrubLegacyKeychain(); err != nil {
+			return fmt.Errorf("remove legacy macOS Keychain accounts: %w", err)
+		}
 	}
 	return nil
 }

internal/cmd/initcmd/init.go

@@ -43,10 +43,10 @@ func Register(rootCmd *cobra.Command, opts *root.Options) {
 	cmd := &cobra.Command{
 		Use:   "init",
 		Short: "First-time setup (stores the API key in the OS keyring)",
-		Long: `Configure nrq. The API key is stored in the OS keyring (never on
-disk) and is ingested ONLY via stdin, a named env var, or an interactive
-no-echo prompt — never as a flag/positional literal (§1.5.1). account_id
-and region are non-secret and written to config.yml.`,
+		Long: `Configure nrq. The API key is stored in the OS keyring (never in
+plaintext, never in config.yml) and is ingested ONLY via stdin, a named env
+var, or an interactive no-echo prompt — never as a flag/positional literal
+(§1.5.1). account_id and region are non-secret and written to config.yml.`,
 		Example: `  # Interactive (no-echo API key prompt)
   nrq init
 
@@ -55,8 +55,9 @@ and region are non-secret and written to config.yml.`,
   nrq init --api-key-from-env NEWRELIC_API_KEY --account-id 12345
 
   # Resolve a one-time migration conflict by forcing the legacy value
-  nrq init --overwrite --api-key-stdin`,
-		Args: cobra.NoArgs,
+  # (no ingress flag — stdin/env would replace the forced legacy value)
+  nrq init --overwrite`,
+		Args: root.NoPositionalArgs, // never echo a fat-fingered API key (§1.12)
 		RunE: func(cmd *cobra.Command, _ []string) error {
 			return runInit(o)
 		},

internal/cmd/root/root.go

@@ -1,6 +1,7 @@
 package root
 
 import (
+	"errors"
 	"io"
 	"os"
 
@@ -86,7 +87,7 @@ const rootLong = `nrq is a command-line interface for New Relic.
 It provides commands for managing applications, dashboards, alerts,
 users, and other New Relic resources.
 
-First-time setup (stores the API key in the OS keyring, never on disk):
+First-time setup (API key in the OS keyring — never plaintext, never in config.yml):
   nrq init
 
 Non-interactive credential ingress:
@@ -150,3 +151,21 @@ func RegisterAll(cmd *cobra.Command, opts *Options, fns ...RegisterFunc) {
 		fn(cmd, opts)
 	}
 }
+
+// NoPositionalArgs is a cobra Args validator for the secret-ingress commands
+// (init, set-credential). cobra.NoArgs formats its error as
+// `unknown command %q for %q`, quoting args[0] — so `nrq init NRAK-xxx`
+// would echo the fat-fingered API key to stderr and any logs (§1.12). This
+// rejects positional args with a STATIC message that never contains the
+// argument value.
+func NoPositionalArgs(_ *cobra.Command, args []string) error {
+	if len(args) > 0 {
+		return errNoPositionalArgs
+	}
+	return nil
+}
+
+var errNoPositionalArgs = errors.New(
+	"this command takes no positional arguments; a secret must be provided " +
+		"via stdin, a named environment variable, or an interactive prompt — " +
+		"never as a command-line argument (see --help; §1.5.1)")