Skip to content

Commit ba9937b

Browse files
committed
Add depth limits
1 parent 7dce9f5 commit ba9937b

14 files changed

Lines changed: 209 additions & 17 deletions

CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1717
existing `?` unwrap for a die-loud form, e.g.
1818
`"pkgs.json" parseJson tryAs Manifest ? :packages? (:name?) map`.
1919
A `tryAs` whose source type could never be the target is a type error.
20+
Validation depth is limited to 1024 nested levels; exceeding it (a cyclic
21+
value, a cyclic `type` declaration, or absurdly deep data) fails the script
22+
with a clear error rather than producing `none`.
2023

2124
- CLI completions for `cargo`: subcommands (including installed third-party
2225
ones via `cargo --list`), per-subcommand options, and dynamic values for

ai/tryas_review.md

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
# tryAs review — outstanding items
2+
3+
Critical review of the `try-as` branch (`7dce9f5 Implement tryAs`), 2026-07-09.
4+
The unbounded-recursion crashes found in this review are already **fixed** on the
5+
branch (depth budget of 1024 in `validateObj`, see `maxTryAsValidateDepth` in
6+
`mshell/TypeExpr.go`, with regression tests in `tests/fail/tryas_cyclic_type.msh`,
7+
`tests/fail/tryas_cyclic_value.msh`, `tests/fail/tryas_depth_exceeded.msh`, and
8+
`tests/success/tryas_depth.msh`). Everything below is still open and needs a
9+
decision.
10+
11+
## 1. The recursive-type contradiction (design decision needed)
12+
13+
The runtime and the checker disagree about whether recursive named types exist.
14+
15+
- Runtime: `tryAs Name` resolves names late through `state.typeDefs`, so
16+
`type Tree = {value: int, kids?: [Tree]}` validates finite values correctly.
17+
Verified: `type A = [B]` / `type B = [A]` works at runtime.
18+
- Checker: `CheckProgram` pre-pass 1 resolves each `type` body in file order
19+
*before* declaring the name, so any self- or forward-reference is
20+
"unknown type" — recursive types are statically unrepresentable.
21+
22+
Consequence: recursive types (the natural way to type JSON trees, which is the
23+
flagship `tryAs` use case) work only when you *don't* pass `--check-types`.
24+
The two halves should agree. Options:
25+
26+
- **(a)** Register all type names before resolving bodies so the checker admits
27+
recursion. The arena/TypeId side needs a story for the cycle (a `TidNamed`
28+
indirection or iterative resolution). Runtime is already safe now that the
29+
depth budget exists. This is the option that makes the flagship use case work.
30+
- **(b)** Make the runtime reject what the checker rejects: resolve/validate the
31+
type AST eagerly at `type` declaration time and fail on unknown names. Loses
32+
recursive types entirely.
33+
34+
## 2. JSON integer footgun passes the checker silently (high)
35+
36+
`parseJson` produces a `float` for every JSON number, so any `int` inside a
37+
`tryAs` target is **always-none** against JSON data — and `--check-types` says
38+
nothing. Verified:
39+
40+
```mshell
41+
"[1, 2]" parseJson tryAs [int] # always none; checker silent
42+
```
43+
44+
The docs warn about this (good), but the branch already added a checker error
45+
for statically-impossible casts (`5 tryAs str`); this is the same bug class and
46+
the one users will actually hit, since `{count: int}` is what everyone writes
47+
first. Options:
48+
49+
- Checker warning when the cast source is the JSON output union and the target
50+
contains `int` anywhere.
51+
- Accept integral floats for `int` in `validateObj` — rejected in the branch's
52+
own comments because validation deliberately mirrors `match` type-arm
53+
semantics; changing it here without changing `match` creates a new
54+
inconsistency.
55+
- Make `parseJson` produce `int` for integral numbers — biggest change, fixes
56+
the root cause, affects existing scripts that assume float.
57+
58+
## 3. Wildcard shapes validate nothing at runtime (medium)
59+
60+
`TypeShapeExpr.validateObj` never reads `a.Wildcard`. Verified inconsistency:
61+
62+
```mshell
63+
{"a": 1, "b": "x"} tryAs {a: float, *: float} # just — wildcard ignored
64+
{"a": 1, "b": "x"} tryAs {*: float} # none — dict form checks values
65+
```
66+
67+
For static `as` dropping the wildcard was harmless (width subtyping already
68+
allows extra keys — comment at the `TypeShapeExpr` declaration in
69+
`mshell/TypeExpr.go`), but `tryAs` makes runtime claims: a user who writes
70+
`*: float` is asking for enforcement they silently don't get. Options:
71+
72+
- Validate non-field keys against the wildcard type in
73+
`TypeShapeExpr.validateObj` (my preference: it's what the syntax says).
74+
- Reject wildcard shapes as `tryAs` targets with a clear error (fail closed).
75+
76+
Either is better than the current silent half-validation.
77+
78+
## 4. Quotation targets are half-open (medium)
79+
80+
`tryAs (int -- int)` checks only that the value is a quotation
81+
(`TypeQuoteExpr.validateObj`) — signatures are not verified, so
82+
`("hi" wl) tryAs (int -- int)` produces `just`, and the checker afterward
83+
trusts `Maybe[(int -- int)]` for a value with a different signature. That is a
84+
static-soundness hole shaped like a checked cast. It is documented, but
85+
consider failing closed instead: reject quote types in `tryAs` targets with an
86+
error ("quotation signatures cannot be verified at runtime"), rather than
87+
issuing a certificate the runtime can't back. If kind-only checking is kept,
88+
it stays a documented sharp edge.
89+
90+
## 5. Union arm errors are value-dependent (low)
91+
92+
`1 tryAs int | Bogus` never reports the unknown type `Bogus` (first arm
93+
matches, short-circuit); a value that matches no arm hits the error instead.
94+
So whether a malformed type expression is diagnosed depends on the data. A
95+
one-time eager walk of the type AST at cast time (validating all names/arity)
96+
would make the errors deterministic. Cheap to do now; confusing bug reports
97+
later if skipped.
98+
99+
## 6. Notes, no action required
100+
101+
- `validateObj`'s `case Maybe:` (value, non-pointer) branch is dead code — the
102+
codebase only ever pushes `*Maybe`. Harmless.
103+
- The `just` result retains the original mutable object; a post-validation
104+
mutation through an alias (`append` on a still-reachable inner list) can
105+
invalidate the certified type. Inherent to the language's aliasing
106+
semantics, not specific to this branch; maybe worth one sentence in the
107+
docs next to "parse, don't validate".
108+
- Lexer edges verified good: `tryas`, `tryAsX`, `tryA`, `trying` all stay
109+
LITERAL; bare `tryAs` and `Maybe`-without-argument produce clean parse
110+
errors.
111+
- `builtinNamedTypes` table (single source for checker resolution + runtime
112+
check) and the `TypeExpression` interface forcing `validateObj` on every
113+
node kind are both good structure — keep that pattern for future node kinds.
114+
- Test-coverage suggestions once decisions above land: wildcard-shape
115+
behavior (whichever way it goes), a quotation-target case, nested
116+
`Maybe[Maybe[T]]`, and a `tryAs` inside a quotation in `tests/success/`
117+
(works today — verified `(tryAs int ...) map` — but only top-level uses are
118+
covered, and quotations dispatch through the separate `evaluateItems` path
119+
in `mshell/Evaluator.go`).

doc/mshell.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -590,6 +590,7 @@ Shape validation allows extra keys, requires all non-optional fields, and valida
590590
A `none` conforms to `Maybe[T]` for every `T`; quotation types are checked by kind only (signatures are not verified at runtime).
591591
Named types must be declared before the cast runs.
592592
Note that `parseJson` produces a `float` for every JSON number, so validate JSON numbers as `float`, not `int`.
593+
Validation depth is limited to 1024 nested levels; exceeding it (a cyclic value, a cyclic `type` declaration, or absurdly deep data) fails the script with a clear error rather than producing `none`.
593594

594595
Dictionary types are split into homogeneous dictionaries and shapes.
595596
A homogeneous dictionary is for dynamic keys where every value has the same type.

doc/type_system.inc.html

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,7 @@ <h2 id="type-system-tryas">Checked casts with <code>tryAs</code> <a class="secti
189189
A <code>none</code> conforms to <code>Maybe[T]</code> for every <code>T</code>; a <code>just</code> validates its inner value.
190190
Quotation types are checked by kind only — signatures are not verified at runtime.
191191
A named type must have been declared before the cast runs.
192+
Validation depth is limited to 1024 nested levels (structural nesting, named-type expansion, and union dispatch each consume one); exceeding the limit fails the script with a clear error rather than producing <code>none</code>, which turns a cyclic value or a cyclic <code>type</code> declaration into a diagnosable failure.
192193
</p>
193194

194195
<h1 id="type-system-dictionaries">Dictionaries <a class="section-link" href="#type-system-dictionaries" aria-label="Permalink">§</a> <a class="back-to-top" href="#type-system-top">Back to top</a></h1>

mshell/Evaluator.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -947,7 +947,7 @@ func (state *EvalState) processTryAsCast(cast *MShellTryAsCast, stack *MShellSta
947947
if err != nil {
948948
return state.FailWithMessage(fmt.Sprintf("%d:%d: 'tryAs' requires a value on the stack.\n", cast.TryAsToken.Line, cast.TryAsToken.Column))
949949
}
950-
conforms, verr := cast.Target.validateObj(state, obj)
950+
conforms, verr := cast.Target.validateObj(state, obj, maxTryAsValidateDepth)
951951
if verr != nil {
952952
return state.FailWithMessage(fmt.Sprintf("%d:%d: %s\n", cast.TryAsToken.Line, cast.TryAsToken.Column, verr.Error()))
953953
}

mshell/TypeExpr.go

Lines changed: 58 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -75,11 +75,32 @@ func (parser *MShellParser) skipToShapeSync() {
7575
// introduced without implementing runtime validation — the program will
7676
// not compile. validateObj is the runtime half of `tryAs`: it reports
7777
// whether a value conforms to the type. Its error return is reserved for
78-
// malformed casts (an unknown type name), which fail the script rather
79-
// than producing a `none`.
78+
// malformed casts (an unknown type name, or a blown depth budget), which
79+
// fail the script rather than producing a `none`.
80+
//
81+
// depth is the remaining recursion budget. Every node checks it on entry
82+
// and passes depth-1 to child validations, so the walk is bounded even
83+
// against a cyclic value (a list appended to itself) or a runtime type
84+
// environment with a cyclic declaration (`type Loop = Loop`), both of
85+
// which would otherwise overflow the Go stack and kill the process.
86+
// Entry points start at maxTryAsValidateDepth.
8087
type TypeExpression interface {
8188
MShellParseItem
82-
validateObj(state *EvalState, obj MShellObject) (bool, error)
89+
validateObj(state *EvalState, obj MShellObject, depth int) (bool, error)
90+
}
91+
92+
// maxTryAsValidateDepth bounds validateObj recursion. Structural nesting,
93+
// named-type expansion, and union dispatch each consume one level, so a
94+
// recursive union type like `type T = [float | T]` spends about four
95+
// levels per level of value nesting — 1024 still admits values nested
96+
// ~250 deep, well past any real boundary data (serde, for comparison,
97+
// caps JSON at 128), while turning cyclic values and cyclic type
98+
// declarations into a loud, catchable script failure instead of a fatal
99+
// stack overflow.
100+
const maxTryAsValidateDepth = 1024
101+
102+
func errTryAsDepth() error {
103+
return fmt.Errorf("tryAs validation exceeded the maximum depth of %d; the value or a 'type' declaration is likely cyclic", maxTryAsValidateDepth)
83104
}
84105

85106
// TypePrim is a primitive type keyword (int, float, bool, str).
@@ -855,7 +876,10 @@ var builtinNamedTypes = map[string]builtinNamedType{
855876
// validation allows extra keys (width subtyping), requires all
856877
// non-optional fields, and validates optional fields only when present.
857878

858-
func (a *TypePrim) validateObj(state *EvalState, obj MShellObject) (bool, error) {
879+
func (a *TypePrim) validateObj(state *EvalState, obj MShellObject, depth int) (bool, error) {
880+
if depth <= 0 {
881+
return false, errTryAsDepth()
882+
}
859883
switch a.Tid {
860884
case TidInt:
861885
_, ok := obj.(MShellInt)
@@ -876,7 +900,10 @@ func (a *TypePrim) validateObj(state *EvalState, obj MShellObject) (bool, error)
876900
return false, fmt.Errorf("tryAs cannot check the type '%s'", a.Tok.Lexeme)
877901
}
878902

879-
func (a *TypeNamed) validateObj(state *EvalState, obj MShellObject) (bool, error) {
903+
func (a *TypeNamed) validateObj(state *EvalState, obj MShellObject, depth int) (bool, error) {
904+
if depth <= 0 {
905+
return false, errTryAsDepth()
906+
}
880907
if bt, ok := builtinNamedTypes[a.Name]; ok {
881908
return bt.runtimeCheck(obj), nil
882909
}
@@ -897,44 +924,53 @@ func (a *TypeNamed) validateObj(state *EvalState, obj MShellObject) (bool, error
897924
if len(a.Args) != 1 {
898925
return false, fmt.Errorf("Maybe requires a type argument: Maybe[T]")
899926
}
900-
return a.Args[0].validateObj(state, inner)
927+
return a.Args[0].validateObj(state, inner, depth-1)
901928
}
902929
body, ok := state.typeDefs[a.Name]
903930
if !ok {
904931
return false, fmt.Errorf("unknown type '%s' in tryAs; a `type %s = ...` declaration must run before the cast", a.Name, a.Name)
905932
}
906-
return body.validateObj(state, obj)
933+
return body.validateObj(state, obj, depth-1)
907934
}
908935

909-
func (a *TypeListExpr) validateObj(state *EvalState, obj MShellObject) (bool, error) {
936+
func (a *TypeListExpr) validateObj(state *EvalState, obj MShellObject, depth int) (bool, error) {
937+
if depth <= 0 {
938+
return false, errTryAsDepth()
939+
}
910940
list, ok := obj.(*MShellList)
911941
if !ok {
912942
return false, nil
913943
}
914944
for _, item := range list.Items {
915-
itemOk, err := a.Elem.validateObj(state, item)
945+
itemOk, err := a.Elem.validateObj(state, item, depth-1)
916946
if err != nil || !itemOk {
917947
return itemOk, err
918948
}
919949
}
920950
return true, nil
921951
}
922952

923-
func (a *TypeDictExpr) validateObj(state *EvalState, obj MShellObject) (bool, error) {
953+
func (a *TypeDictExpr) validateObj(state *EvalState, obj MShellObject, depth int) (bool, error) {
954+
if depth <= 0 {
955+
return false, errTryAsDepth()
956+
}
924957
dict, ok := obj.(*MShellDict)
925958
if !ok {
926959
return false, nil
927960
}
928961
for _, val := range dict.Items {
929-
valOk, err := a.Value.validateObj(state, val)
962+
valOk, err := a.Value.validateObj(state, val, depth-1)
930963
if err != nil || !valOk {
931964
return valOk, err
932965
}
933966
}
934967
return true, nil
935968
}
936969

937-
func (a *TypeShapeExpr) validateObj(state *EvalState, obj MShellObject) (bool, error) {
970+
func (a *TypeShapeExpr) validateObj(state *EvalState, obj MShellObject, depth int) (bool, error) {
971+
if depth <= 0 {
972+
return false, errTryAsDepth()
973+
}
938974
dict, ok := obj.(*MShellDict)
939975
if !ok {
940976
return false, nil
@@ -947,22 +983,28 @@ func (a *TypeShapeExpr) validateObj(state *EvalState, obj MShellObject) (bool, e
947983
}
948984
return false, nil
949985
}
950-
fieldOk, err := f.Type.validateObj(state, val)
986+
fieldOk, err := f.Type.validateObj(state, val, depth-1)
951987
if err != nil || !fieldOk {
952988
return fieldOk, err
953989
}
954990
}
955991
return true, nil
956992
}
957993

958-
func (a *TypeQuoteExpr) validateObj(state *EvalState, obj MShellObject) (bool, error) {
994+
func (a *TypeQuoteExpr) validateObj(state *EvalState, obj MShellObject, depth int) (bool, error) {
995+
if depth <= 0 {
996+
return false, errTryAsDepth()
997+
}
959998
_, ok := obj.(*MShellQuotation)
960999
return ok, nil
9611000
}
9621001

963-
func (a *TypeUnionExpr) validateObj(state *EvalState, obj MShellObject) (bool, error) {
1002+
func (a *TypeUnionExpr) validateObj(state *EvalState, obj MShellObject, depth int) (bool, error) {
1003+
if depth <= 0 {
1004+
return false, errTryAsDepth()
1005+
}
9641006
for _, arm := range a.Arms {
965-
armOk, err := arm.validateObj(state, obj)
1007+
armOk, err := arm.validateObj(state, obj, depth-1)
9661008
if err != nil {
9671009
return false, err
9681010
}

tests/fail/tryas_cyclic_type.msh

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
# A cyclic runtime type declaration must exhaust the tryAs depth
2+
# budget and fail loudly instead of overflowing the Go stack.
3+
type Loop = Loop
4+
1 tryAs Loop
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
4:3: tryAs validation exceeded the maximum depth of 1024; the value or a 'type' declaration is likely cyclic

tests/fail/tryas_cyclic_value.msh

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
# A cyclic value (a list containing itself) reached through a
2+
# recursive type must exhaust the tryAs depth budget and fail
3+
# loudly instead of overflowing the Go stack.
4+
type T = [int | T]
5+
[1] l! @l @l append tryAs T
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
5:21: tryAs validation exceeded the maximum depth of 1024; the value or a 'type' declaration is likely cyclic

0 commit comments

Comments
 (0)