Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fromJSON: Mark nested structures as secret if the JSON string is secret #191

Merged
merged 6 commits into from
Dec 5, 2023
Merged

fromJSON: Mark nested structures as secret if the JSON string is secret #191

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
67a9f70
Add test
komalali Dec 5, 2023
0d13dbc
Keep JSON objects marked as secret
komalali Dec 5, 2023
f659cf8
Add array test case
komalali Dec 5, 2023
8570063
Update changelog
komalali Dec 5, 2023
f93bca4
Add test to demonstrate bug in nested values
komalali Dec 5, 2023
d5bd158
Plumb secretness through FromJSON
komalali Dec 5, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG_PENDING.md
View file
Open in desktop
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)
3 changes: 2 additions & 1 deletion eval/eval.go
View file
Open in desktop
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 @@ -925,6 +925,7 @@ func (e *evalContext) evaluateBuiltinFromJSON(x *expr, repr *fromJSONExpr) *valu
v.unknown = true
return v
}
ev.Secret = v.secret
komalali marked this conversation as resolved.
Show resolved Hide resolved

return unexport(ev, x)
}
Expand Down
42 changes: 24 additions & 18 deletions eval/eval_test.go
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
Loading