From 24c84404f489a911150a8037b8b49c8b0bb7673d Mon Sep 17 00:00:00 2001
From: Ethan Fremen <ethan@mindlace.net>
Date: Sat, 1 Aug 2026 19:00:25 -0700
Subject: [PATCH] fix: don't treat Object.prototype members as reserved words
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

`readWord` classified a command-start word as a keyword with
`text in RESERVED_WORDS`. `in` walks the prototype chain, so any
Object.prototype member name matched, and `RESERVED_WORDS[text]` then
handed `setToken` a function instead of a `Token`.

Two names reach the lookup through the existing fast-path guard
(lowercase first char, length <= 8): `toString` and `valueOf`. Both are
plausible shell command/function names, and the failure is silent — the
bogus token makes the parser abandon the rest of the script with no
entry in `errors`:

    parse("toString foo")                    // commands: []
    parse("echo hi; toString foo; echo bye") // commands: [echo hi]

Inside a compound command it instead produces spurious errors
(`if toString; then echo a; fi` reports "expected 'then'").

Replace the `in` test plus second lookup with a single lookup guarded by
`typeof reserved === "number"`. Own entries are always numeric `Token`
values, inherited members never are. This also drops one hash lookup
from the hot path; bench/self.ts is unchanged-to-slightly-faster.

The other string-keyed lookup tables were checked: `UNARY_TEST_OPS` and
`BINARY_TEST_OPS` compare `=== 1`, and `REDIRECT_OPS` is keyed only by
lexer-produced operator strings, so none are reachable this way.
---
 src/lexer.ts        | 15 +++++++++------
 test/parser.test.ts | 17 +++++++++++++++++
 2 files changed, 26 insertions(+), 6 deletions(-)

diff --git a/src/lexer.ts b/src/lexer.ts
index 462badb..f7100d1 100644
--- a/src/lexer.ts
+++ b/src/lexer.ts
@@ -1000,12 +1000,15 @@ export class Lexer {
     if (ctx === LexContext.CommandStart) {
       if (!hasExpansions && !quoted) {
         const fc = text.charCodeAt(0);
-        if (
-          ((fc >= CH_a && fc <= CH_z && text.length <= 8) || fc === CH_BANG || fc === CH_LBRACE || fc === CH_RBRACE) &&
-          text in RESERVED_WORDS
-        ) {
-          setToken(out, RESERVED_WORDS[text], text, tokenStart, wordEnd);
-          return;
+        if ((fc >= CH_a && fc <= CH_z && text.length <= 8) || fc === CH_BANG || fc === CH_LBRACE || fc === CH_RBRACE) {
+          // Single lookup, then a typeof check — `text in RESERVED_WORDS` would also match
+          // inherited Object.prototype members, making `toString` and `valueOf` parse as
+          // keywords. Own reserved words are always numeric Token values.
+          const reserved = RESERVED_WORDS[text];
+          if (typeof reserved === "number") {
+            setToken(out, reserved, text, tokenStart, wordEnd);
+            return;
+          }
         }
         if (fc === CH_LBRACKET && text === "[[") {
           setToken(out, Token.DblLBracket, text, tokenStart, wordEnd);
diff --git a/test/parser.test.ts b/test/parser.test.ts
index 36a9b1f..733be2f 100644
--- a/test/parser.test.ts
+++ b/test/parser.test.ts
@@ -51,6 +51,23 @@ test("set and trap commands", () => {
   assert.equal(ast.commands.length, 4);
 });
 
+// Command names that collide with Object.prototype members must not be mistaken
+// for reserved words by the keyword lookup.
+test("Object.prototype member names are ordinary command names", () => {
+  for (const name of ["toString", "valueOf", "constructor", "hasOwnProperty", "__proto__", "isPrototypeOf"]) {
+    const ast = parse(`${name} arg`);
+    assert.equal(ast.commands.length, 1, `no command for: ${name}`);
+    assert.equal(getCmd(ast).name?.text, name);
+    assert.deepEqual(args(getCmd(ast)), ["arg"]);
+  }
+});
+
+test("Object.prototype member name does not truncate the rest of the script", () => {
+  const ast = parse("echo hi; toString foo; echo bye");
+  assert.equal(ast.commands.length, 3);
+  assert.equal(getCmd(ast, 2).name?.text, "echo");
+});
+
 // ── Real-world patterns ─────────────────────────────────────────────
 
 test("real-world scripts parse without errors", () => {