Skip to content

Commit 442072e

Browse files
authored
chore: remove all possible AsString panics (#183)
Any call to `AsString` is unsafe. All places where the error is ignored is used in errors, so a placeholder or empty string is ok. A panic was observed by a customer. I am not sure where it occured. To be safe, I am using the function we have to protect these `AsString` calls. I added a custom linter to prevent these calls, this PR just removes all calls that were not fixed already.
1 parent 8f67d5f commit 442072e

File tree

6 files changed

+40
-82
lines changed

6 files changed

+40
-82
lines changed

extract/parameter.go

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -267,6 +267,7 @@ func optionalStringEnum[T ~string](block *terraform.Block, key string, def T, va
267267
func requiredString(block *terraform.Block, key string) (string, *hcl.Diagnostic) {
268268
tyAttr := block.GetAttribute(key)
269269
tyVal := tyAttr.Value()
270+
270271
if tyVal.Type() != cty.String {
271272
typeName := "<nil>"
272273
if !tyVal.Type().Equals(cty.NilType) {
@@ -282,7 +283,6 @@ func requiredString(block *terraform.Block, key string) (string, *hcl.Diagnostic
282283

283284
if tyAttr.IsNotNil() {
284285
diag.Subject = &(tyAttr.HCLAttribute().Range)
285-
// diag.Context = &(block.HCLBlock().DefRange)
286286
diag.Expression = tyAttr.HCLAttribute().Expr
287287
}
288288

@@ -297,8 +297,23 @@ func requiredString(block *terraform.Block, key string) (string, *hcl.Diagnostic
297297
return "", diag
298298
}
299299

300-
// nolint:gocritic // string type asserted
301-
return tyVal.AsString(), nil
300+
tyValStr, ok := hclext.AsString(tyVal)
301+
if !ok {
302+
// Either the val is unknown or null
303+
diag := &hcl.Diagnostic{
304+
Severity: hcl.DiagError,
305+
Summary: fmt.Sprintf("Invalid %q attribute for block %s", key, block.Label()),
306+
Detail: "Expected a string, got an unknown or null value",
307+
EvalContext: block.Context().Inner(),
308+
}
309+
310+
if tyAttr.IsNotNil() {
311+
diag.Subject = &(tyAttr.HCLAttribute().Range)
312+
diag.Expression = tyAttr.HCLAttribute().Expr
313+
}
314+
return "", diag
315+
}
316+
return tyValStr, nil
302317
}
303318

304319
func optionalBoolean(block *terraform.Block, key string) bool {

hclext/merge.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,12 @@ func MergeObjects(a, b cty.Value) cty.Value {
1111
output[key] = val
1212
}
1313
b.ForEachElement(func(key, val cty.Value) (stop bool) {
14-
// TODO: Should this error be captured?
15-
if key.Type() != cty.String {
16-
return true
17-
}
1814
//nolint:gocritic // string type asserted above
19-
k := key.AsString()
15+
k, ok := AsString(key)
16+
if !ok {
17+
// TODO: Should this error be captured?
18+
return stop
19+
}
2020
old := output[k]
2121
if old.IsKnown() && isNotEmptyObject(old) && isNotEmptyObject(val) {
2222
output[k] = MergeObjects(old, val)

hclext/references.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,12 @@ func CreateDotReferenceFromTraversal(traversals ...hcl.Traversal) string {
6868
switch {
6969
case part.Key.Type().Equals(cty.String):
7070
//nolint:gocritic // string type asserted above
71-
refParts = append(refParts, fmt.Sprintf("[%s]", part.Key.AsString()))
71+
stringName, ok := AsString(part.Key)
72+
if !ok {
73+
// Nothing we can do, just put a placeholder
74+
stringName = "??"
75+
}
76+
refParts = append(refParts, fmt.Sprintf("[%s]", stringName))
7277
case part.Key.Type().Equals(cty.Number):
7378
idx, _ := part.Key.AsBigFloat().Int64()
7479
refParts = append(refParts, fmt.Sprintf("[%d]", idx))

init.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ import (
55
"github.com/zclconf/go-cty/cty"
66
"github.com/zclconf/go-cty/cty/function"
77
"golang.org/x/xerrors"
8+
9+
"github.com/coder/preview/hclext"
810
)
911

1012
// init intends to override some of the default functions afforded by terraform.
@@ -27,7 +29,11 @@ func init() {
2729
Impl: func(args []cty.Value, retType cty.Type) (cty.Value, error) {
2830
// This code is taken directly from https://github.com/mitchellh/go-homedir/blob/af06845cf3004701891bf4fdb884bfe4920b3727/homedir.go#L58
2931
// The only change is that instead of expanding the path, we return an error
30-
path := args[0].AsString()
32+
path, ok := hclext.AsString(args[0])
33+
if !ok {
34+
return cty.NilVal, xerrors.Errorf("invalid path argument")
35+
}
36+
3137
if len(path) == 0 {
3238
return cty.StringVal(path), nil
3339
}

paramhook.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,10 @@ func parameterContextsEvalHook(input Input) func(ctx *tfcontext.Context, blocks
3939
continue
4040
}
4141

42-
//nolint:gocritic // string type asserted
43-
name := nameVal.AsString()
42+
name, ok := hclext.AsString(nameVal)
43+
if !ok {
44+
continue
45+
}
4446
var value cty.Value
4547
pv, ok := input.RichParameterValue(name)
4648
if ok {

types/primitive.go

Lines changed: 0 additions & 70 deletions
This file was deleted.

0 commit comments

Comments
 (0)