Skip to content

feat(cli): STACKBILT_API_KEY env var auth; deprecate charter login#111

Merged
stackbilt-admin merged 2 commits intomainfrom
feat/stackbilt-api-key-env-var
Apr 16, 2026
Merged

feat(cli): STACKBILT_API_KEY env var auth; deprecate charter login#111
stackbilt-admin merged 2 commits intomainfrom
feat/stackbilt-api-key-env-var

Conversation

@stackbilt-admin
Copy link
Copy Markdown
Member

Additive preparation for the 1.0 package split. No public API removed or renamed — follows the OSS additive-only update policy.

Why

charter login is the single most eyebrow-raising OSS/commercial coupling in @stackbilt/cli: an OSS governance tool soliciting a bearer token and writing it to ~/.charter/credentials.json. For a regulated-company security review, that's the line that gets flagged — not that the tool calls an external API the user opted into, but that the tool asks for and persists credentials.

Supporting the standard STACKBILT_API_KEY env var lets users authenticate the commercial commands (run, architect) without the CLI persisting anything to disk. The solicitation in the README / charter login output gets demoted from "Preferred" to "Deprecated alternative." The capability remains; the audit surface gets much cleaner today while the full @stackbilt/build package split is scoped separately.

What ships

credentials.ts

  • API_KEY_ENV_VAR = 'STACKBILT_API_KEY' export.
  • resolveApiKey(){ apiKey, source: 'env' | 'credentials', baseUrl? } | null. Env var wins over stored credentials. Empty-string and whitespace-only env vars are treated as unset and fall through. loadCredentials(), saveCredentials(), clearCredentials() preserved unchanged for back-compat.

commands/architect.ts, commands/run.ts

  • loadCredentials()resolveApiKey(). run's useGateway decision honors the env var identically to stored credentials, so users with only the env var set get the same rich gateway scaffold output that charter login users get today.

commands/login.ts

  • Deprecation notice to stderr on every invocation (except --logout). Command functionality unchanged.
  • Help text reworked: env var is the Preferred path, stored credentials the Deprecated alternative.
  • If STACKBILT_API_KEY is set and --key is not provided, report the env var as the authoritative source.

http-client.ts

  • Scaffold auth-error message now points at STACKBILT_API_KEY first, charter login marked deprecated.

Tests

  • credentials.test.ts (new, 4 tests): env-var precedence over stored credentials, whitespace trimming, empty-string fallthrough, whitespace-only fallthrough.
  • login.test.ts (new, 2 tests): stderr deprecation notice on invocation; env-var reporting path when set.
  • All 405 existing tests pass.

Docs

  • packages/cli/README.md: new Authentication (optional) section — one paragraph, documents the env-var path and flags charter login as deprecated.
  • CHANGELOG.md: [Unreleased] section with Added / Deprecated / Changed.
  • Root README.md intentionally unchanged in this PR. The hero still leads with scaffolding; that rewrite lands with the package split so the docs and the code tell a coherent story in one release note.

Why additive, not a breaking change (0.11.0 minor, not 1.0)

The OSS policy requires a major bump for public-API removal. Yanking login.ts + credentials.ts would force a coordinated 1.0 ship that also needs @stackbilt/build to exist. That package doesn't exist yet. The env-var bridge is the disciplined sequencing move: remove the solicitation optics now (additively), do the proper split later when the receiving package is built.

Signal that informed the deprecation window

30-day telemetry on one active install: validate (14), doctor (7), adf.evidence (7), run (5), blast (5), audit (3), surface (3). architect and scaffold don't register. run is ~10% of governance use combined — real but not dominant. Justifies a deprecation-with-notice path rather than same-release removal.

Follow-ups (explicitly not this PR)

  • Root README rewrite aligned with the package split.
  • RFC: Extract commercial surface from @stackbilt/cli into @stackbilt/build — list of files, target repo, publish pipeline.
  • 1.0 cut: remove login, credentials persistence, run / architect / scaffold. They live in @stackbilt/build by then.

Test plan

  • pnpm build clean.
  • pnpm test — 405/405 pass (including the new 6 tests and the previously-flaky precommit-hook integration test which passed cleanly this run at 40s).
  • Spot-check: charter login with no args emits the stderr deprecation notice and preserves existing "Logged in as: …" output when stored creds are present.
  • Spot-check: STACKBILT_API_KEY=ea_… charter login emits the deprecation notice and reports "Using STACKBILT_API_KEY from environment: …".
  • Review eyes on the deprecation notice wording — is "will be removed in 1.0" too specific if we want timing flexibility? Alternative: "will be removed in a future major release."

Kurt Overmier and others added 2 commits April 16, 2026 08:19
Additive preparation for the 1.0 package split (see charter RFC TBD).
No public API removed or renamed — follows OSS update policy.

Why
  The CLI currently solicits a Stackbilt API key via `charter login`, which
  is the most eyebrow-raising OSS/commercial coupling in the package: an
  OSS governance tool writing a bearer token to ~/.charter/credentials.json.
  Supporting the standard env-var path lets users authenticate the
  commercial commands (`run`, `architect`) without the CLI persisting
  anything to disk, materially improving the audit story now while the
  full split to @stackbilt/build is scoped and sequenced separately.

