Skip to content

Commit 45ccff0

Browse files
fix(lib): escape object keys in expressions and hcl maps (#569)
### Related issue Fixes #350 ### Description Object keys in expressions and HCL maps are now serialized through one shared quoted-string helper, `quotedStringKey`. Before this change, the direct-object branches of `resolveExpressionPart` and the HCL map renderer interpolated keys without escaping. - Keys escape quotes, backslashes, control characters, `${` (to `$${`) and `%{` (to `%%{`). - Backspace and form feed use `\u0008` and `\u000c`. Terraform quoted strings reject the `\b` and `\f` escapes that `JSON.stringify` emits. - HCL map keys keep `${...}`, so token references stay interpolations. - A key built from a token renders as a parenthesised key expression, for example `(resource.id) = "v"`. HCL has no interpolation syntax for object keys. - A key that mixes literal text and a token escapes the literal fragment with the same rules as a plain key. It then emits the token as `${...}`, for example `("pre-${resource.id}") = "v"`. - A property whose value resolves to `undefined` (for example `Lazy.anyValue`) is omitted. This matches the existing behavior for a literal `undefined` value. **Behavior change:** a literal `${...}` key in an `Fn.*` object argument is now escaped and kept as text. It is no longer evaluated as an interpolation. For example, `Fn.keys({ "${resource.id}": "v" })` used to resolve the reference. It now keeps the text `${resource.id}` as the key. Configurations that relied on the old evaluation will show a plan diff. To use a reference as a key, pass the token itself, for example `{ [resource.id]: "v" }`. ### Checklist - [x] I have updated the PR title to match [CDKTN's style guide](https://github.com/open-constructs/cdk-terrain/blob/main/CONTRIBUTING.md#pull-requests-1) - [x] I have run the linter on my code locally - [x] I have performed a self-review of my code - [x] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the [documentation](https://github.com/open-constructs/cdk-terrain-docs/tree/main/content) if applicable - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective or that my feature works if applicable - [x] New and existing unit tests pass locally with my changes
1 parent 856442e commit 45ccff0

5 files changed

Lines changed: 291 additions & 23 deletions

File tree

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
/**
2+
* Copyright (c) HashiCorp, Inc.
3+
* SPDX-License-Identifier: MPL-2.0
4+
*/
5+
6+
/**
7+
* Serializes an object key fragment as the inner content of a Terraform
8+
* quoted-string literal, without the surrounding quotes.
9+
*
10+
* `JSON.stringify` covers quotes, backslashes and most control characters.
11+
* Terraform quoted strings reject the `\b` and `\f` escapes that JSON emits, so
12+
* those use four-digit Unicode escapes. `$${` and `%%{` are the Terraform
13+
* escapes for sequences that would otherwise start a template interpolation or
14+
* directive.
15+
*
16+
* Set `keepTemplates` when the fragment may hold a resolved token, which
17+
* renders as a `${...}` reference that must stay an interpolation.
18+
*/
19+
export function quotedStringKeyFragment(
20+
fragment: string,
21+
keepTemplates = false,
22+
): string {
23+
// Replacer functions, not replacement strings: in a replacement string `$$`
24+
// is itself an escape for a literal `$` and would undo the escape below.
25+
const escaped = JSON.stringify(fragment)
26+
.slice(1, -1)
27+
.replace(/\\(\\|b|f)/g, (match, selector: string) =>
28+
selector === "b" ? "\\u0008" : selector === "f" ? "\\u000c" : match,
29+
);
30+
if (keepTemplates) {
31+
return escaped;
32+
}
33+
return escaped.replace(/\$\{/g, () => "$${").replace(/%\{/g, () => "%%{");
34+
}
35+
36+
/**
37+
* Serializes an object key as a Terraform quoted-string literal.
38+
*
39+
* Set `keepTemplates` when the key may hold a resolved token, which renders as
40+
* a `${...}` reference that must stay an interpolation.
41+
*/
42+
export function quotedStringKey(key: string, keepTemplates = false): string {
43+
return `"${quotedStringKeyFragment(key, keepTemplates)}"`;
44+
}

‎packages/cdktn/src/hcl/render.ts‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
* Copyright (c) HashiCorp, Inc.
33
* SPDX-License-Identifier: MPL-2.0
44
*/
5+
import { quotedStringKey } from "./quoted-string-key";
56
import { Tokenization } from "../tokens/token";
67

78
/**
@@ -19,7 +20,16 @@ function escapeQuotes(str: string): string {
1920
*
2021
*/
2122
function wrapIdentifierInQuotesIfNeeded(key: string): string {
22-
return /(^\d)|[^A-Za-z0-9_-]/.test(key) ? `"${key}"` : key;
23+
if (!/(^\d)|[^A-Za-z0-9_-]/.test(key)) {
24+
return key;
25+
}
26+
// A key containing `${` may be a resolved token reference rather than a
27+
// literal string, so its `${...}` body is expression syntax, not text to
28+
// escape. Leave it on the old (unescaped) path to avoid corrupting it.
29+
if (key.includes("${")) {
30+
return `"${key}"`;
31+
}
32+
return quotedStringKey(key, true);
2333
}
2434

2535
/**

‎packages/cdktn/src/tfExpression.ts‎

Lines changed: 86 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -7,27 +7,13 @@ import { App } from "./app";
77
import { TerraformStack } from "./terraform-stack";
88
import { ITerraformDependable } from "./terraform-dependable";
99
import { Construct } from "constructs";
10+
import {
11+
quotedStringKey,
12+
quotedStringKeyFragment,
13+
} from "./hcl/quoted-string-key";
1014

1115
const TERRAFORM_IDENTIFIER_REGEX = /^[_a-zA-Z][_a-zA-Z0-9]*$/;
1216

13-
/**
14-
* Serializes an object key as a Terraform quoted-string literal:
15-
* `JSON.stringify` covers quotes, backslashes and control characters, and
16-
* `$${`/`%%{` are Terraform's own escapes for what would otherwise become
17-
* template interpolation/directive sequences inside the quoted string. The
18-
* pre-existing direct-object branches of `resolveExpressionPart` still
19-
* interpolate keys unescaped - tracked in
20-
* https://github.com/open-constructs/cdk-terrain/issues/350.
21-
*/
22-
function quotedStringKey(key: string): string {
23-
// Replacer functions, not replacement strings: in a replacement string
24-
// `$$` is itself an escape (for a literal `$`), so `"$${"` would emit
25-
// just `${` - silently undoing the escape this exists to produce.
26-
return JSON.stringify(key)
27-
.replace(/\$\{/g, () => "$${")
28-
.replace(/%\{/g, () => "%%{");
29-
}
30-
3117
// eslint-disable-next-line jsdoc/require-jsdoc
3218
class TFExpression extends Intrinsic implements IResolvable {
3319
protected resolveExpressionPart(context: IResolveContext, arg: any): string {
@@ -73,7 +59,7 @@ class TFExpression extends Intrinsic implements IResolvable {
7359
return `{${Object.keys(resolvedArg)
7460
.map(
7561
(key) =>
76-
`${quotedStringKey(key)} = ${this.resolveArg(context, resolvedArg[key])}`,
62+
`${this.resolveObjectKey(context, key)} = ${this.resolveArg(context, resolvedArg[key])}`,
7763
)
7864
.join(", ")}}`;
7965
}
@@ -86,8 +72,21 @@ class TFExpression extends Intrinsic implements IResolvable {
8672
}
8773

8874
if (typeof resolvedArg === "object" && resolvedArg !== null) {
89-
return `{${Object.keys(resolvedArg)
90-
.map((key) => `"${key}" = ${this.resolveArg(context, arg[key])}`)
75+
// Use arg's own keys, not resolvedArg's: context.resolve() deep-resolves
76+
// object keys too, replacing a token placeholder with its resolved
77+
// reference text and dropping the `${...}` wrapper, so resolvedArg's
78+
// keys no longer look like tokens and no longer match `arg`'s.
79+
// context.resolve() also drops undefined-valued entries (mirroring
80+
// JSON.stringify), so filter those out here to match resolvedArg's keys.
81+
// A resolvable value (e.g. Lazy.anyValue()) only reveals `undefined`
82+
// once resolved, so resolve the value - not just check `arg[key]` - to
83+
// detect it.
84+
return `{${Object.keys(arg)
85+
.filter((key) => context.resolve(arg[key]) !== undefined)
86+
.map(
87+
(key) =>
88+
`${this.resolveObjectKey(context, key)} = ${this.resolveArg(context, arg[key])}`,
89+
)
9190
.join(", ")}}`;
9291
}
9392

@@ -120,6 +119,72 @@ class TFExpression extends Intrinsic implements IResolvable {
120119
return this.resolveExpressionPart(context, arg);
121120
}
122121

122+
// An object key built from a token (e.g. `{[res.id]: "v"}`) is a JS string
123+
// holding the token's placeholder by the time it reaches here, and reads
124+
// identically to a literal key unless we check for the placeholder.
125+
// HCL has no object-key interpolation syntax, so a token key can only be
126+
// expressed as a parenthesised key expression, e.g. `(resource.id) = "v"`.
127+
private resolveObjectKey(context: IResolveContext, key: string): string {
128+
const tokenList = Tokenization.reverseString(key);
129+
const hasToken = tokenList.tokens.length + tokenList.intrinsic.length > 0;
130+
131+
if (!hasToken) {
132+
return quotedStringKey(key);
133+
}
134+
135+
return `(${this.resolveTokenKey(context, key)})`;
136+
}
137+
138+
// A mixed literal/token key (e.g. `{[`prefix-${res.id}`]: "v"}`) must stay a
139+
// key expression, not go through resolveArg()'s resolveString(), which
140+
// escapes a literal fragment for use as a Terraform *value* (only `\n` and
141+
// `${`) rather than as a quoted-string *key* (which also needs quotes and
142+
// backslashes escaped).
143+
private resolveTokenKey(context: IResolveContext, key: string): string {
144+
const tokenList = Tokenization.reverseString(key);
145+
const numberOfTokens = tokenList.tokens.length + tokenList.intrinsic.length;
146+
147+
// A key that is only a token (no literal fragments) is a bare reference
148+
// expression, not a quoted string - e.g. `(resource.id) = "v"`.
149+
if (
150+
tokenList.literals.length === 0 &&
151+
tokenList.escapes.length === 0 &&
152+
numberOfTokens === 1
153+
) {
154+
return context.resolve(key);
155+
}
156+
157+
const joinResult = tokenList.join({
158+
join: (left, right) => {
159+
const leftTokens = Tokenization.reverse(left);
160+
const leftTokenList = Tokenization.reverseString(left);
161+
const rightTokens = Tokenization.reverse(right);
162+
163+
const leftTokenCount =
164+
leftTokenList.intrinsic.length + leftTokenList.tokens.length;
165+
166+
let leftValue = left;
167+
168+
if (leftTokenList.literals.length === 1 && leftTokenCount === 0) {
169+
leftValue = quotedStringKeyFragment(left);
170+
}
171+
172+
if (leftTokenList.literals.length === 0 && leftTokenCount === 1) {
173+
leftValue = `\${${leftTokens[0]}}`;
174+
}
175+
176+
const rightValue =
177+
rightTokens.length === 0
178+
? quotedStringKeyFragment(right)
179+
: `\${${rightTokens[0]}}`;
180+
181+
return `${leftValue}${rightValue}`;
182+
},
183+
});
184+
185+
return `"${joinResult}"`;
186+
}
187+
123188
/**
124189
* Escape string removes characters from the string that are not allowed in Terraform or JSON
125190
* It must only be used on non-token values

‎packages/cdktn/test/functions.test.ts‎

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
// Copyright (c) HashiCorp, Inc
22
// SPDX-License-Identifier: MPL-2.0
3-
import { Testing, TerraformStack, TerraformOutput, Fn } from "../src";
3+
import { Testing, TerraformStack, TerraformOutput, Fn, Lazy } from "../src";
44
import { TerraformVariable } from "../src/terraform-variable";
55
import { TerraformLocal } from "../src/terraform-local";
66
import { ref } from "../src/tfExpression";
@@ -465,6 +465,30 @@ test("undefined and null", () => {
465465
`);
466466
});
467467

468+
test("a Lazy value that resolves to undefined omits its key, like a literal undefined", () => {
469+
const app = Testing.app();
470+
const stack = new TerraformStack(app, "test");
471+
472+
new TerraformOutput(stack, "test-output", {
473+
value: Fn.merge([
474+
{
475+
key: Lazy.anyValue({ produce: () => undefined }),
476+
b: "x",
477+
},
478+
]),
479+
});
480+
481+
expect(Testing.synth(stack)).toMatchInlineSnapshot(`
482+
"{
483+
"output": {
484+
"test-output": {
485+
"value": "\${merge({\\"b\\" = \\"x\\"})}"
486+
}
487+
}
488+
}"
489+
`);
490+
});
491+
468492
describe("throws error on unescaped double quote string inputs", () => {
469493
const testsNotOk = ['"', '\\\\"', '\\\\\\\\"', '\\ \\\\"'];
470494

Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,125 @@
1+
// Copyright (c) HashiCorp, Inc
2+
// SPDX-License-Identifier: MPL-2.0
3+
import { Construct } from "constructs";
4+
import { Fn, Testing, TerraformLocal, TerraformStack, Token } from "../src";
5+
import { quotedStringKey } from "../src/hcl/quoted-string-key";
6+
import { renderMap } from "../src/hcl/render";
7+
import { call, Expression } from "../src/tfExpression";
8+
import { resolve } from "../src/_tokens";
9+
import { TestResource } from "./helper";
10+
11+
const stack = new TerraformStack(new Construct(undefined as any, "scope"), "s");
12+
const resolveExpression = (expr: Expression) => resolve(stack, expr);
13+
14+
describe("quotedStringKey", () => {
15+
it.each([
16+
["plain", `"plain"`],
17+
['he"llo', `"he\\"llo"`],
18+
["a\\b", `"a\\\\b"`],
19+
["${injected}", `"$\${injected}"`],
20+
["%{if x}", `"%%{if x}"`],
21+
["line\nbreak", `"line\\nbreak"`],
22+
["back\bspace", `"back\\u0008space"`],
23+
["form\ffeed", `"form\\u000cfeed"`],
24+
["\\b", `"\\\\b"`],
25+
])("serializes %j", (key, expected) => {
26+
expect(quotedStringKey(key)).toBe(expected);
27+
});
28+
});
29+
30+
describe("object keys in expressions", () => {
31+
it.each([
32+
['he"llo', `{"he\\"llo" = "x"}`],
33+
["${injected}", `{"$\${injected}" = "x"}`],
34+
["a\\b", `{"a\\\\b" = "x"}`],
35+
])("escapes the key %j of a direct object argument", (key, expected) => {
36+
expect(resolveExpression(call("lookup", [{ [key]: "x" }]))).toBe(
37+
`\${lookup(${expected})}`,
38+
);
39+
});
40+
41+
it("escapes the key of an object wrapped in a token", () => {
42+
expect(
43+
resolveExpression(call("lookup", [Token.asAny({ 'he"llo': "x" })])),
44+
).toBe(`\${lookup({"he\\"llo" = "x"})}`);
45+
});
46+
47+
it("renders a pure token key as a parenthesised key expression", () => {
48+
const res = new TestResource(stack, "object-keys-pure", {
49+
name: "test",
50+
});
51+
expect(resolveExpression(call("merge", [{ [res.stringValue]: "v" }]))).toBe(
52+
`\${merge({(test_resource.object-keys-pure.string_value) = "v"})}`,
53+
);
54+
});
55+
56+
it("renders a mixed literal/token key as a parenthesised interpolation", () => {
57+
const res = new TestResource(stack, "object-keys-mixed", {
58+
name: "test",
59+
});
60+
expect(
61+
resolveExpression(call("merge", [{ [`pre-${res.stringValue}`]: "w" }])),
62+
).toBe(
63+
`\${merge({("pre-\${test_resource.object-keys-mixed.string_value}") = "w"})}`,
64+
);
65+
});
66+
67+
it("escapes a quote in the literal fragment of a mixed key", () => {
68+
const res = new TestResource(stack, "object-keys-mixed-quote", {
69+
name: "test",
70+
});
71+
expect(
72+
resolveExpression(
73+
call("merge", [{ [`prefix"-${res.stringValue}`]: "w" }]),
74+
),
75+
).toBe(
76+
`\${merge({("prefix\\"-\${test_resource.object-keys-mixed-quote.string_value}") = "w"})}`,
77+
);
78+
});
79+
80+
it("escapes a backslash in the literal fragment of a mixed key", () => {
81+
const res = new TestResource(stack, "object-keys-mixed-backslash", {
82+
name: "test",
83+
});
84+
expect(
85+
resolveExpression(call("merge", [{ [`a\\b-${res.stringValue}`]: "w" }])),
86+
).toBe(
87+
`\${merge({("a\\\\b-\${test_resource.object-keys-mixed-backslash.string_value}") = "w"})}`,
88+
);
89+
});
90+
});
91+
92+
describe("map keys in the HCL renderer", () => {
93+
it("keeps bare identifiers unquoted", () => {
94+
expect(renderMap({ simple_key: "x" })).toBe(`{\nsimple_key = "x"\n}`);
95+
});
96+
97+
it.each([
98+
['he"llo', `"he\\"llo"`],
99+
["a\\b", `"a\\\\b"`],
100+
["with space", `"with space"`],
101+
])("quotes the key %j", (key, expected) => {
102+
expect(renderMap({ [key]: "x" })).toBe(`{\n${expected} = "x"\n}`);
103+
});
104+
105+
it("keeps a function-token key with a quoted string literal parseable", () => {
106+
const app = Testing.app();
107+
const localStack = new TerraformStack(app, "test");
108+
109+
new TerraformLocal(localStack, "m", { [Fn.lower("ABC")]: "v" });
110+
111+
const hcl = Testing.synthHcl(localStack);
112+
expect(hcl).toContain(`"\${lower("ABC")}" = "v"`);
113+
});
114+
115+
it("keeps a resource-reference key as an interpolation", () => {
116+
const app = Testing.app();
117+
const localStack = new TerraformStack(app, "test");
118+
const res = new TestResource(localStack, "res", { name: "test" });
119+
120+
new TerraformLocal(localStack, "m", { [res.stringValue]: "v" });
121+
122+
const hcl = Testing.synthHcl(localStack);
123+
expect(hcl).toContain(`"\${test_resource.res.string_value}" = "v"`);
124+
});
125+
});

0 commit comments

Comments
 (0)