Repository navigation
feat!: resolve SDK auth via OS keychain through createSDK helper - #3999
Merged
Merged
Conversation
github-actions
Bot
added
size/XL
|
This PR is labeled size/XL (29384 lines of code churn, excluding docs, Large PRs are harder to review well. If it can be split into smaller, independently reviewable pieces, consider doing so. This is a suggestion, not a blocker — the check still passes. |
Route all HerokuSDK / HerokuApiClient construction through a single createSDK / createApiClient helper in src/lib/sdk.ts so the CLI-resolved token and platform baseUrl are threaded in one place rather than re-derived (or omitted) at ~170 call sites. Each call site may still pass its own `extensions` (and any other HerokuSDKOptions); the helper's generic preserves the extension-augmented resource types on the returned SDK. An ESLint no-restricted-syntax guard bans `new HerokuSDK` / `new HerokuApiClient` outside src/lib/sdk.ts to keep construction centralized. The helper threads a lazy TokenProvider backed by APIClient.getAuthEntry(), so SDK calls resolve the token on the same path as the rest of the CLI (OS keychain via credential-manager, HEROKU_API_KEY override preserved) while the SDK and heroku-fetch take on no credential-manager dependency (stay Node-agnostic / browser-safe). BREAKING CHANGE: SDK-backed commands now resolve their auth token from the OS keychain (via APIClient.getAuthEntry) instead of ~/.netrc. Previously most SDK-backed commands constructed the SDK without a token and fell through to heroku-fetch 0.1.x's default provider, which read HEROKU_API_KEY then ~/.netrc. Users authenticated via `heroku login` (which writes the OS keychain) are unaffected; any workflow relying on a hand-maintained ~/.netrc entry for these commands must ensure the credential is in the keychain.
eablack
force-pushed
the
eb/cli-createsdk-token-resolution
branch
from
October 16, 2026 21:03
81d637d to
1ac4d67
Compare
Contributor
There was a problem hiding this comment.
One real gap (inline on ps/index.ts): some @heroku/sdk 0.10.0 resources build their own HerokuApiClient from the per-call options.clientOptions, not from the SDK-level options. createSDK's token and baseUrl never reach those calls, so ps --extended and run:inside still go through heroku-fetch's netrc / HEROKU_API_KEY default.
The other comments are smaller cleanups.
… guard Order the message/selector keys alphabetically in the no-restricted-syntax entries so `eslint .` passes; no behavior change to the SDK/client guard.
listExtended and run:inside's exec path (postRun) each build a bare HerokuApiClient from per-call clientOptions rather than the SDK-level client from createSDK, so the CLI-resolved token never reached them. Pass clientOptions: apiClientOptions(heroku) at both call sites and add regression tests that assert the outgoing Authorization header via nock with a stubbed getAuthEntry sentinel.
status needs a tokenless platform client for the public trust endpoint; replace its ad-hoc APIClient construction with a single createUnauthenticatedClient(baseUrl) helper in sdk.ts so unauthenticated client construction lives in one place alongside createSDK/createApiClient.
disable/enable each called createSDK twice to pull metrics and platform separately; destructure both from one createSDK(this.heroku) call.
Match callee.property.name so member-expression constructions of HerokuSDK and HerokuApiClient are also flagged, and note that a later flat-config block setting the same rule replaces rather than merges its options.
The comments still described the old clientOptions: {token: this.heroku.auth}
wiring; describe createSDK's token provider (tokenFor -> getAuthEntry) instead.
sdk 0.11.0 depends on @heroku/heroku-fetch ^0.2.0; bump the CLI's own heroku-fetch pin from ^0.1.6 to match so both resolve to a single copy. The skew otherwise left two structurally-incompatible HerokuApiClientOptions types colliding in src/lib/sdk.ts. Also drop the version-pinned heroku-fetch reference from the tokenFor comment so it can't go stale.
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Centralizes all
HerokuSDK/HerokuApiClientconstruction in the CLI behind a singlecreateSDK/createApiClienthelper (src/lib/sdk.ts), so the CLI-resolved auth token and platformbaseUrlare threaded in one place instead of being re-derived — or omitted — at ~170 call sites. An ESLintno-restricted-syntaxguard bansnew HerokuSDK/new HerokuApiClientanywhere outsidesrc/lib/sdk.tsto keep construction centralized going forward.Each call site still specifies its own
extensions(and any otherHerokuSDKOptions): the helper is generic over the extension tuple (createSDK<const Exts>(…): HerokuSDK<Exts>), so the extension-augmented resource types are preserved on the returned SDK. 63 of the 171 call sites pass extensions, across 17 distinct extension sets.~/.netrcThis is the headline behavioral change and the reason for the
feat!/ major bump.new HerokuSDK()plus ~70 passing only{extensions}). On heroku-fetch0.1.xa tokenless client falls through to heroku-fetch's default provider, which resolvesHEROKU_API_KEY→~/.netrc(vianetrc-parser). The OS keychain was never consulted on that path. (A minority of sites already threadedheroku.auth, which is keychain-sourced.)createSDK/createApiClient, which resolve the token viaAPIClient.getAuthEntry()→@heroku/heroku-credential-manager→ OS keychain (the same store the rest of the CLI uses), withHEROKU_API_KEYstill taking precedence as an override.Net effect: SDK-backed commands now read credentials from the same OS keychain as the rest of the CLI. Users authenticated via
heroku login(which writes the keychain) are unaffected or fixed; any workflow that relied on a hand-maintained~/.netrcentry for these commands must ensure the credential is in the keychain.The SDK and
heroku-fetchtake on nocredential-managerdependency — the Node-only keychain integration lives entirely in the CLI'screateSDKhelper via the existingtoken/TokenProviderseam, so heroku-fetch stays Node-agnostic / browser-safe.Follow-up (not in this PR)
Bumping the CLI's direct
@heroku/heroku-fetchto^0.2.0(which drops the implicit~/.netrcfallback entirely) is deferred to a separate atomic flip, to be done once@heroku/sdk@0.11.0— which bundles heroku-fetch0.2.0— ships, so the direct dep and the SDK's transitive dep move together rather than producing a dual-version tree. This PR is behavior-neutral with respect to that bump:createSDKthreads the token explicitly, so it is forward-compatible with both0.1.xand0.2.0.Type of Change
Breaking Changes (major semver update)
!after your change type to denote a change that breaks current behaviorFeature Additions (minor semver update)
Patch Updates (patch semver update)
Testing
Notes:
createSDKresolves the token lazily (aTokenProviderfunction, not a snapshot), so aheroku login/logoutafter construction is honored. Unit tests mockAPIClient.getAuthEntryat the resolution seam.Steps:
heroku status,heroku apps,heroku logs -n, andheroku apps:errorsall succeed.heroku loginwrites the OS keychain (serviceheroku-cli, account updated at login time) and that nothing is written to~/.netrc(empty, zero heroku entries, untouched).Related Issues
GUS work item: W-23096184