feat(template): cap the file size at 1 MiB and add BasicExpression.UsesContext
- commit
- a07f9c8e1cd4b4e6a4becfa136525989f53ca06e
- author
- brice · · 2026-10-12 16:14 UTC
- committer
- brice · · 2026-10-12 16:17 UTC
- parent
- a51d16b
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
5 files · +113 -1
M template/eval_test.go +34 -0
| @@ -210,3 +210,37 @@ func TestJSNumberString(t *testing.T) { | |||
| 210 | 210 | } | |
| 211 | 211 | } | |
| 212 | 212 | } | |
| 213 | + | ||
| 214 | + | func TestUsesContext(t *testing.T) { | |
| 215 | + | info := mustInfo(t, "step-env") | |
| 216 | + | tests := []struct { | |
| 217 | + | expr, ctx string | |
| 218 | + | want bool | |
| 219 | + | }{ | |
| 220 | + | {"matrix.os", "matrix", true}, | |
| 221 | + | {"MATRIX.os", "matrix", true}, | |
| 222 | + | {"github.ref == 'x' && matrix.os", "matrix", true}, | |
| 223 | + | {"format('{0}', fromJSON(matrix.a))", "matrix", true}, | |
| 224 | + | {"!(matrix.a)", "matrix", true}, | |
| 225 | + | {"github.event.matrix", "matrix", false}, | |
| 226 | + | {"github['matrix']", "matrix", false}, | |
| 227 | + | {"github[matrix.k]", "matrix", true}, | |
| 228 | + | {"'matrix.os'", "matrix", false}, | |
| 229 | + | {"contains(github.ref, 'matrix.')", "matrix", false}, | |
| 230 | + | {"matrix", "matrix", true}, | |
| 231 | + | {"env.matrix", "matrix", false}, | |
| 232 | + | } | |
| 233 | + | for _, tc := range tests { | |
| 234 | + | e := NewExpression(Origin{}, tc.expr, info) | |
| 235 | + | if e.Err() != nil { | |
| 236 | + | t.Errorf("%s: %v", tc.expr, e.Err()) | |
| 237 | + | continue | |
| 238 | + | } | |
| 239 | + | if got := e.UsesContext(tc.ctx); got != tc.want { | |
| 240 | + | t.Errorf("UsesContext(%q, %q) = %v, want %v", tc.expr, tc.ctx, got, tc.want) | |
| 241 | + | } | |
| 242 | + | } | |
| 243 | + | if (&BasicExpression{}).UsesContext("matrix") { | |
| 244 | + | t.Error("empty expression uses a context") | |
| 245 | + | } | |
| 246 | + | } | |
| @@ -210,3 +210,37 @@ func TestJSNumberString(t *testing.T) { | |||||
| 210 | } | 210 | } | ||
| 211 | } | 211 | } | ||
| 212 | } | 212 | } | ||
| 213 | + | ||||
| 214 | + | func TestUsesContext(t *testing.T) { | |||
| 215 | + | info := mustInfo(t, "step-env") | |||
| 216 | + | tests := []struct { | |||
| 217 | + | expr, ctx string | |||
| 218 | + | want bool | |||
| 219 | + | }{ | |||
| 220 | + | {"matrix.os", "matrix", true}, | |||
| 221 | + | {"MATRIX.os", "matrix", true}, | |||
| 222 | + | {"github.ref == 'x' && matrix.os", "matrix", true}, | |||
| 223 | + | {"format('{0}', fromJSON(matrix.a))", "matrix", true}, | |||
| 224 | + | {"!(matrix.a)", "matrix", true}, | |||
| 225 | + | {"github.event.matrix", "matrix", false}, | |||
| 226 | + | {"github['matrix']", "matrix", false}, | |||
| 227 | + | {"github[matrix.k]", "matrix", true}, | |||
| 228 | + | {"'matrix.os'", "matrix", false}, | |||
| 229 | + | {"contains(github.ref, 'matrix.')", "matrix", false}, | |||
| 230 | + | {"matrix", "matrix", true}, | |||
| 231 | + | {"env.matrix", "matrix", false}, | |||
| 232 | + | } | |||
| 233 | + | for _, tc := range tests { | |||
| 234 | + | e := NewExpression(Origin{}, tc.expr, info) | |||
| 235 | + | if e.Err() != nil { | |||
| 236 | + | t.Errorf("%s: %v", tc.expr, e.Err()) | |||
| 237 | + | continue | |||
| 238 | + | } | |||
| 239 | + | if got := e.UsesContext(tc.ctx); got != tc.want { | |||
| 240 | + | t.Errorf("UsesContext(%q, %q) = %v, want %v", tc.expr, tc.ctx, got, tc.want) | |||
| 241 | + | } | |||
| 242 | + | } | |||
| 243 | + | if (&BasicExpression{}).UsesContext("matrix") { | |||
| 244 | + | t.Error("empty expression uses a context") | |||
| 245 | + | } | |||
| 246 | + | } | |||
M template/read_test.go +30 -0
| @@ -129,3 +129,33 @@ func TestAliasBombIsRejected(t *testing.T) { | |||
| 129 | 129 | t.Errorf("errs = %q", errs) | |
| 130 | 130 | } | |
| 131 | 131 | } | |
| 132 | + | ||
| 133 | + | func TestFileSizeLimit(t *testing.T) { | |
| 134 | + | s, err := WorkflowSchema() | |
| 135 | + | if err != nil { | |
| 136 | + | t.Fatal(err) | |
| 137 | + | } | |
| 138 | + | big := []byte("on: push\n# " + strings.Repeat("x", MaxFileBytes) + "\n") | |
| 139 | + | ctx := NewContext(s) | |
| 140 | + | if tok := ReadYAML(ctx, "workflow-root-strict", "big.yml", big); tok != nil { | |
| 141 | + | t.Error("token returned for an oversized file") | |
| 142 | + | } | |
| 143 | + | errs := ctx.Messages() | |
| 144 | + | if len(errs) != 1 || !strings.HasPrefix(errs[0], "big.yml: The file is too large: ") { | |
| 145 | + | t.Errorf("errors = %q", errs) | |
| 146 | + | } | |
| 147 | + | // Exactly at the limit is read. | |
| 148 | + | ok := append([]byte("on: push\njobs: {}\n# "), make([]byte, 0)...) | |
| 149 | + | pad := MaxFileBytes - len(ok) - 1 | |
| 150 | + | ok = append(ok, []byte(strings.Repeat("x", pad)+"\n")...) | |
| 151 | + | if len(ok) != MaxFileBytes { | |
| 152 | + | t.Fatalf("test file is %d bytes", len(ok)) | |
| 153 | + | } | |
| 154 | + | ctx = NewContext(s) | |
| 155 | + | ReadYAML(ctx, "workflow-root-strict", "ok.yml", ok) | |
| 156 | + | for _, m := range ctx.Messages() { | |
| 157 | + | if strings.Contains(m, "too large") { | |
| 158 | + | t.Errorf("file at the limit refused: %s", m) | |
| 159 | + | } | |
| 160 | + | } | |
| 161 | + | } | |
| @@ -129,3 +129,33 @@ func TestAliasBombIsRejected(t *testing.T) { | |||||
| 129 | t.Errorf("errs = %q", errs) | 129 | t.Errorf("errs = %q", errs) | ||
| 130 | } | 130 | } | ||
| 131 | } | 131 | } | ||
| 132 | + | ||||
| 133 | + | func TestFileSizeLimit(t *testing.T) { | |||
| 134 | + | s, err := WorkflowSchema() | |||
| 135 | + | if err != nil { | |||
| 136 | + | t.Fatal(err) | |||
| 137 | + | } | |||
| 138 | + | big := []byte("on: push\n# " + strings.Repeat("x", MaxFileBytes) + "\n") | |||
| 139 | + | ctx := NewContext(s) | |||
| 140 | + | if tok := ReadYAML(ctx, "workflow-root-strict", "big.yml", big); tok != nil { | |||
| 141 | + | t.Error("token returned for an oversized file") | |||
| 142 | + | } | |||
| 143 | + | errs := ctx.Messages() | |||
| 144 | + | if len(errs) != 1 || !strings.HasPrefix(errs[0], "big.yml: The file is too large: ") { | |||
| 145 | + | t.Errorf("errors = %q", errs) | |||
| 146 | + | } | |||
| 147 | + | // Exactly at the limit is read. | |||
| 148 | + | ok := append([]byte("on: push\njobs: {}\n# "), make([]byte, 0)...) | |||
| 149 | + | pad := MaxFileBytes - len(ok) - 1 | |||
| 150 | + | ok = append(ok, []byte(strings.Repeat("x", pad)+"\n")...) | |||
| 151 | + | if len(ok) != MaxFileBytes { | |||
| 152 | + | t.Fatalf("test file is %d bytes", len(ok)) | |||
| 153 | + | } | |||
| 154 | + | ctx = NewContext(s) | |||
| 155 | + | ReadYAML(ctx, "workflow-root-strict", "ok.yml", ok) | |||
| 156 | + | for _, m := range ctx.Messages() { | |||
| 157 | + | if strings.Contains(m, "too large") { | |||
| 158 | + | t.Errorf("file at the limit refused: %s", m) | |||
| 159 | + | } | |||
| 160 | + | } | |||
| 161 | + | } | |||
M template/reader.go +8 -1
| @@ -14,8 +14,15 @@ import ( | |||
| 14 | 14 | // misplaced expressions, are recorded in ctx; the returned token holds what | |
| 15 | 15 | // could be read and is nil when the document is not valid YAML. | |
| 16 | 16 | // | |
| 17 | - | // An empty document is an error, as in the reference. | |
| 17 | + | // An empty document is an error, as in the reference. A document larger than | |
| 18 | + | // [MaxFileBytes] is refused before it is parsed, with one error about the | |
| 19 | + | // file. | |
| 18 | 20 | func ReadYAML(ctx *Context, rootType, file string, content []byte) (root Token) { | |
| 21 | + | if len(content) > MaxFileBytes { | |
| 22 | + | e := &Error{File: file, fileOnly: true, Raw: fmt.Sprintf("The file is too large: %d bytes, the limit is %d bytes", len(content), MaxFileBytes)} | |
| 23 | + | ctx.add(e) | |
| 24 | + | return nil | |
| 25 | + | } | |
| 19 | 26 | res := readYAMLDocument(file, content) | |
| 20 | 27 | for _, e := range res.errs { | |
| 21 | 28 | ctx.add(e) | |
| @@ -14,8 +14,15 @@ import ( | |||||
| 14 | // misplaced expressions, are recorded in ctx; the returned token holds what | 14 | // misplaced expressions, are recorded in ctx; the returned token holds what | ||
| 15 | // could be read and is nil when the document is not valid YAML. | 15 | // could be read and is nil when the document is not valid YAML. | ||
| 16 | // | 16 | // | ||
| 17 | - | // An empty document is an error, as in the reference. | 17 | + | // An empty document is an error, as in the reference. A document larger than |
| 18 | + | // [MaxFileBytes] is refused before it is parsed, with one error about the | |||
| 19 | + | // file. | |||
| 18 | func ReadYAML(ctx *Context, rootType, file string, content []byte) (root Token) { | 20 | func ReadYAML(ctx *Context, rootType, file string, content []byte) (root Token) { | ||
| 21 | + | if len(content) > MaxFileBytes { | |||
| 22 | + | e := &Error{File: file, fileOnly: true, Raw: fmt.Sprintf("The file is too large: %d bytes, the limit is %d bytes", len(content), MaxFileBytes)} | |||
| 23 | + | ctx.add(e) | |||
| 24 | + | return nil | |||
| 25 | + | } | |||
| 19 | res := readYAMLDocument(file, content) | 26 | res := readYAMLDocument(file, content) | ||
| 20 | for _, e := range res.errs { | 27 | for _, e := range res.errs { | ||
| 21 | ctx.add(e) | 28 | ctx.add(e) | ||
M template/tokens.go +33 -0
| @@ -288,3 +288,36 @@ func trimDisplay(s string) string { | |||
| 288 | 288 | // is empty. Callers that only inspect the tree (to look for function calls, | |
| 289 | 289 | // for instance) use it with the types of package expr. | |
| 290 | 290 | func (t *BasicExpression) Node() expr.Node { return t.node } | |
| 291 | + | ||
| 292 | + | // UsesContext reports whether the expression reads the named context | |
| 293 | + | // ("matrix", "secrets"...), anywhere in its syntax tree and whatever the | |
| 294 | + | // case. A function call is not a use of a context named like the function. | |
| 295 | + | func (t *BasicExpression) UsesContext(name string) bool { return nodeUsesContext(t.node, name) } | |
| 296 | + | ||
| 297 | + | func nodeUsesContext(n expr.Node, name string) bool { | |
| 298 | + | switch n := n.(type) { | |
| 299 | + | case *expr.ContextAccess: | |
| 300 | + | return strings.EqualFold(n.Name.Lexeme, name) | |
| 301 | + | case *expr.Unary: | |
| 302 | + | return nodeUsesContext(n.Expr, name) | |
| 303 | + | case *expr.Binary: | |
| 304 | + | return nodeUsesContext(n.Left, name) || nodeUsesContext(n.Right, name) | |
| 305 | + | case *expr.Grouping: | |
| 306 | + | return nodeUsesContext(n.Group, name) | |
| 307 | + | case *expr.IndexAccess: | |
| 308 | + | return nodeUsesContext(n.Expr, name) || nodeUsesContext(n.Index, name) | |
| 309 | + | case *expr.Logical: | |
| 310 | + | for _, a := range n.Args { | |
| 311 | + | if nodeUsesContext(a, name) { | |
| 312 | + | return true | |
| 313 | + | } | |
| 314 | + | } | |
| 315 | + | case *expr.FunctionCall: | |
| 316 | + | for _, a := range n.Args { | |
| 317 | + | if nodeUsesContext(a, name) { | |
| 318 | + | return true | |
| 319 | + | } | |
| 320 | + | } | |
| 321 | + | } | |
| 322 | + | return false | |
| 323 | + | } | |
| @@ -288,3 +288,36 @@ func trimDisplay(s string) string { | |||||
| 288 | // is empty. Callers that only inspect the tree (to look for function calls, | 288 | // is empty. Callers that only inspect the tree (to look for function calls, | ||
| 289 | // for instance) use it with the types of package expr. | 289 | // for instance) use it with the types of package expr. | ||
| 290 | func (t *BasicExpression) Node() expr.Node { return t.node } | 290 | func (t *BasicExpression) Node() expr.Node { return t.node } | ||
| 291 | + | ||||
| 292 | + | // UsesContext reports whether the expression reads the named context | |||
| 293 | + | // ("matrix", "secrets"...), anywhere in its syntax tree and whatever the | |||
| 294 | + | // case. A function call is not a use of a context named like the function. | |||
| 295 | + | func (t *BasicExpression) UsesContext(name string) bool { return nodeUsesContext(t.node, name) } | |||
| 296 | + | ||||
| 297 | + | func nodeUsesContext(n expr.Node, name string) bool { | |||
| 298 | + | switch n := n.(type) { | |||
| 299 | + | case *expr.ContextAccess: | |||
| 300 | + | return strings.EqualFold(n.Name.Lexeme, name) | |||
| 301 | + | case *expr.Unary: | |||
| 302 | + | return nodeUsesContext(n.Expr, name) | |||
| 303 | + | case *expr.Binary: | |||
| 304 | + | return nodeUsesContext(n.Left, name) || nodeUsesContext(n.Right, name) | |||
| 305 | + | case *expr.Grouping: | |||
| 306 | + | return nodeUsesContext(n.Group, name) | |||
| 307 | + | case *expr.IndexAccess: | |||
| 308 | + | return nodeUsesContext(n.Expr, name) || nodeUsesContext(n.Index, name) | |||
| 309 | + | case *expr.Logical: | |||
| 310 | + | for _, a := range n.Args { | |||
| 311 | + | if nodeUsesContext(a, name) { | |||
| 312 | + | return true | |||
| 313 | + | } | |||
| 314 | + | } | |||
| 315 | + | case *expr.FunctionCall: | |||
| 316 | + | for _, a := range n.Args { | |||
| 317 | + | if nodeUsesContext(a, name) { | |||
| 318 | + | return true | |||
| 319 | + | } | |||
| 320 | + | } | |||
| 321 | + | } | |||
| 322 | + | return false | |||
| 323 | + | } | |||
M template/yaml.go +8 -0
| @@ -15,6 +15,14 @@ import ( | |||
| 15 | 15 | "go.yaml.in/yaml/v3" | |
| 16 | 16 | ) | |
| 17 | 17 | ||
| 18 | + | // MaxFileBytes is the largest document [ReadYAML] accepts: 1 MiB. The check | |
| 19 | + | // comes before any parsing, so that a hostile file costs nothing more than | |
| 20 | + | // its length; a file this size can still expand through aliases (see | |
| 21 | + | // [MaxNodes]). | |
| 22 | + | // | |
| 23 | + | // TODO(verify with recording): GitHub's own limit and message. | |
| 24 | + | const MaxFileBytes = 1 << 20 | |
| 25 | + | ||
| 18 | 26 | // MaxNodes is the number of YAML nodes a document may expand to, counting | |
| 19 | 27 | // every use of an alias. The reference expands aliases without a limit, so a | |
| 20 | 28 | // small document of nested aliases can allocate gigabytes. | |
| @@ -15,6 +15,14 @@ import ( | |||||
| 15 | "go.yaml.in/yaml/v3" | 15 | "go.yaml.in/yaml/v3" | ||
| 16 | ) | 16 | ) | ||
| 17 | 17 | ||||
| 18 | + | // MaxFileBytes is the largest document [ReadYAML] accepts: 1 MiB. The check | |||
| 19 | + | // comes before any parsing, so that a hostile file costs nothing more than | |||
| 20 | + | // its length; a file this size can still expand through aliases (see | |||
| 21 | + | // [MaxNodes]). | |||
| 22 | + | // | |||
| 23 | + | // TODO(verify with recording): GitHub's own limit and message. | |||
| 24 | + | const MaxFileBytes = 1 << 20 | |||
| 25 | + | ||||
| 18 | // MaxNodes is the number of YAML nodes a document may expand to, counting | 26 | // MaxNodes is the number of YAML nodes a document may expand to, counting | ||
| 19 | // every use of an alias. The reference expands aliases without a limit, so a | 27 | // every use of an alias. The reference expands aliases without a limit, so a | ||
| 20 | // small document of nested aliases can allocate gigabytes. | 28 | // small document of nested aliases can allocate gigabytes. | ||