Skip to content

Commit

Permalink
Mark nested structures as secret if fromJSON string is secret (#191)
Browse files Browse the repository at this point in the history
  • Loading branch information
komalali authored Dec 5, 2023
1 parent 44b746a commit a18e45e
Show file tree
Hide file tree
Showing 30 changed files with 2,076 additions and 49 deletions.
1 change: 1 addition & 0 deletions CHANGELOG_PENDING.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,4 @@
### Bug Fixes

* Fix a nil pointer dereference in Syntax.NodeError. [#180](https://github.com/pulumi/esc/pull/180)
* Mark nested structures as secret if the JSON string is secret. [#191](https://github.com/pulumi/esc/pull/191)
4 changes: 2 additions & 2 deletions eval/eval.go
Original file line number Diff line number Diff line change
Expand Up @@ -342,7 +342,7 @@ func (e *evalContext) evaluate() (*value, syntax.Diagnostics) {
return v, e.diags
}

// evaluateImports evalutes an environment's imports.
// evaluateImports evaluates an environment's imports.
func (e *evalContext) evaluateImports() {
mine := &imported{evaluating: true}
defer func() { mine.evaluating = false }()
Expand Down Expand Up @@ -919,7 +919,7 @@ func (e *evalContext) evaluateBuiltinFromJSON(x *expr, repr *fromJSONExpr) *valu
return v
}

ev, err := esc.FromJSON(jv)
ev, err := esc.FromJSON(jv, v.secret)
if err != nil {
e.errorf(repr.syntax(), "internal error: decoding JSON value: %v", err)
v.unknown = true
Expand Down
42 changes: 24 additions & 18 deletions eval/eval_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -181,13 +181,14 @@ func normalize[T any](t *testing.T, v T) T {

func TestEval(t *testing.T) {
type expectedData struct {
LoadDiags syntax.Diagnostics `json:"loadDiags,omitempty"`
CheckDiags syntax.Diagnostics `json:"checkDiags,omitempty"`
Check *esc.Environment `json:"check,omitempty"`
CheckJSON any `json:"checkJson,omitempty"`
EvalDiags syntax.Diagnostics `json:"evalDiags,omitempty"`
Eval *esc.Environment `json:"eval,omitempty"`
EvalJSON any `json:"evalJson,omitempty"`
LoadDiags syntax.Diagnostics `json:"loadDiags,omitempty"`
CheckDiags syntax.Diagnostics `json:"checkDiags,omitempty"`
Check *esc.Environment `json:"check,omitempty"`
CheckJSON any `json:"checkJson,omitempty"`
EvalDiags syntax.Diagnostics `json:"evalDiags,omitempty"`
Eval *esc.Environment `json:"eval,omitempty"`
EvalJSONRedacted any `json:"evalJsonRedacted,omitempty"`
EvalJSONRevealed any `json:"evalJSONRevealed,omitempty"`
}

path := filepath.Join("testdata", "eval")
Expand All @@ -213,24 +214,27 @@ func TestEval(t *testing.T) {
sortEnvironmentDiagnostics(evalDiags)

var checkJSON any
var evalJSON any
var evalJSONRedacted any
var evalJSONRevealed any
if check != nil {
check = normalize(t, check)
checkJSON = esc.NewValue(check.Properties).ToJSON(true)
}
if actual != nil {
actual = normalize(t, actual)
evalJSON = esc.NewValue(actual.Properties).ToJSON(true)
evalJSONRedacted = esc.NewValue(actual.Properties).ToJSON(true)
evalJSONRevealed = esc.NewValue(actual.Properties).ToJSON(false)
}

bytes, err := json.MarshalIndent(expectedData{
LoadDiags: loadDiags,
CheckDiags: checkDiags,
EvalDiags: evalDiags,
Check: check,
Eval: actual,
EvalJSON: evalJSON,
CheckJSON: checkJSON,
LoadDiags: loadDiags,
CheckDiags: checkDiags,
EvalDiags: evalDiags,
Check: check,
Eval: actual,
EvalJSONRedacted: evalJSONRedacted,
EvalJSONRevealed: evalJSONRevealed,
CheckJSON: checkJSON,
}, "", " ")
bytes = append(bytes, '\n')
require.NoError(t, err)
Expand Down Expand Up @@ -268,8 +272,10 @@ func TestEval(t *testing.T) {

// work around a comparison issue when comparing nil slices/maps against zero-length slices/maps
if actual != nil {
evalJSON := esc.NewValue(actual.Properties).ToJSON(true)
assert.Equal(t, expected.EvalJSON, evalJSON)
evalJSONRedacted := esc.NewValue(actual.Properties).ToJSON(true)
assert.Equal(t, expected.EvalJSONRedacted, evalJSONRedacted)
evalJSONRevealed := esc.NewValue(actual.Properties).ToJSON(false)
assert.Equal(t, expected.EvalJSONRevealed, evalJSONRevealed)
}

if check != nil {
Expand Down
14 changes: 13 additions & 1 deletion eval/testdata/eval/builtin-combine/expected.json
Original file line number Diff line number Diff line change
Expand Up @@ -1852,7 +1852,7 @@
]
}
},
"evalJson": {
"evalJsonRedacted": {
"builtins": [
"[secret]",
"[secret]",
Expand All @@ -1863,5 +1863,17 @@
"foo": "bar"
},
"password": "[secret]"
},
"evalJSONRevealed": {
"builtins": [
"hunter2,bar",
"aHVudGVyMiBiYXI=",
"\"hunter2 bar\"",
"hunter2 bar"
],
"open": {
"foo": "bar"
},
"password": "hunter2"
}
}
14 changes: 13 additions & 1 deletion eval/testdata/eval/builtin-errs/expected.json
Original file line number Diff line number Diff line change
Expand Up @@ -2115,7 +2115,19 @@
]
}
},
"evalJson": {
"evalJsonRedacted": {
"builtins": [
"[unknown]",
"[unknown]",
"[unknown]",
"[unknown]",
"[unknown]",
"[unknown]",
"[unknown]"
],
"number": 42
},
"evalJSONRevealed": {
"builtins": [
"[unknown]",
"[unknown]",
Expand Down
5 changes: 4 additions & 1 deletion eval/testdata/eval/ciphertext-invalid/expected.json
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,10 @@
]
}
},
"evalJson": {
"evalJsonRedacted": {
"password": "[secret]"
},
"evalJSONRevealed": {
"password": "[unknown]"
}
}
5 changes: 4 additions & 1 deletion eval/testdata/eval/ciphertext/expected.json
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,10 @@
]
}
},
"evalJson": {
"evalJsonRedacted": {
"password": "[secret]"
},
"evalJSONRevealed": {
"password": "hunter2"
}
}
13 changes: 12 additions & 1 deletion eval/testdata/eval/cycle/expected.json
Original file line number Diff line number Diff line change
Expand Up @@ -960,7 +960,18 @@
]
}
},
"evalJson": {
"evalJsonRedacted": {
"a": {
"p": "[unknown]"
},
"b": {
"p": "[unknown]"
},
"c": {
"p": "[unknown]"
}
},
"evalJSONRevealed": {
"a": {
"p": "[unknown]"
},
Expand Down
8 changes: 7 additions & 1 deletion eval/testdata/eval/duplicate-keys/expected.json
Original file line number Diff line number Diff line change
Expand Up @@ -441,7 +441,13 @@
]
}
},
"evalJson": {
"evalJsonRedacted": {
"foo": "bar",
"qux": {
"foo": "bar"
}
},
"evalJSONRevealed": {
"foo": "bar",
"qux": {
"foo": "bar"
Expand Down
9 changes: 8 additions & 1 deletion eval/testdata/eval/escape-keys/expected.json
Original file line number Diff line number Diff line change
Expand Up @@ -504,7 +504,14 @@
]
}
},
"evalJson": {
"evalJsonRedacted": {
"object": {
"\"quoted\"": "baz",
"0leadingDigit": "bar",
"with-hyphen": "foo"
}
},
"evalJSONRevealed": {
"object": {
"\"quoted\"": "baz",
"0leadingDigit": "bar",
Expand Down
3 changes: 2 additions & 1 deletion eval/testdata/eval/import-cycle-2/expected.json
Original file line number Diff line number Diff line change
Expand Up @@ -60,5 +60,6 @@
"type": "object"
}
},
"evalJson": {}
"evalJsonRedacted": {},
"evalJSONRevealed": {}
}
3 changes: 2 additions & 1 deletion eval/testdata/eval/import-cycle/expected.json
Original file line number Diff line number Diff line change
Expand Up @@ -60,5 +60,6 @@
"type": "object"
}
},
"evalJson": {}
"evalJsonRedacted": {},
"evalJSONRevealed": {}
}
19 changes: 18 additions & 1 deletion eval/testdata/eval/imports/expected.json
Original file line number Diff line number Diff line change
Expand Up @@ -2210,7 +2210,24 @@
]
}
},
"evalJson": {
"evalJsonRedacted": {
"some_list": [
"hello",
"world"
],
"some_number": 42,
"some_object": {
"alpha": null,
"baz": "qux",
"beta": "bar",
"foo": "bar",
"goodbye": "for now",
"hello": "world",
"zed": "bar"
},
"some_string": "hello"
},
"evalJSONRevealed": {
"some_list": [
"hello",
"world"
Expand Down
31 changes: 30 additions & 1 deletion eval/testdata/eval/interp/expected.json
Original file line number Diff line number Diff line change
Expand Up @@ -2898,7 +2898,36 @@
]
}
},
"evalJson": {
"evalJsonRedacted": {
"34": "this should be okay",
"35": {
"okay": "also okay"
},
"a": {
"p": "foo",
"q": "foo",
"u": {
"\"baz": "merp"
}
},
"b": {
"p": "foo"
},
"c": {
"a": [
"foo",
"hello, world!",
"foo bar"
],
"b": "world!",
"s": [
"merp",
"this should be okay",
"also okay"
]
}
},
"evalJSONRevealed": {
"34": "this should be okay",
"35": {
"okay": "also okay"
Expand Down
71 changes: 70 additions & 1 deletion eval/testdata/eval/invalid-access/expected.json
Original file line number Diff line number Diff line change
Expand Up @@ -8122,7 +8122,76 @@
]
}
},
"evalJson": {
"evalJsonRedacted": {
"array": [
1,
2,
3
],
"errors": [
"[unknown]",
"[unknown]",
"[unknown]",
"[unknown]",
"[unknown]",
"[unknown]",
"[unknown]",
"[unknown]",
"[unknown]",
"[unknown]",
"[unknown]",
"[unknown]",
"[unknown]"
],
"myObject": {
"foo": "bar"
},
"object": {
"baz": "qux",
"hello": "world"
},
"open": {
"anyOf": "hello",
"array": [
2,
"items",
{
"some": "object"
},
[
"array"
]
],
"boolean": true,
"false": false,
"hello": "hello",
"map": {
"blue": 42,
"hello": "world"
},
"null": null,
"number": 42,
"oneOf": 42,
"pi": 3.14,
"record": {
"foo": "bar"
},
"ref": {
"baz": "qux"
},
"string": "esc",
"true": true,
"tuple": [
"hello",
"world"
]
},
"otherObject": {
"foo": "bar"
},
"string": "esc"
},
"evalJSONRevealed": {
"array": [
1,
2,
Expand Down
16 changes: 15 additions & 1 deletion eval/testdata/eval/literals/expected.json
Original file line number Diff line number Diff line change
Expand Up @@ -1007,7 +1007,21 @@
]
}
},
"evalJson": {
"evalJsonRedacted": {
"some_bool": true,
"some_list": [
"hello",
"world"
],
"some_null": null,
"some_number": 42,
"some_object": {
"goodbye": "for now",
"hello": "world"
},
"some_string": "hello"
},
"evalJSONRevealed": {
"some_bool": true,
"some_list": [
"hello",
Expand Down
Loading

0 comments on commit a18e45e

Please sign in to comment.