feat: add a keyring provider for the OS credential store - #150
Open
husniadil wants to merge 7 commits into
Open
Conversation
Secrets whose payload is JSON were rendered with fmt.Sprintf("%v"), which is
Go's debug formatting rather than JSON. The value a process received was then
not the value that was stored:
1754110382 -> "1.754110382e+09"
["a","b"] -> "[a b]"
{"token":"secret"} -> "map[token:secret]"
The first case is the damaging one. JSON has no integer type, so every number
decodes as float64, and %v prints large float64 values in scientific notation.
Any large integer in a secret — a Unix timestamp, an account id, a port —
arrived corrupted, with no error to indicate it.
Nothing surfaced this because the conversion cannot fail: %v accepts any value
and always produces a string, so a type it handles badly still looks like a
successful fetch. The failure appears later, in the process that receives the
value.
DecodeSecretJSON keeps numbers as json.Number so their original text survives,
and StringifyValue re-encodes arrays and objects as JSON so the receiving
process can parse them back. Scalars keep their literal form.
Four providers decode JSON payloads and are affected: aws_secretsmanager,
gcloud_secretmanager, azure_keyvault, and bitwarden in 'note' format.
1password, infisical and bitwarden_sm build string-only maps from their SDKs,
so %v was already a no-op there; they move to the shared helper so the
behaviour cannot drift apart later.
Payloads that are not JSON still fall back to being treated as a single value,
including the case of trailing content after a JSON object, which
json.Unmarshal rejected and a bare json.Decoder would not.
CONFIGURATION.md gains a Value Types table under Key Mappings, since the
conversion applies to every provider that parses JSON rather than to one.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Hmh2p2Bg6kmxxvzpFDW2WL
A JSON pointer can address an array element or a bare scalar, neither of
which fits map[string]interface{}. DecodeSecretJSON keeps its object-only
contract and is now expressed in terms of the general decoder.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Hmh2p2Bg6kmxxvzpFDW2WL
Credential blobs nest, and their keys contain dots, pipes and colons, so a dotted path would need an escaping rule invented here. RFC 6901 already defines one, and gives array indexing for free. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Hmh2p2Bg6kmxxvzpFDW2WL
sstart already depends on go-keyring for its cache and OIDC tokens but never exposed the OS credential store as a source of secrets. Reading fails loudly when the store is unavailable: unlike the cache, an empty result here hands the child process an environment with no secrets in it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Hmh2p2Bg6kmxxvzpFDW2WL
A flat JSON secret behaves the same whichever provider holds it; a keyring item that yielded one opaque blob instead would read as a bug. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Hmh2p2Bg6kmxxvzpFDW2WL
Reading a credential blob whole exposes every secret inside it to the child process. A pointer lets a config ask for the one value it needs; the schema knowledge lives in the user's config, not in sstart. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Hmh2p2Bg6kmxxvzpFDW2WL
Includes how to populate the store on each platform, since the provider only reads and there is no sstart command that writes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Hmh2p2Bg6kmxxvzpFDW2WL
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
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.
What this adds
A
keyringprovider that reads a secret from the operating system's credentialstore: Keychain on macOS, Credential Manager on Windows, Secret Service on Linux.
sstart already depends on
zalando/go-keyringand already uses it for thesecrets cache and for OIDC token storage. Nothing exposed it as a source of
secrets, so the one credential store a developer's machine already has — backed
up by the OS backup tool, unlocked by the OS login — was the one place sstart
could not read from.
No build tags and no platform gating:
go-keyringships a native backend foreach platform, so there is no platform to reject.
Why it is named
keyringThe codebase already says "keyring" everywhere, including the exported
StorageBackendKeyring = "keyring"ininternal/oidc/storage.go. "Keychain" isApple's product name, and naming it that would hide the provider from Windows
and Linux users who are equally supported.
Reading into a payload
A credential item often holds a JSON blob rather than one secret. Read whole,
every secret inside it is exported to the child process. The optional
pointerfield selects one node first:
pointeris an RFC 6901 JSON pointer rather than a dotted path because real keynames contain dots, pipes and colons — a live example is
plugin:engineering:github|1eea5f27. RFC 6901 already defines the escaping, andgives array indices (
/scopes/0) for free.It is called
pointerand notpathbecausepathalready means "where thesecret lives" in
dotenv,infisicalandvault. A test asserts that a straypath:is not silently read as a pointer.Note that no application's schema is encoded in sstart. The pointer lives in the
user's config, so a different vendor's credential shape needs a different
pointer, not a new sstart release.
Behaviour
Without
pointer, a JSON object is expanded into one variable per key, honouringkeysexactly as the cloud providers do; anything else becomes a single variablenamed
<PROVIDER_ID>_SECRET, orkeyif given. Withpointer, the selected nodegoes through the same rule. Values follow the conversion in #149.
serviceanduserare both required:go-keyringexposes onlyGet/Set/Delete/DeleteAll, so without enumeration an item is unreachablewithout its exact identity.
Two deliberate choices worth flagging for review:
An unavailable keyring is an error, not a silent skip. The cache disables
itself quietly because a miss only costs a re-fetch. A provider that yields
nothing instead hands the child process an environment with no secrets in it.
No
sync.Onceavailability probe, unlikeinternal/cache/cache.go. Theprovider makes exactly one keyring call regardless, so a probe would only add a
second credential-store access — and on macOS a second access prompt — to learn
what the real call is about to report. Mapping the error from
Getgives thesame two outcomes with no package-level mutable state.
Read-only
There is no write path, and no
sstart keyring set. Adding a write commandchanges what sstart is, and that is a call for you to make rather than something
to slip in with a provider.
CONFIGURATION.mddocuments how to populate thestore with each platform's own tool.
Verification
go-keyring'sMockInit()andMockInitWithError()— no OS keyring and no container needed, including thekeyring-unavailable path.
~0/~1, array indices, keyscontaining
|and:, and the error cases.reason the field exists.
error all verified against an item created with
security add-generic-passwordand deleted afterwards.
golang:1.25: both packages pass, the binary builds withCGO, and a headless host with no Secret Service produces
exec: "dbus-launch": executable file not found in $PATH. On Linux this usually means no Secret Service is running, which is common on headless hosts.errors.Is(err, ErrNotFound)is false there, so the unavailable branch istaken rather than the not-found one.
-short ./tests/end2end/...passes locally with Docker (365s).go build ./...andgo vetclean.Not verified: Windows Credential Manager. I have no Windows machine to test
on, so the docs make no claim about it beyond the backend
go-keyringuses.internal/cache,internal/provider/gcsmandinternal/provider/vaultunittests fail on
maintoday. I confirmed they fail identically at this branch'sstarting commit, before any of this work. #148 fixes them.
Registration
internal/cli/root.gogets the blank import. Without it the package compiles andevery test passes while the binary reports
unknown provider kind: keyring—the same failure #146 fixes for
azure_keyvault. Verified by building the binaryand running it against a
keyringconfig.