Skip to content

Commit 8868d03

Browse files
jeremyruppeljeremyruppel
committed
fix(coderd/templatebuilder): auto-quote string variable values for HCL
The backend now accepts raw string values from callers (e.g. "anthropic") and wraps them in HCL quotes automatically via hclQuote(). Previously callers were required to send pre-quoted HCL literals, which is not a reasonable API contract for frontend consumers. - validateStringValue now validates raw strings (rejects interpolation and overlong values) - toHCLLiteral wraps string values in quotes with proper escaping - hclQuote handles backslash, quote, newline, and carriage return escaping - null is passed through unquoted for all types
1 parent 649117b commit 8868d03

3 files changed

Lines changed: 64 additions & 63 deletions

File tree

‎coderd/templatebuilder/compose.go‎

Lines changed: 46 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -245,15 +245,17 @@ func mergeModuleVariables(manifest ModuleManifest, callerVars map[string]string)
245245
// missingkey=error surfaces the omission at render time.
246246
}
247247

248-
// Overlay validated caller values.
248+
// Overlay validated caller values, converting to HCL literals.
249249
for k, val := range callerVars {
250-
merged[k] = val
250+
merged[k] = toHCLLiteral(allowedVars[k], val)
251251
}
252252
return merged, nil
253253
}
254254

255-
// validateVariableValue checks that value is a valid HCL literal for the
256-
// variable's declared type. The literal "null" is accepted for any type.
255+
// validateVariableValue checks that the caller-supplied value is valid for
256+
// the variable's declared type. String values are plain text (not
257+
// pre-quoted); quoting for HCL happens later in toHCLLiteral.
258+
// The literal "null" is accepted for any type.
257259
func validateVariableValue(v ModuleVariable, value string) error {
258260
if value == "null" {
259261
return nil
@@ -270,45 +272,55 @@ func validateVariableValue(v ModuleVariable, value string) error {
270272
}
271273
}
272274

273-
// validateStringValue checks that value is a valid quoted HCL string literal.
274-
// It must start and end with '"', contain no unescaped newlines or quotes,
275-
// and must not contain HCL interpolation/directive markers.
275+
// toHCLLiteral converts a validated caller value into an HCL literal.
276+
// The literal "null" is passed through for any type. Strings are wrapped
277+
// in quotes with interior characters escaped; bools and numbers are
278+
// already valid HCL literals.
279+
func toHCLLiteral(v ModuleVariable, value string) string {
280+
if value == "null" {
281+
return value
282+
}
283+
if v.Type == "string" {
284+
return hclQuote(value)
285+
}
286+
return value
287+
}
288+
289+
// validateStringValue checks that a raw (unquoted) string value is safe
290+
// to embed in an HCL quoted string. It rejects HCL interpolation/directive
291+
// markers and values that exceed the maximum length.
276292
func validateStringValue(value string) error {
277293
if len(value) > maxStringValueLen {
278294
return xerrors.Errorf("value exceeds maximum length of %d bytes", maxStringValueLen)
279295
}
280-
if len(value) < 2 || value[0] != '"' || value[len(value)-1] != '"' {
281-
return xerrors.New("must be a quoted string (e.g. \"value\")")
282-
}
283-
284-
inner := value[1 : len(value)-1]
285-
286-
if strings.Contains(inner, "${") || strings.Contains(inner, "%{") {
296+
if strings.Contains(value, "${") || strings.Contains(value, "%{") {
287297
return xerrors.New("must not contain HCL interpolation or directive sequences")
288298
}
299+
return nil
300+
}
289301

290-
// Walk the inner content to reject unescaped newlines and quotes.
291-
for i := 0; i < len(inner); i++ {
292-
ch := inner[i]
293-
if ch == '\\' {
294-
i++
295-
if i >= len(inner) {
296-
// Trailing backslash with no character to escape.
297-
// In HCL this would escape the closing quote delimiter,
298-
// producing an unterminated string.
299-
return xerrors.New("must not end with a trailing backslash")
300-
}
301-
continue
302-
}
303-
if ch == '"' {
304-
return xerrors.New("must not contain unescaped quotes")
305-
}
306-
if ch == '\n' || ch == '\r' {
307-
return xerrors.New("must not contain unescaped newlines")
302+
// hclQuote wraps a raw string in HCL double-quotes, escaping backslashes,
303+
// double-quotes, and newlines so the result is a valid HCL string literal.
304+
func hclQuote(s string) string {
305+
var b strings.Builder
306+
b.Grow(len(s) + 2)
307+
_, _ = b.WriteRune('"')
308+
for i := 0; i < len(s); i++ {
309+
switch s[i] {
310+
case '\\':
311+
_, _ = b.WriteString("\\\\")
312+
case '"':
313+
_, _ = b.WriteString("\\\"")
314+
case '\n':
315+
_, _ = b.WriteString("\\n")
316+
case '\r':
317+
_, _ = b.WriteString("\\r")
318+
default:
319+
_ = b.WriteByte(s[i])
308320
}
309321
}
310-
311-
return nil
322+
_, _ = b.WriteRune('"')
323+
return b.String()
312324
}
313325

314326
// validateNumberValue checks that value is a valid HCL number literal.

‎coderd/templatebuilder/compose_internal_test.go‎

Lines changed: 15 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ func TestMergeModuleVariables(t *testing.T) {
9797
t.Run("CallerProvidesRequired", func(t *testing.T) {
9898
t.Parallel()
9999
merged, err := mergeModuleVariables(manifest, map[string]string{
100-
"required_no_default": `"value"`,
100+
"required_no_default": "value",
101101
})
102102
require.NoError(t, err)
103103
require.Equal(t, `"value"`, merged["required_no_default"])
@@ -153,11 +153,11 @@ func TestMergeModuleVariables(t *testing.T) {
153153
t.Run("InvalidStringValueRejected", func(t *testing.T) {
154154
t.Parallel()
155155
_, err := mergeModuleVariables(manifest, map[string]string{
156-
"optional_no_default": "unquoted",
156+
"optional_no_default": "${var.foo}",
157157
})
158158
require.Error(t, err)
159159
require.Contains(t, err.Error(), `variable "optional_no_default"`)
160-
require.Contains(t, err.Error(), "quoted string")
160+
require.Contains(t, err.Error(), "interpolation")
161161
})
162162

163163
t.Run("NullAcceptedForAnyType", func(t *testing.T) {
@@ -189,28 +189,17 @@ func TestValidateStringValue(t *testing.T) {
189189
value string
190190
wantErr string
191191
}{
192-
{"ValidEmpty", `""`, ""},
193-
{"ValidSimple", `"hello"`, ""},
194-
{"ValidPath", `"/home/coder"`, ""},
195-
{"ValidURL", `"https://github.com/coder/coder"`, ""},
196-
{"ValidLiteralBackslashN", `"line\\nbreak"`, ""},
197-
{"ValidEscapedQuote", `"say \"hi\""`, ""},
198-
{"ValidEscapedBackslash", `"path\\to\\file"`, ""},
199-
200-
{"RejectedUnquoted", "hello", "quoted string"},
201-
{"RejectedMissingOpenQuote", `hello"`, "quoted string"},
202-
{"RejectedMissingCloseQuote", `"hello`, "quoted string"},
203-
{"RejectedEmpty", "", "quoted string"},
204-
{"RejectedSingleChar", `"`, "quoted string"},
205-
{"RejectedUnescapedNewline", "\"line\nbreak\"", "unescaped newlines"},
206-
{"RejectedCarriageReturn", "\"line\rbreak\"", "unescaped newlines"},
207-
{"RejectedUnescapedQuote", `"say "hi""`, "unescaped quotes"},
208-
{"RejectedHCLInterpolation", `"${var.foo}"`, "interpolation"},
209-
{"RejectedHCLDirective", `"%{if true}yes%{endif}"`, "interpolation"},
210-
{"RejectedOverlong", `"` + strings.Repeat("a", maxStringValueLen) + `"`, "maximum length"},
211-
{"RejectedTrailingBackslash", `"test\"`, "trailing backslash"},
212-
{"RejectedTrailingBackslashOnly", `"\"`, "trailing backslash"},
213-
{"ValidEvenTrailingBackslashes", `"test\\\\"`, ""},
192+
{"ValidEmpty", "", ""},
193+
{"ValidSimple", "hello", ""},
194+
{"ValidPath", "/home/coder", ""},
195+
{"ValidURL", "https://github.com/coder/coder", ""},
196+
{"ValidWithQuotes", `say "hi"`, ""},
197+
{"ValidWithNewlines", "line\nbreak", ""},
198+
{"ValidWithBackslash", `path\to\file`, ""},
199+
200+
{"RejectedHCLInterpolation", "${var.foo}", "interpolation"},
201+
{"RejectedHCLDirective", "%{if true}yes%{endif}", "interpolation"},
202+
{"RejectedOverlong", strings.Repeat("a", maxStringValueLen+1), "maximum length"},
214203
}
215204

216205
for _, tc := range tests {
@@ -313,7 +302,7 @@ func TestValidateVariableValue(t *testing.T) {
313302
t.Run("UnsupportedTypeRejected", func(t *testing.T) {
314303
t.Parallel()
315304
v := ModuleVariable{Name: "test", Type: "list"}
316-
err := validateVariableValue(v, `"val"`)
305+
err := validateVariableValue(v, "val")
317306
require.Error(t, err)
318307
require.Contains(t, err.Error(), "unsupported variable type")
319308
})

‎coderd/templatebuilder/compose_test.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ func TestCompose(t *testing.T) {
9393
{
9494
ID: "git-clone",
9595
Variables: map[string]string{
96-
"url": `"https://github.com/coder/coder"`,
96+
"url": "https://github.com/coder/coder",
9797
},
9898
},
9999
},
@@ -178,7 +178,7 @@ func TestCompose(t *testing.T) {
178178
{
179179
ID: "code-server",
180180
Variables: map[string]string{
181-
"nonexistent_var": `"value"`,
181+
"nonexistent_var": "value",
182182
},
183183
},
184184
},
@@ -216,7 +216,7 @@ func TestCompose(t *testing.T) {
216216
{
217217
ID: "code-server",
218218
Variables: map[string]string{
219-
"folder": `"${var.evil}"`,
219+
"folder": "${var.evil}",
220220
},
221221
},
222222
},

0 commit comments

Comments
 (0)