feat(aws): add vault expression support for credentials (#474) #41
Loading…
Reference in a new issue
No description provided.
Delete branch "feat/aws-vault-credentials-474"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Summary
accessKeyId,secretAccessKey,sessionToken) and non-sensitiveregionto all AWS CloudControl models via the codegen pipelineAWS_ACCESS_KEY_ID,AWS_SECRET_ACCESS_KEY,AWS_SESSION_TOKEN,AWS_REGION)vault.get(...)expressions in model YAML, mirroring the pattern from Hetzner Cloud (#471)Codegen changes
codegen/aws/libGenerator.ts—AwsCredentialsinterface,createClient()accepts optional credentials, all CRUD functions thread credentialscodegen/aws/extensionModelGenerator.ts— inject credential global args with collision guards,_buildCredentialshelper,_credentialKeysset for filtering desiredState, credential threading in all methodscodegen/aws/pipeline.ts— add credential field names tonewFieldNamesfor upgrade diffingcodegen/designs/aws.md— document authentication and credential global argumentsGenerated output
GlobalArgsSchemaandInputsSchemaz.meta({ sensitive: true })in both schemas;regionis not sensitiveAdded: accessKeyId, secretAccessKey, sessionToken, region2026.06.06.1across all affected servicesTest plan
deno check main.ts— codegen type-checksdeno lint/deno fmt --check— codegen passes lint and formatgenerate:aws ec2 s3 lambda) confirms targeted fixdeno check,deno lint,deno fmt2026.05.27.1upgrades to2026.06.06.1viaswamp extension source add— no version mismatch errorKnown limitation
Hand-written list method enrichments (e.g.
codegen/aws/enrichments/rds-dbcluster-list.enrich.ts) create their own SDK clients and do not receive vault-expression credentials. This can be addressed in a follow-up issue.Closes #474
Adversarial Review
Medium
_buildCredentialsignores the collision guard —codegen/aws/extensionModelGenerator.ts:305-316The collision guard at lines 186-219 correctly removes credential fields from
injectedCredFields(and thus from_credentialKeysand the Zod schemas) when a domain property name collides. However,_buildCredentials(lines 305-316) is emitted identically for every resource — it always reads all four fields (g.accessKeyId,g.secretAccessKey,g.sessionToken,g.region) fromglobalArgs, regardless of which credential fields were actually injected.Breaking scenario: Suppose a CF resource had a camelCase domain property named
region(unlikely with PascalCase conventions, but the guard exists to handle it). The collision guard would correctly:regionfrom_credentialKeys→ the domain property flows intodesiredState(correct).regionfrom the credential fields inGlobalArgsSchema→ no duplicate Zod field (correct).But
_buildCredentialswould still readg.region, interpret the domain property value as an AWS region, and pass it tocreateClient. The user would get API calls routed to an invalid/wrong region.Suggested fix: Generate
_buildCredentialsbody conditionally based oninjectedCredFields, only reading fields that were actually injected:Pipeline collision check uses a different data source than the generator —
codegen/aws/pipeline.ts:360-379vscodegen/aws/extensionModelGenerator.ts:186-190The pipeline checks
Object.keys(domainProperties)(the raw CF schema property names) while the generator parseszodResult.inputSchemaBodywith a regex (/^(\w+)\s*:/). These are derived from the same underlying CF schema so they should produce the same names, but they use fundamentally different mechanisms: one walks the schema object keys, the other parses generated Zod code with a regex. If the Zod generator ever renames or transforms property names, these two checks would diverge silently, causing the upgrade differ to include credential fields the generator actually excluded (or vice versa).Suggested fix: Extract the collision check into a shared pure function that both pipeline.ts and extensionModelGenerator.ts call with the same input (e.g.,
Object.keys(domainProperties)), rather than having two independent implementations.Low
Partial credentials silently fall through —
codegen/aws/libGenerator.ts:32-37createClientchecksif (credentials?.accessKeyId && credentials?.secretAccessKey)before setting explicit credentials. If a user providesaccessKeyIdwithoutsecretAccessKey(e.g., a typo in the vault expression that only wires one field), the explicit credentials are silently ignored and the SDK falls through to the default chain. The user would unknowingly authenticate as the wrong identity (the EC2 instance role, for example) rather than getting a clear error.This is unlikely to cause data loss (the wrong identity would likely lack permissions), but a
console.warnfor partial credentials would help debugging.listmethod does not pass credentials —codegen/aws/extensionModelGenerator.ts:677-716The list method enrichment doesn't receive credentials. The design doc acknowledges this as a known limitation, so this is informational. Users relying on vault-expression credentials who also use list methods would silently fall back to env-var authentication.
Verdict
PASS — The core credential injection logic is sound and well-structured. The
_buildCredentialscollision asymmetry (Medium #1) is a real logic error but cannot trigger in practice with current CloudFormation PascalCase conventions. The pipeline/generator divergent collision checks (Medium #2) are a maintainability concern. Neither blocks this merge.Code Review
Blocking Issues
Stale snapshot tests for
extensionModelGenerator—codegen/aws/__snapshots__/extensionModelGenerator_test.ts.snapdoes not reflect the credential injection feature present in the currentextensionModelGenerator.ts. The snapshots lack:accessKeyId,secretAccessKey,sessionToken,regionfields inGlobalArgsSchemaandInputsSchemaimport type { AwsCredentials } from "./_lib/aws.ts";const _credentialKeys = new Set(...)andfunction _buildCredentials(...)credentialsargument passed tocreateResource,readResource,updateResource,deleteResourcecalls in generated methodsRunning
deno testincodegen/aws/will produce snapshot mismatches for all 7 snapshot test cases. Snapshots need to be regenerated withdeno test --update-snapshots.Suggestions
Stale file path references in
codegen/designs/aws.md— Section 9 refers tosrc/codegen/zodGenerator.tsand section 10 tosrc/pipeline/version.ts. The actual paths arecodegen/shared/zodGenerator.tsandcodegen/shared/version.tsrespectively (confirmed by the imports inpipeline.ts).Redundant
const primaryIdshadowing —generateAwsExtensionModeldeclaresconst primaryIdat the function scope (line 296) and again inside theif (input.listMethod)block (line 679), computing the same value. The inner declaration can be removed since it's already in scope.Code Review
Blocking Issues
None.
Suggestions
No test for credential collision case (
extensionModelGenerator_test.ts): There's no snapshot or unit test covering the case where a CF domain property name collides with a credential field name (e.g., a hypothetical resource with anaccessKeyIdproperty). The collision guard ingenerateAwsExtensionModel(lines 215–217) would silently omit that credential field; a test would document and protect that behavior.getInjectedCredentialFieldshas no direct unit test (extensionModelGenerator.ts:812–817): The function is only exercised indirectly through the generator snapshots. A direct test (similar to theresolveNamingFieldunit tests) would make its contract explicit and protect against regression when theAWS_CREDENTIAL_FIELD_NAMESconstant is extended.enrichStateusesStateDatabefore itstypedeclaration (snapshot line ~1225 and generated enrichment output): The enrichment body is emitted beforeStateSchema/type StateData. TypeScript resolves type aliases throughout the entire file scope, so this compiles correctly, but the ordering is subtly surprising. The design doc doesn't call this out. No action required — noting for awareness.Polling timeout in
pollOperationStatus(libGenerator.ts:111): The poll loop runs up to 60 iterations with exponential backoff capped at 90 s, giving a potential worst-case wait of ~90 minutes. This is intentional per the design doc, but there's no configurable override for the timeout. Not a bug — mentioning in case operator experience reveals a need to tune it.Adversarial Review
Medium
codegen/aws/libGenerator.ts:30(generated_lib/aws.ts) —configtyped asRecord<string, unknown>bypasses CloudControlClient type checkingThe
createClientfunction constructsconfigasRecord<string, unknown>and passes it tonew CloudControlClient(config). This silently bypasses TypeScript's compile-time check for invalid fields. If a typo is introduced (e.g.,credentialinstead ofcredentials), the SDK will silently ignore the malformed config and fall back to the default credential chain — the user's vault-provided credentials will not be used, and the operation will succeed or fail against the wrong AWS account.Breaking scenario: A future edit accidentally sets
config.credential = { ... }(singular). No type error, no runtime error — operations silently run under the wrong identity.Suggested fix: Use
CloudControlClientConfigfrom the SDK to typeconfig, or construct the config object inline with thenew CloudControlClient()call so the type is checked.codegen/aws/libGenerator.ts:36-43(generated_lib/aws.ts) — partial credentials silently degrade to default chainIf a user provides
accessKeyIdin their model YAML but forgetssecretAccessKey, the code logs aconsole.warnand falls back to the default credential chain. The operation proceeds under a potentially different AWS identity than intended.Breaking scenario: User sets
accessKeyId: ${{ vault.get(v, key) }}butsecretAccessKeyvault reference resolves toundefineddue to a typo. The warn goes to console (easy to miss in automated runs), and the resource is created in the wrong account. This is a security-relevant silent degradation.Suggested fix: Consider throwing an error instead of warning when credentials are partially provided. The user clearly intended explicit credentials — silently falling back defeats the purpose.
codegen/aws/extensionModelGenerator.ts:297-299—_credentialKeysset is computed frominjectedCredFields, which may be a subset of all credential field names if collisions existIf a CF resource has a domain property named
region(unlikely but possible — the collision guard filters it), thenregionwon't be in_credentialKeys. In thecreateandupdatemethods, thefor (const [key, value] of Object.entries(g))loop will includeregionindesiredState— but that's actually correct since in a collision scenario,regionIS a domain property. The_buildCredentialsfunction correctly sets the colliding field toundefined. This is handled correctly, noting for completeness.codegen/aws/enrichments/rds-dbcluster.enrich.ts:25-26— enrichment creates its ownRDSClientignoring vault credentials (acknowledged known limitation)The enrichment function
enrichStatecreates its ownRDSClientusing onlyDeno.env.get("AWS_REGION"). When vault-provided credentials are used viaglobalArgs, this enrichment will use a different credential chain than the CloudControl operations. Thegetandsyncmethods callreadResource(with credentials) and thenenrichState(without credentials), so the enrichment may fail or read from a different account.The design doc explicitly calls this out as a known limitation, so not blocking, but worth noting: a user providing explicit credentials who uses
getorsyncon an RDS cluster will get a confusing partial failure if the default chain credentials don't haverds:DescribeDBClusterspermission.Low
codegen/aws/extensionModelGenerator.ts:228— credential field descriptions contain unescaped double quotes riskThe
descstrings are injected via"${f.desc}"into generated.describe(...)calls. Currently these strings are static and safe, but if a future edit adds a double quote in a description, it would produce invalid generated TypeScript. A template literal or JSON.stringify would be more robust.codegen/aws/extensionModelGenerator.ts— no test for the domain property collision guardAll existing test cases pass
domainPropertyNamesthat don't collide with["accessKeyId", "secretAccessKey", "sessionToken", "region"]. There's no test verifying the collision guard works — e.g., a resource with a domain property namedregionshould not haveregioninjected as a credential field. The probability of a real collision is extremely low (CF uses PascalCase), but the guard exists, so it should have a test.codegen/aws/libGenerator.ts:30—createClientcreates a new SDK client on every CRUD callEach
create,read,update, anddeleteinvocation constructs a freshCloudControlClient. This is the existing pattern (pre-PR), but with credentials now flowing through, it's worth noting that connection pooling is not reused across operations in a single model invocation (e.g.,createcalls bothCreateResourceCommandand thenGetResourceCommandon the same client, which is fine — butupdatecallsreadResource+updateResourcewhich create two separate clients).Verdict
PASS — The core credential injection mechanism is well-designed with proper collision guards, sensitive field marking, and clean separation between auth config and domain state. The partial-credentials silent fallback (Medium #2) is the most concerning behavior, but it's a warning-level issue, not a data-loss or security-vulnerability scenario in isolation since the AWS SDK itself will reject requests with invalid default-chain credentials. The enrichment limitation is explicitly documented. No blocking issues found.