Skip to content

Commit 3261b6b

Browse files
authored
fix(deployment): parse secretsmanager whole-secret dynamic-reference form (#860)
1 parent d81db9f commit 3261b6b

18 files changed

Lines changed: 761 additions & 11 deletions

docs/_generated/integ-coverage.json

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2605,6 +2605,7 @@
26052605
"s3-asset-deploy",
26062606
"scheduled-task",
26072607
"sdk-ccapi-crossref",
2608+
"secrets-dynamic-ref",
26082609
"serverless-api",
26092610
"sns-sqs-event",
26102611
"stepfunctions",
@@ -2800,6 +2801,9 @@
28002801
"l1",
28012802
"literal"
28022803
],
2804+
"secrets-dynamic-ref": [
2805+
"l2"
2806+
],
28032807
"serverless-api": [
28042808
"l2"
28052809
],
@@ -3558,6 +3562,7 @@
35583562
"schema-v6-to-v7-migration",
35593563
"schema-v7-to-v8-migration",
35603564
"sdk-ccapi-crossref",
3565+
"secrets-dynamic-ref",
35613566
"state-info-command",
35623567
"tags-propagation",
35633568
"update-policy-mutations",
@@ -3677,6 +3682,9 @@
36773682
"l1",
36783683
"literal"
36793684
],
3685+
"secrets-dynamic-ref": [
3686+
"l2"
3687+
],
36803688
"state-info-command": [
36813689
"l2"
36823690
],
@@ -3699,7 +3707,8 @@
36993707
"composite-stack",
37003708
"event-driven",
37013709
"full-stack-demo",
3702-
"local-run-task-from-state"
3710+
"local-run-task-from-state",
3711+
"secrets-dynamic-ref"
37033712
],
37043713
"signals": {
37053714
"composite-stack": [
@@ -3715,6 +3724,9 @@
37153724
"local-run-task-from-state": [
37163725
"l2",
37173726
"literal"
3727+
],
3728+
"secrets-dynamic-ref": [
3729+
"l2"
37183730
]
37193731
}
37203732
},