What changes
  credentials.ts
    + API_KEY_ENV_VAR = 'STACKBILT_API_KEY'
    + resolveApiKey() → { apiKey, source: 'env' | 'credentials', baseUrl? } | null
      Env var wins over stored credentials; empty/whitespace env var falls
      through. loadCredentials(), saveCredentials(), clearCredentials()
      preserved unchanged.

  commands/architect.ts, commands/run.ts
    - loadCredentials() call sites → resolveApiKey(). `run`'s useGateway
      decision now honors the env var path identically to stored creds.

  commands/login.ts
    - printDeprecationNotice() on stderr for every invocation except
      --logout. Command functionality unchanged. Help text reworked: env
      var is the "Preferred" path, stored credentials the "Deprecated
      alternative." If the env var is set and no --key is given, we
      report the env var as the authoritative source.

  http-client.ts
    - Scaffold auth-error string now points at STACKBILT_API_KEY first,
      `charter login` marked deprecated.

Tests
  + credentials.test.ts — 4 tests covering env-var precedence, trimming,
    empty-string fallthrough, whitespace-only fallthrough.
  + login.test.ts — 2 tests covering the stderr deprecation notice and
    env-var reporting path.
  All 405 existing tests still pass.

Docs
  - packages/cli/README.md: new "Authentication (optional)" section
    documenting the env var and login deprecation. Root README
    unchanged — that rewrite ships with the package split.
  - CHANGELOG.md: [Unreleased] section added with Added/Deprecated/Changed
    subsections.

Follow-ups (not in this PR)
  - Root README rewrite when package split lands
  - RFC: extract commercial surface into @stackbilt/build
  - 1.0 cut removes login, credentials persistence, run/architect/scaffold

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Addresses feedback on PR #111:

- credentials.test.ts: mock node:fs so loadCredentials returns deterministic
  results instead of reading the developer's real ~/.charter/credentials.json.
  Empty/whitespace-env-var cases are now pinned to the stored-credentials
  fallback with stubbed data rather than being conditionally skipped.
  Added explicit "env wins when both sources set" test that was claimed by
  the describe block but never exercised.

- STACKBILT_API_BASE_URL companion env var: resolveApiKey now returns a
  baseUrl from the env when source='env'. Fixes the silent base-URL drop
  for users migrating from `charter login --url https://custom ...`.

- login.ts: --logout now emits the deprecation notice before clearing
  credentials. One-line fix; users exiting the deprecated surface see the
  upgrade path.

- CLI README: "Authentication (optional)" gains a one-line security
  caveat about env-var inheritance by child processes, recommending
  per-invocation setting in CI over global export in shared shells.

- auth-wiring.test.ts (new): mocks resolveApiKey and EngineClient; pins
  the useGateway decision for `run` (scaffold vs build) and asserts
  architect forwards the resolved apiKey + baseUrl into the client
  constructor. 5 tests covering env/credentials/null.

All 16 auth-related tests pass (credentials: 9, login: 2, auth-wiring: 5).
Full suite: 413/415 pass; the two failures are the same WSL-flaky
precommit-hook integration test already flagged on PR #110 and unrelated
to this diff.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@stackbilt-admin
Copy link
Copy Markdown
Member Author

Review response — 869eb1d

Addressed the blocker + non-critical items. Keeping "will be removed in 1.0" per your explicit call.

Blocker fixed:

  • Test isolation leakcredentials.test.ts now mocks node:fs via vi.mock, so loadCredentials returns deterministic values per test. Empty/whitespace-env-var cases now explicitly stub stored credentials and assert the 'credentials' source path (previously: conditionally skipped via if (result !== null), false-passing on any machine with stored creds).
  • Missing "env wins over stored" test — added. Sets both env var and stored credentials via the fs mock, asserts env wins with source: 'env'.

Non-critical items:

  • STACKBILT_API_BASE_URL companion env var — chose option (a) from your three. resolveApiKey now returns a baseUrl from the env when source='env'. Silent baseUrl drop on migration fixed. New test in credentials.test.ts covers both set and unset cases.
  • --logout deprecation notice — added. printDeprecationNotice() fires before clearCredentials().
  • README env-var inheritance caveat — added one line in the "Authentication (optional)" section: "Environment variables are inherited by any child processes spawned from the same shell and may appear in /proc/<pid>/environ. In CI, prefer setting the variable per-invocation (e.g., a job-scoped secret) rather than exporting it globally in a shared developer shell."
  • Wiring tests for architect + run — new auth-wiring.test.ts (5 tests). Mocks resolveApiKey and EngineClient via vi.mock + vi.hoisted. Pins:
    • architect forwards env-sourced apiKey + baseUrl into EngineClient constructor
    • architect passes apiKey=null when resolve returns null
    • run uses gateway (scaffold) when env var provides a key
    • run falls back to engine (build) when no key resolves
    • run uses gateway when stored credentials resolve — parity with env path

Deferred per your own framing:

  • Masking tightening (pre-existing pattern, broader cleanup — not a regression in this PR).

Open question answered: kept "will be removed in 1.0" per your call. Vague deprecations rot; commit to the window.

Test results after changes:

  • Auth suite (credentials + login + auth-wiring): 16/16 pass
  • Full suite: 413/415 pass — the two failures are the same WSL-flaky precommit-hook.test.ts > auto-tidies bloated CLAUDE.md integration test already flagged on refactor(blast,cli,serve): Zod-Core-Out vertical slice (#109) #110. Unrelated to this diff; passes in isolation under lower I/O pressure.

@stackbilt-admin stackbilt-admin merged commit 552f2ff into main Apr 16, 2026
3 checks passed
stackbilt-admin pushed a commit that referenced this pull request Apr 16, 2026
Synchronized version bump for all @stackbilt/* packages to 0.11.0.

Highlights:
- STACKBILT_API_KEY / STACKBILT_API_BASE_URL env-var auth (#111)
- Zod-Core-Out vertical slice for @stackbilt/blast + charter_blast MCP tool (#110)
- charter login deprecated; scheduled for 1.0 removal alongside @stackbilt/build split (#112)

See CHANGELOG.md [0.11.0] for the full entry.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant