fix: update aws-actions/configure-aws-credentials from v4 to v6 (node24)#95
Open
kinyoklion wants to merge 9 commits into
Open
fix: update aws-actions/configure-aws-credentials from v4 to v6 (node24)#95kinyoklion wants to merge 9 commits into
kinyoklion wants to merge 9 commits into
Conversation
The v4 release uses Node 20 which is deprecated. v6 uses Node 24. This resolves the Node 20 deprecation warning for this dependency. Co-Authored-By: rlamb@launchdarkly.com <4955475+kinyoklion@users.noreply.github.com>
Contributor
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
… implementation The dkershner6 action is pinned to node20 and has no node24 release. Replace it with a shell script that uses the AWS CLI directly, including ::add-mask:: for secret redaction in logs. Co-Authored-By: rlamb@launchdarkly.com <4955475+kinyoklion@users.noreply.github.com>
joker23
approved these changes
Jun 17, 2026
Rewrites the SSM fetch script to fix issues found in review: - Write multiline secret values to $GITHUB_ENV via a randomized heredoc delimiter (a plain NAME=value truncates multiline secrets and parses the tail as additional env entries); mask every line. - Fail loudly when a requested parameter is missing instead of silently exporting fewer vars (handles InvalidParameters), and validate all paths are present before writing any to avoid a partially-populated $GITHUB_ENV. - Resolve pairs by position rather than matching returned names back, so duplicate paths each get their own env var and no stale value is reused. - Parse with mapfile + trim() via parameter expansion (xargs mangled quotes/backslashes; unquoted command substitution word-split input). Adds a mocked smoke test (actions/release-secrets/tests) and a CI workflow that runs it on changes to the action.
Move ::add-mask:: registration to the moment each value is read from the AWS response, before it is stored in the map or used anywhere else, so the unmasked-in-memory window is as small as possible and any future code between fetch and write is covered by masking.
…lper Replace the bash ssm-get-parameters.sh with a TypeScript implementation under actions/release-secrets/ssm that uses @actions/core (masking + GITHUB_ENV export) and @aws-sdk/client-ssm (the GetParameters call). The composite invokes it via `node "$GITHUB_ACTION_PATH/ssm/dist/index.js"`. Why: - @actions/core owns the secret-handling plumbing (masking, multiline-safe GITHUB_ENV heredoc export) instead of hand-rolling it in shell. - The AWS SDK removes the ambient `aws` CLI dependency and pins a reproducible client version via package-lock.json. - First-party and unit-tested (vitest + aws-sdk-client-mock). Behavior is preserved from the hardened bash: parse/trim/validate pairs, chunk by 10, decrypt, mask every value (whole + per line, since runner log masking is line-oriented), fail before writing anything if a parameter is missing, and support duplicate paths. CI (test-release-secrets-ssm.yml) runs typecheck, unit tests, and a check-dist step that fails if the committed bundle drifts from source; the build Node is pinned via .nvmrc so that comparison is stable.
![semgrep-code-launchdarkly[bot]](https://avatars.githubusercontent.com/u/8039656?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…cooldown - Replace npm/package-lock.json with Yarn 4 (via corepack) + yarn.lock - Add .yarnrc.yml npmMinimalAgeGate: "7d" install-time cooldown, matching the org Renovate minimumReleaseAge policy; nodeLinker: node-modules so ncc bundles - Bump all deps to the latest CVE-free, >=7-day-old versions (@actions/core 3, @aws-sdk/client-ssm 3.1067, typescript 6, vitest 4, @types/node 25, ncc 0.44). Clears the Semgrep vitest/vite/esbuild findings; `yarn npm audit` is clean - Drop the hand-rolled per-line maskSecret loop and call core.setSecret directly: since actions/runner #1521 the runner already masks each line of a multiline secret, so replicating it was redundant - Modernize tsconfig (Preserve/Bundler resolution + explicit rootDir) for TS 6 and @actions/core v3 being ESM-only - Convert CI to corepack/yarn (immutable install) and SHA-pin to the latest >=7-day-old actions: checkout v6.0.3, setup-node v6.4.0
There is no coverage tooling configured (vitest runs without --coverage and no provider is installed), and vitest defaults to the v8 provider, which does not honor istanbul pragmas. The guarded line is the CLI entrypoint, exercised via `node dist/index.js` rather than unit tests.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
…ectors The `require.main === module` guard compiled to an always-false check once the source became an ES module (tsconfig module: Preserve, needed for @actions/core v3's ESM-only package), so ncc wrapped the entry and `run()` never executed — the action was a silent no-op (malformed input exited 0, nothing exported). - Split the entrypoint into src/main.ts, which imports run() from ./index and invokes it unconditionally; src/index.ts is now a side-effect-free library that tests import without triggering execution. - Build now targets src/main.ts. - Add a CI "Smoke-test the bundled entrypoint" step that runs node dist/index.js on malformed input and fails if it exits 0 — the unit tests import run() from source, so only an artifact-level check can catch a no-op bundle. - Reject version/label selectors (`name:3`) in parsePairs: SSM returns the base name in Parameter.Name, so a selector would be fetched but then reported missing. Fail early with a clear message instead. Add a regression test. Found by multi-agent review (adversarial agent, by executing the bundle).
joker23
approved these changes
Jun 22, 2026
The runImpl indirection added nothing: runImpl was internal, called from one place, and the only difference was run() supplying the default SSMClient. A default parameter is already a lazy DI seam (tests inject a mock client, so the real client is never constructed), so a single async run() is equivalent.
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
Eliminates both Node 20 deprecation warnings from the
release-secretscomposite action, and replaces the unmaintained third-party SSM action with a first-party, dependency-pinned TypeScript helper.aws-actions/configure-aws-credentialsv4 → v6 (node20 → node24). The only breaking change in v6 is the Node bump; inputs (audience,role-to-assume,aws-region) are unchanged. Pinned by SHA to v6.2.0.Replaces
dkershner6/aws-ssm-getparameters-action(node20, no upstream update) withactions/release-secrets/ssm— a small TypeScript action bundled todist/index.jswithncc, run vianode dist/index.js. It parses thessm_parameter_pairsformat, fetches parameters withWithDecryption(chunked to the 10-name API limit), masks values via@actions/core, validates every requested path resolved before exporting anything, and exports viacore.exportVariable.Masking and multiline-safe
$GITHUB_ENVwrites are handled by@actions/core/ the runner rather than hand-rolled shell:core.setSecretalready masks both the whole value and each line of a multiline secret (the runner'sadd-maskhandler splits on newlines since actions/runner#1521), andcore.exportVariablewrites theGITHUB_ENVheredoc.Supply-chain hardening
packageManagerfield) instead of npm..yarnrc.ymlsetsnpmMinimalAgeGate: "7d", so no package version newer than 7 days is ever resolved — mirroring the org RenovateminimumReleaseAge: "7 days"policy.nodeLinker: node-moduleskeepsnccbundling working.yarn npm audit --allis clean. This also resolves the Semgrep findings on the (dev-only)vitest/vite/esbuildtransitives.actions/checkout@v6.0.3,actions/setup-node@v6.4.0).CI
.github/workflows/test-release-secrets-ssm.ymlruns on changes to the action:yarn install --immutable,yarn typecheck,yarn test(vitest, AWS SDK mocked), and a check-dist step that rebuilds and fails if the committeddist/is stale. The build is deterministic; notedist/is now multiple co-located chunks (newer AWS SDK + ncc code-splitting), all committed alongsideindex.js.How to test
Use a workflow that calls
release-secretsand verify:Link to Devin session: https://app.devin.ai/sessions/8bbd83e7ff0a489a9b06f810dab17d52
Requested by: @kinyoklion