Skip to content

Commit d97cf5a

Browse files
committed
fix(deployment): gate {Condition: X} named-condition reference on conditionResolver presence
The `{Condition: <name>}` named-condition reference branch in `resolveValue` fired for every property, not just inside a condition definition. A resource/output property whose value was exactly a single-key `{ "Condition": "<string>" }` object was silently coerced to a boolean via `resolveConditionReference` — and in normal property context (no `conditionResolver`, name absent from `context.conditions`) that returns `false`, corrupting the property. The reference form is only reachable during `evaluateConditions`, the sole code path that threads a `conditionResolver` hook onto the context. Gate the branch on `context.conditionResolver` being present so a resource property literally named `Condition` is never coerced to false; it now falls through and resolves as an ordinary object, exactly as before the #840 change. The single-key + `typeof string` guards remain as defense in depth. Composite-condition behavior is unaffected (it always runs with `conditionResolver` set). Adds a unit test pinning that a single-key `{ Condition: "Foo" }` property resolved in normal context stays a plain object and is not `false` (review fix).
1 parent 49a6f53 commit d97cf5a

2 files changed

Lines changed: 34 additions & 3 deletions

File tree

src/deployment/intrinsic-function-resolver.ts

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -528,10 +528,20 @@ export class IntrinsicFunctionResolver {
528528
// `{Condition: <name>}` — a named-condition reference. Valid only inside
529529
// another condition's definition (`Fn::And` / `Fn::Or` / `Fn::Not`),
530530
// where it resolves to the referenced condition's evaluated boolean
531-
// (issue #840). Guard against false positives: only treat a single-key
532-
// `{Condition: "<string>"}` object as the reference, never a resource
533-
// property literally named `Condition` alongside siblings.
531+
// (issue #840). This form is ONLY reachable during `evaluateConditions`,
532+
// which is the sole code path that threads a `conditionResolver` hook onto
533+
// the context — so gate the branch on `context.conditionResolver` being
534+
// present. Outside that context (a normal resource / output property), a
535+
// single-key `{Condition: "<string>"}` object is a plain property literally
536+
// named `Condition`, NOT an intrinsic, and must fall through to be resolved
537+
// as an ordinary object — otherwise it would be silently coerced to a
538+
// boolean (and to `false`, since neither the resolver hook nor the
539+
// already-evaluated `conditions` map is available there), corrupting the
540+
// property. The single-key + `typeof string` guards remain as defense in
541+
// depth so even inside `evaluateConditions` a composite-condition object
542+
// carrying sibling keys is never misread as the reference form.
534543
if (
544+
context.conditionResolver &&
535545
'Condition' in obj &&
536546
Object.keys(obj).length === 1 &&
537547
typeof obj['Condition'] === 'string'

tests/unit/deployment/intrinsic-functions.test.ts

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1943,4 +1943,25 @@ describe('IntrinsicFunctionResolver - evaluateConditions composite refs (#840)',
19431943
);
19441944
expect(result).toEqual({ Condition: 'SomeString', Other: 'pval' });
19451945
});
1946+
1947+
it('does not coerce a single-key { Condition: "X" } resource property to a boolean in normal context', async () => {
1948+
// A single-key `{ Condition: '<string>' }` object IS the named-condition
1949+
// reference form — but only inside `evaluateConditions`, which threads a
1950+
// `conditionResolver` hook onto the context. In a normal resource / output
1951+
// property context (the public `resolve` entry, no conditionResolver) the
1952+
// same shape is just a plain property literally named `Condition` and MUST
1953+
// resolve as an ordinary object — never get coerced to `false` by
1954+
// resolveConditionReference (which, lacking a resolver hook AND a
1955+
// `conditions` map, would otherwise return false and corrupt the property).
1956+
const result = await resolver.resolve(
1957+
{ Condition: 'Foo' },
1958+
{
1959+
template: { Resources: {} },
1960+
resources: {},
1961+
parameters: {},
1962+
}
1963+
);
1964+
expect(result).toEqual({ Condition: 'Foo' });
1965+
expect(result).not.toBe(false);
1966+
});
19461967
});

0 commit comments

Comments
 (0)