fix(secrets): support binary Secrets Manager values#442
Merged
Conversation
AWS Secrets Manager stores a value as either SecretString or SecretBinary. Both
secret-retrieval paths — the secretsmanager: ref resolver (pkg/secrets) and the
awssm credential resolver (pkg/awscreds) — read only SecretString and rejected
binary secrets ("binary secrets are not supported"). A secret written via the
binary API comes back as SecretBinary, so resolving such a secret failed.
Add a shared secrets.ValueFromGetSecretOutput that prefers the string form and
falls back to the (already base64-decoded) binary bytes, and route both
pkg/awscreds and secrets.Resolve through it so the two paths behave consistently
and can't drift.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR updates SchemaBot’s AWS Secrets Manager integration so secrets stored as SecretBinary (not just SecretString) can be resolved consistently across both the generic secretsmanager: reference resolver (pkg/secrets) and the AWS credential backend (pkg/awscreds).
Changes:
- Adds
secrets.ValueFromGetSecretOutputto extract a secret value, preferringSecretStringand falling back toSecretBinary. - Routes both
pkg/secretsandpkg/awscredsthrough the shared helper to avoid drift and remove duplicated “string-only” logic. - Adds a unit test covering string / binary / neither cases.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| pkg/secrets/resolve.go | Uses a shared helper to support SecretBinary when resolving secretsmanager: references. |
| pkg/secrets/resolve_test.go | Adds unit coverage for the new helper’s string/binary/error behavior. |
| pkg/awscreds/awscreds.go | Switches Secrets Manager fetching to the shared helper so binary secrets work in the credentials backend too. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- ValueFromGetSecretOutput tests SecretBinary with non-nil, not length, so an intentionally empty binary value round-trips as "" consistently with the string form (covered by a new test case). - Name the secret in the awscreds get-secret error for triage parity with the secretsmanager: resolver. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
aparajon
force-pushed
the
aparajon/awssm-binary-secrets
branch
from
d5df714 to
e495666
Compare
morgo
approved these changes
Jun 19, 2026
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.
Why this matters
AWS Secrets Manager stores a value as either
SecretStringorSecretBinary. SchemaBot's two retrieval paths — thesecretsmanager:reference resolver (pkg/secrets) and theawssmcredential backend (pkg/awscreds) — both read onlySecretStringand rejected binary secrets with"binary secrets are not supported". A secret written via the binary API comes back asSecretBinary, so resolving such a secret (e.g. a DDL password stored as binary) failed at resolution time.What it does
secrets.ValueFromGetSecretOutput(resp, name)— prefers the string form, falls back to the binary bytes (which aws-sdk-go-v2 has already base64-decoded), and errors only when neither is set.pkg/awscreds(getSecretValue) andsecrets.Resolvethrough it, so the two paths behave consistently and can't drift. This removes the duplicatedSecretString-only logic (and its identical limitation) from both.Behavior for string secrets is unchanged; binary secrets now resolve instead of erroring.
This PR was generated by Claude Code (claude-opus-4-8[1m]).