docs/_generated/scenario-coverage.json

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,10 @@
6464
"tag": "drift-revert-roundtrip",
6565
"description": "cdkd drift detection + `--revert` round-trip via each provider.update()."
6666
},
67+
{
68+
"tag": "dynamic-reference-resolution",
69+
"description": "CloudFormation dynamic references (`{{resolve:secretsmanager:...}}` / `{{resolve:ssm:...}}`) resolved by cdkd itself (`resolveDynamicReferences`) BEFORE the property reaches the provider — JSON-key (`:SecretString:<key>`), whole-secret, and version-stage forms + plaintext SSM param; the deployed resource carries the RESOLVED value, never the literal token. (`ssm-secure:` is NOT resolved by cdkd and is intentionally out of scope.)"
70+
},
6771
{
6872
"tag": "eventsourcemapping-fresh-source-race",
6973
"description": "`AWS::Lambda::EventSourceMapping` created against a FRESH source (SQS/Kinesis/DynamoDB-stream) + a FRESH execution role in the SAME deploy: the ESM create races source-readiness + role/policy propagation (cdkd dispatches with no level barrier), AND the orphan-ESM-on-redeploy collision class (a killed mid-deploy leaves an out-of-state ESM that collides on the next CREATE). The fixture pre-flight-scans for orphan ESMs by stack name, asserts the ESM reaches Enabled + actually delivers a probe message to the Lambda, and asserts no orphan ESM survives destroy."
@@ -1106,6 +1110,13 @@
11061110
"sdk-ccapi-crossref-boundary"
11071111
]
11081112
},
1113+
{
1114+
"name": "secrets-dynamic-ref",
1115+
"annotated": true,
1116+
"scenarios": [
1117+
"dynamic-reference-resolution"
1118+
]
1119+
},
11091120
{
11101121
"name": "serverless-api",
11111122
"annotated": true,
@@ -1322,6 +1333,13 @@
13221333
"drift-revert-vpc"
13231334
]
13241335
},
1336+
{
1337+
"scenario": "dynamic-reference-resolution",
1338+
"description": "CloudFormation dynamic references (`{{resolve:secretsmanager:...}}` / `{{resolve:ssm:...}}`) resolved by cdkd itself (`resolveDynamicReferences`) BEFORE the property reaches the provider — JSON-key (`:SecretString:<key>`), whole-secret, and version-stage forms + plaintext SSM param; the deployed resource carries the RESOLVED value, never the literal token. (`ssm-secure:` is NOT resolved by cdkd and is intentionally out of scope.)",
1339+
"fixtures": [
1340+
"secrets-dynamic-ref"
1341+
]
1342+
},
13251343
{
13261344
"scenario": "eventsourcemapping-fresh-source-race",
13271345
"description": "`AWS::Lambda::EventSourceMapping` created against a FRESH source (SQS/Kinesis/DynamoDB-stream) + a FRESH execution role in the SAME deploy: the ESM create races source-readiness + role/policy propagation (cdkd dispatches with no level barrier), AND the orphan-ESM-on-redeploy collision class (a killed mid-deploy leaves an out-of-state ESM that collides on the next CREATE). The fixture pre-flight-scans for orphan ESMs by stack name, asserts the ESM reaches Enabled + actually delivers a probe message to the Lambda, and asserts no orphan ESM survives destroy.",

docs/changelog-cdkd.md

Lines changed: 1 addition & 0 deletions
Large diffs are not rendered by default.

docs/integ-coverage.md

Lines changed: 3 additions & 3 deletions
Large diffs are not rendered by default.

docs/scenario-coverage.md

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44

55
Run `vp run scenario-coverage` to regenerate.
66

7-
**64 / 64 canonical scenarios** have at least one integ fixture exercising them. **131 / 150 integ fixtures** carry a `.scenarios.json` sidecar (with 0+ tags); the rest are un-annotated and contributor-reviewed below.
7+
**65 / 65 canonical scenarios** have at least one integ fixture exercising them. **132 / 151 integ fixtures** carry a `.scenarios.json` sidecar (with 0+ tags); the rest are un-annotated and contributor-reviewed below.
88

99
## How this is computed
1010

@@ -26,7 +26,7 @@ This report is a visibility tool, not a commit-time gate. Many cdkd fixtures leg
2626

2727
_None._ Every canonical scenario has at least one integ fixture tagged with it.
2828

29-
## Per-scenario coverage (64 scenarios)
29+
## Per-scenario coverage (65 scenarios)
3030

3131
| Scenario | Description | Integ Fixture(s) |
3232
|---|---|---|
@@ -46,6 +46,7 @@ _None._ Every canonical scenario has at least one integ fixture tagged with it.
4646
| `docker-image-asset-ecr-publish` | cdkd's deploy-time Docker ASSET pipeline (`DockerAssetPublisher`): `docker build` of a local Dockerfile -> ECR auth -> `docker push` to the CDK-managed container-assets repo, then an `AWS::Lambda::Function` with `PackageType=Image` pointing at the pushed image. Distinct from the local-emulation container scenarios (which never touch AWS) — this verifies the real build+push happens during `cdkd deploy`, the image runs (Lambda invoke), and the pushed image is gone after destroy. | [`docker-image-asset`](../tests/integration/docker-image-asset/) |
4747
| `drift-revert-array-canonicalization` | cdkd drift no-false-positive on tag-list / resource-id / ARN array REORDER (issue #802 `drift-normalize.ts` canonicalization) while still detecting real value / Action / SG-rule drift. | [`drift-revert-arrays`](../tests/integration/drift-revert-arrays/) |
4848
| `drift-revert-roundtrip` | cdkd drift detection + `--revert` round-trip via each provider.update(). | [`drift-revert`](../tests/integration/drift-revert/)<br>[`drift-revert-arrays`](../tests/integration/drift-revert-arrays/)<br>[`drift-revert-vpc`](../tests/integration/drift-revert-vpc/) |
49+
| `dynamic-reference-resolution` | CloudFormation dynamic references (`{{resolve:secretsmanager:...}}` / `{{resolve:ssm:...}}`) resolved by cdkd itself (`resolveDynamicReferences`) BEFORE the property reaches the provider — JSON-key (`:SecretString:<key>`), whole-secret, and version-stage forms + plaintext SSM param; the deployed resource carries the RESOLVED value, never the literal token. (`ssm-secure:` is NOT resolved by cdkd and is intentionally out of scope.) | [`secrets-dynamic-ref`](../tests/integration/secrets-dynamic-ref/) |
4950
| `eventsourcemapping-fresh-source-race` | `AWS::Lambda::EventSourceMapping` created against a FRESH source (SQS/Kinesis/DynamoDB-stream) + a FRESH execution role in the SAME deploy: the ESM create races source-readiness + role/policy propagation (cdkd dispatches with no level barrier), AND the orphan-ESM-on-redeploy collision class (a killed mid-deploy leaves an out-of-state ESM that collides on the next CREATE). The fixture pre-flight-scans for orphan ESMs by stack name, asserts the ESM reaches Enabled + actually delivers a probe message to the Lambda, and asserts no orphan ESM survives destroy. | [`eventsourcemapping-race`](../tests/integration/eventsourcemapping-race/) |
5051
| `export-to-cfn-handover` | cdkd → CloudFormation migration via 2-phase IMPORT changeset + phase-2 UPDATE. | [`export`](../tests/integration/export/) |
5152
| `exports-index-region-resolve` | Exports index store (`Fn::ImportValue` tracking, `_index/{region}/exports.json`) auto-detects the bucket region via `GetBucketLocation` before its write/remove, so a cross-region state bucket no longer hits S3 301 PermanentRedirect (issue #819). | [`cross-region-state-bucket`](../tests/integration/cross-region-state-bucket/) |

scripts/build-scenario-coverage-matrix.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,10 @@ const KNOWN_SCENARIOS: Record<string, string> = {
126126
'sdk-ccapi-crossref-boundary':
127127
'Heterogeneous SDK-Provider <-> Cloud Control API routing in ONE stack (a silent-drop top-level property flips a resource to the CC path per #614) with `Fn::GetAtt` cross-references crossing the boundary in BOTH directions — SDK-routed consumer reads a CC-routed producer attribute AND CC-routed consumer reads an SDK-routed producer attribute. Exercises the constructAttribute fallback for CC-API physical-id shapes (memory `feedback_silent_drop_forces_cc_api_routing`) and the CC delete path bypassing the SDK provider delete() (memory `feedback_cc_api_routing_bypasses_sdk_delete_logic`).',
128128

129+
// ---- Dynamic reference patterns ----
130+
'dynamic-reference-resolution':
131+
'CloudFormation dynamic references (`{{resolve:secretsmanager:...}}` / `{{resolve:ssm:...}}`) resolved by cdkd itself (`resolveDynamicReferences`) BEFORE the property reaches the provider — JSON-key (`:SecretString:<key>`), whole-secret, and version-stage forms + plaintext SSM param; the deployed resource carries the RESOLVED value, never the literal token. (`ssm-secure:` is NOT resolved by cdkd and is intentionally out of scope.)',
132+
129133
// ---- Custom Resource patterns ----
130134
'custom-resource-async-poll':
131135
'Custom Resource backed by Lambda + cfn-response via S3 pre-signed URL polling.',

src/deployment/intrinsic-function-resolver.ts

Lines changed: 28 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2442,6 +2442,13 @@ export class IntrinsicFunctionResolver {
24422442
* SECRET_ID can be a simple name or an ARN (arn:aws:secretsmanager:REGION:ACCOUNT:secret:NAME)
24432443
* which contains colons, so we cannot simply split on ':'.
24442444
* Instead, we find ':SecretString:' or ':SecretBinary:' as the delimiter.
2445+
*
2446+
* The whole-secret form omits everything after the type segment and carries no trailing
2447+
* colon: "secretsmanager:SECRET_ID:SecretString" (returns the full secret string). We detect
2448+
* it with an end-anchored check so that, for the whole-secret form, a SECRET_ID that merely
2449+
* contains ":SecretString" mid-name is not split incorrectly. (The end-anchored fallback only
2450+
* runs when no mid-string ":SecretString:" delimiter is present, so the json-key / version
2451+
* forms are unaffected.)
24452452
*/
24462453
private async resolveSecretsManagerReference(inner: string): Promise<string> {
24472454
// inner = "secretsmanager:SECRET_ID:SecretString:JSON_KEY:VERSION_STAGE:VERSION_ID"
@@ -2454,8 +2461,24 @@ export class IntrinsicFunctionResolver {
24542461
let versionStage = '';
24552462
let versionId = '';
24562463

2457-
const secretStringIdx = afterService.indexOf(':SecretString:');
2458-
const secretBinaryIdx = afterService.indexOf(':SecretBinary:');
2464+
let secretStringIdx = afterService.indexOf(':SecretString:');
2465+
let secretBinaryIdx = afterService.indexOf(':SecretBinary:');
2466+
let delimiterLenAtBinary = ':SecretBinary:'.length;
2467+
let delimiterLenAtString = ':SecretString:'.length;
2468+
2469+
// Whole-secret form: "<SECRET_ID>:SecretString" / "<SECRET_ID>:SecretBinary" with NO
2470+
// trailing colon and no JSON_KEY (end of string). The trailing-colon indexOf above misses
2471+
// it, so fall back to an END-ANCHORED check. An end-anchored check (not a loose includes)
2472+
// avoids a false split when a secret NAME legitimately contains ":SecretString" mid-name.
2473+
if (secretStringIdx < 0 && afterService.endsWith(':SecretString')) {
2474+
secretStringIdx = afterService.length - ':SecretString'.length;
2475+
delimiterLenAtString = ':SecretString'.length;
2476+
}
2477+
if (secretBinaryIdx < 0 && afterService.endsWith(':SecretBinary')) {
2478+
secretBinaryIdx = afterService.length - ':SecretBinary'.length;
2479+
delimiterLenAtBinary = ':SecretBinary'.length;
2480+
}
2481+
24592482
const delimiterIdx =
24602483
secretStringIdx >= 0 && secretBinaryIdx >= 0
24612484
? Math.min(secretStringIdx, secretBinaryIdx)
@@ -2464,12 +2487,12 @@ export class IntrinsicFunctionResolver {
24642487
: secretBinaryIdx;
24652488
const delimiterLen =
24662489
delimiterIdx >= 0 && delimiterIdx === secretBinaryIdx
2467-
? ':SecretBinary:'.length
2468-
: ':SecretString:'.length;
2490+
? delimiterLenAtBinary
2491+
: delimiterLenAtString;
24692492

24702493
if (delimiterIdx >= 0) {
24712494
secretId = afterService.substring(0, delimiterIdx);
2472-
// remaining = "JSON_KEY:VERSION_STAGE:VERSION_ID"
2495+
// remaining = "JSON_KEY:VERSION_STAGE:VERSION_ID" (empty for the whole-secret form)
24732496
const remaining = afterService.substring(delimiterIdx + delimiterLen);
24742497
const remainingParts = remaining.split(':');
24752498
jsonKey = remainingParts[0] || '';
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
*.js
2+
!jest.config.js
3+
*.d.ts
4+
node_modules
5+
6+
# CDK asset staging directory
7+
.cdk.staging
8+
cdk.out
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
{
2+
"scenarios": ["dynamic-reference-resolution"]
3+
}
Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
# secrets-dynamic-ref
2+
3+
Failure-seeking integration test for CloudFormation **dynamic references** in
4+
resource properties:
5+
6+
- `{{resolve:secretsmanager:...}}`
7+
- `{{resolve:ssm:...}}`
8+
9+
cdkd resolves these itself in `resolveDynamicReferences`
10+
([src/deployment/intrinsic-function-resolver.ts](../../../src/deployment/intrinsic-function-resolver.ts))
11+
BEFORE the property is handed to the provider — CloudFormation never sees the
12+
literal `{{resolve:...}}` token. This test surfaces bugs where a dynamic
13+
reference resolves to the **wrong value** or **stays literal** in the deployed
14+
resource.
15+
16+
## Stack
17+
18+
`CdkdSecretsDynamicRefExample` (cheap, no VPC):
19+
20+
- A SecretsManager secret with a **known JSON value**
21+
(`{"username":"cdkd-user","password":"cdkd-known-pw-123"}`).
22+
- An SSM `String` parameter with a **known value** (`cdkd-known-ssm-value`).
23+
- A consumer `AWS::Lambda::Function` (inline code, asset-free) whose
24+
**environment variables** are literal `{{resolve:...}}` dynamic-reference
25+
strings. The handler is never invoked; `verify.sh` reads
26+
`GetFunctionConfiguration` and asserts each env var carries the resolved
27+
value.
28+
29+
The env-var values are authored as literal `{{resolve:...}}` strings (CDK
30+
emits them as `Fn::Join` arrays interpolating `AWS::AccountId`), NOT via CDK's
31+
`secretValueFromJson` token — so the test pins the exact dynamic-reference
32+
grammar regardless of the CDK version's token shape.
33+
34+
## Dynamic-reference forms exercised
35+
36+
| Form | Example | cdkd support |
37+
| --- | --- | --- |
38+
| secretsmanager JSON-key | `{{resolve:secretsmanager:NAME:SecretString:password}}` | SUPPORTED |
39+
| secretsmanager whole-secret | `{{resolve:secretsmanager:NAME:SecretString}}` | SUPPORTED |
40+
| secretsmanager version-stage | `{{resolve:secretsmanager:NAME:SecretString:password:AWSCURRENT}}` | SUPPORTED |
41+
| ssm plaintext param | `{{resolve:ssm:NAME}}` | SUPPORTED |
42+
| ssm-secure SecureString | `{{resolve:ssm-secure:NAME}}` | **NOT** resolved by cdkd — out of scope (see below) |
43+
44+
`ssm-secure` is intentionally **not** exercised: cdkd's
45+
`resolveDynamicReferences` routes only `secretsmanager` and `ssm`; an
46+
`ssm-secure:` reference hits the `else` branch (warn + leave literal), so it
47+
would deploy a broken value. A secret **version-ID** form
48+
(`...:SecretString:key::<uuid>`) is also not exercised because the version id
49+
is not knowable ahead of deploy; the version-**stage** slot (`AWSCURRENT`)
50+
covers the optional-trailing-field grammar.
51+
52+
## What verify.sh asserts
53+
54+
1. Deploy the stack with the local cdkd binary.
55+
2. Read the consumer Lambda's env vars via `GetFunctionConfiguration`.
56+
3. For each env var: it is **not** still a literal `{{resolve:...}}` token, AND
57+
it equals the known expected value. A wrong-or-literal value FAILS with
58+
specifics.
59+
4. Destroy, then assert the Lambda, secret, SSM parameter, and state file are
60+
all gone.
61+
62+
**Security:** secret-derived values are never printed; assertions mask them
63+
(`xx***(len=N)`). Only PASS/FAIL plus a masked snippet appears in the log.
64+
65+
## Run
66+
67+
```bash
68+
vp run build # from repo root — verify.sh runs node dist/cli.js
69+
/run-integ secrets-dynamic-ref
70+
```
71+
72+
`verify.sh` requires `STATE_BUCKET` (e.g. `cdkd-state-{accountId}`) and honors
73+
`AWS_REGION` (defaults to `us-east-1`).

0 commit comments

Comments
 (0)