From 1e809ec6603f944af523c99a5dcef198f4108d89 Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Mon, 11 May 2026 15:26:41 +0000 Subject: [PATCH] fix: single-quote literalness (B2) + LooksLikePath trailing-backslash (B3) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two parser correctness fixes; public API surface unchanged. B2 — single-quote literalness v0.1.0-alpha resolved `$HOME` uniformly regardless of quote style, so `echo '$HOME'` produced Kind=Tilde. Per SPEC §5 single-quoted contents are literal bytes — no variable expansion. Lexer now marks single-quoted QuotedString tokens with an internal IsSingleQuoted flag and the resolver short-circuits tilde / $HOME / $VAR / glob / filesystem:: handling when set. `cat '/etc/passwd'` still resolves a literal absolute path; `echo '$HOME'` stays Kind=Literal. B3 — LooksLikePath trailing-backslash carve-out A double-quoted token like `"foo\\"` lexes to Value `foo\` (escape collapse). The trailing `\` is an artifact, not a meaningful path signal, but the heuristic was returning IsPath=true. Tightened the rule: forward slash anywhere still counts, backslash counts only when it's at a non-trailing position. `dir/` still classifies as a path (trailing `/` is a meaningful bash directory hint). Changes: - src/ShellSyntaxTree/Internal/Bash/Lexing/BashToken.cs: new IsSingleQuoted init-only flag (additive on the internal record). - src/ShellSyntaxTree/Internal/Bash/Lexing/BashLexer.cs: ReadSingleQuoted sets IsSingleQuoted=true on its emitted token. - src/ShellSyntaxTree/Internal/Resolving/BashResolver.cs: new Resolve overload accepting isLiteralBytes; LooksLikePath updated for the backslash carve-out. - src/ShellSyntaxTree/Internal/Bash/Parsing/BashCommandParser.cs: threads token.IsSingleQuoted through to the resolver from the QuotedString case. - SPEC.md §8: new Step 0 (Single-quoted bypass) preamble + LooksLikePath heuristic clarification. - tests/.../Corpus/bash/104, 109: updated to assert corrected outputs. - tests/.../Corpus/bash/119, 120, 121: new regression guards (single-quoted absolute path still resolves, cd dir/ still a path, single-quoted $VAR/file stays literal). - Directory.Build.props + RELEASE_NOTES.md: bump to 0.1.2-alpha. Public API impact: zero. No PublicApiSnapshotTests delta. dotnet build -c Release: 0 warnings, 0 errors. dotnet test -c Release: 359/359 passing (was 356; +3 entries). dotnet pack: ShellSyntaxTree.0.1.2-alpha.nupkg + .snupkg clean. --- Directory.Build.props | 2 +- RELEASE_NOTES.md | 33 ++++++++++++ SPEC.md | 15 +++++- .../Internal/Bash/Lexing/BashLexer.cs | 3 +- .../Internal/Bash/Lexing/BashToken.cs | 14 ++++- .../Bash/Parsing/BashCommandParser.cs | 6 ++- .../Internal/Resolving/BashResolver.cs | 52 +++++++++++++++++-- .../104_echo_single_quoted_literal_home.json | 6 +-- .../109_echo_trailing_backslash_escaped.json | 6 +-- .../119_cat_single_quoted_absolute_path.json | 20 +++++++ .../bash/120_cd_trailing_forward_slash.json | 20 +++++++ .../121_rm_single_quoted_var_pattern.json | 20 +++++++ 12 files changed, 181 insertions(+), 16 deletions(-) create mode 100644 tests/ShellSyntaxTree.Tests/Corpus/bash/119_cat_single_quoted_absolute_path.json create mode 100644 tests/ShellSyntaxTree.Tests/Corpus/bash/120_cd_trailing_forward_slash.json create mode 100644 tests/ShellSyntaxTree.Tests/Corpus/bash/121_rm_single_quoted_var_pattern.json diff --git a/Directory.Build.props b/Directory.Build.props index 53dc724..8f78ad0 100644 --- a/Directory.Build.props +++ b/Directory.Build.props @@ -7,7 +7,7 @@ latest enable true - 0.1.1 + 0.1.2 alpha diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index 87df713..fe2852b 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -1,3 +1,36 @@ +#### 0.1.2-alpha May 11th 2026 #### + +Two parser correctness fixes. Public API unchanged. + +**Fixed** + +- **Single-quoted strings are now literal per SPEC §5 (B2).** Previously + `echo '$HOME'` produced `Kind=Tilde` because the resolver substituted + `$HOME` uniformly regardless of quote style. Now the lexer marks + single-quoted `QuotedString` tokens with the internal `IsSingleQuoted` + flag, and the resolver bypasses tilde / `$HOME` / `$VAR` / glob / + `filesystem::` handling for them. `echo '$HOME'` stays `Kind=Literal`, + `Resolved=null`; `cat '/etc/passwd'` still resolves a path. Matches + bash semantics. +- **`LooksLikePath` no longer false-positives on a lone trailing + backslash (B3).** A double-quoted token like `"foo\\"` lexes to + Value `foo\`; the trailing `\` is an escape-collapse artifact, not a + meaningful path signal. The heuristic now requires a backslash at a + non-trailing position. Forward-slash behavior is unchanged — `dir/` + still classifies as a path (trailing `/` is a meaningful bash + directory hint). + +**Behavior notes** + +- Public API surface is unchanged (no `PublicApiSnapshotTests` delta). +- SPEC.md §8: new "Step 0: Single-quoted bypass" preamble; LooksLikePath + heuristic updated to call out the trailing-backslash carve-out. +- Corpus entries 104 (`echo 'literal $HOME'`) and 109 (`echo "trailing + backslash\\"`) updated to the corrected outputs. Three new entries + (119–121) pin the regression guards: single-quoted absolute paths + still resolve, `cd dir/` still classifies as a path, single-quoted + `$VAR` stays literal under `rm`. + #### 0.1.1-alpha May 11th 2026 #### Bug fix release for v0.1.0-alpha consumers. diff --git a/SPEC.md b/SPEC.md index e015b5f..9b0852b 100644 --- a/SPEC.md +++ b/SPEC.md @@ -619,6 +619,14 @@ path, then the verb chain continues with `log`. For each Arg with potential path content, the resolver attempts to produce a normalized absolute path. Resolution order: +0. **Single-quoted bypass.** If the source token came from a single-quoted + string (per §5: bytes are preserved literally — no escape processing, + no variable expansion), the resolver skips steps 1–5 entirely. Kind is + `Literal`; `IsPath` is `true` and `Resolved` is set only when the slot + is a path AND `TryResolveAbsolutePath` on the raw bytes succeeds. So + `cat '/etc/passwd'` still produces a resolved path, but `echo '$HOME'` + stays literal — `$HOME` is not expanded inside single quotes. + 1. **Tilde expansion.** `~` → `BashParserOptions.HomeDirectory`. `~/foo` → `/foo`. `~user` not supported → `DynamicSkip`. @@ -675,12 +683,17 @@ LooksLikePath(token) = || token starts with '\\' or ':' (Windows absolute) || token starts with './' or '../' (Unix relative) || token starts with '~' (Tilde) -|| token contains '/' or '\\' anywhere +|| token contains '/' anywhere +|| token contains '\\' at a NON-TRAILING position || token ends with a known file extension (.json, .md, .txt, .conf, ...) || token is in the args of a FileVerb at a position the per-verb rule marks as a path ``` +A lone trailing `\\` is excluded because it commonly appears as a +double-quote escape-collapse artifact (`"foo\\"` lexes to Value `foo\\`) +and is not a meaningful path signal on its own. + The per-verb rule wins when present; the heuristic is the fallback. --- diff --git a/src/ShellSyntaxTree/Internal/Bash/Lexing/BashLexer.cs b/src/ShellSyntaxTree/Internal/Bash/Lexing/BashLexer.cs index 2b90b47..2373c2b 100644 --- a/src/ShellSyntaxTree/Internal/Bash/Lexing/BashLexer.cs +++ b/src/ShellSyntaxTree/Internal/Bash/Lexing/BashLexer.cs @@ -270,7 +270,8 @@ private static int ReadSingleQuoted( // Strip the delimiters from the value per SPEC §5. var inner = src.Slice(start + 1, i - start - 1).ToString(); tokens.Add(new BashToken( - BashTokenKind.QuotedString, inner, null, start, (i - start) + 1, null)); + BashTokenKind.QuotedString, inner, null, start, (i - start) + 1, null) + { IsSingleQuoted = true }); return i + 1; } diff --git a/src/ShellSyntaxTree/Internal/Bash/Lexing/BashToken.cs b/src/ShellSyntaxTree/Internal/Bash/Lexing/BashToken.cs index 1bd000d..4203f67 100644 --- a/src/ShellSyntaxTree/Internal/Bash/Lexing/BashToken.cs +++ b/src/ShellSyntaxTree/Internal/Bash/Lexing/BashToken.cs @@ -36,4 +36,16 @@ internal readonly record struct BashToken( string? OperatorText, int SourceStart, int SourceLength, - string? UnparseableReason); + string? UnparseableReason) +{ + /// + /// True when this is a single-quoted . + /// Bash semantics: contents are literal bytes — no variable expansion, + /// no glob handling, no filesystem:: stripping. The resolver + /// consults this flag to bypass meta-character processing on the + /// token's . Default false for every other + /// kind and for double-quoted strings (which allow $HOME + /// substitution per SPEC §8 step 3). + /// + public bool IsSingleQuoted { get; init; } +} diff --git a/src/ShellSyntaxTree/Internal/Bash/Parsing/BashCommandParser.cs b/src/ShellSyntaxTree/Internal/Bash/Parsing/BashCommandParser.cs index bd1acdd..a47ad06 100644 --- a/src/ShellSyntaxTree/Internal/Bash/Parsing/BashCommandParser.cs +++ b/src/ShellSyntaxTree/Internal/Bash/Parsing/BashCommandParser.cs @@ -1190,7 +1190,11 @@ private static void ExtractRedirectsAndArgs( positionalIndex++; } - var (kind, resolved, isPath) = BashResolver.Resolve(t.Value, treatAsPath, options, workingDirectoryUnknown); + // Single-quoted tokens carry literal bytes per SPEC §5 + // — bypass tilde / $HOME / $VAR / glob handling so + // `'$HOME'` doesn't expand. + var (kind, resolved, isPath) = BashResolver.Resolve( + t.Value, treatAsPath, options, workingDirectoryUnknown, t.IsSingleQuoted); argList.Add(new Arg { Raw = sourceRaw, diff --git a/src/ShellSyntaxTree/Internal/Resolving/BashResolver.cs b/src/ShellSyntaxTree/Internal/Resolving/BashResolver.cs index f6a81ae..e38dfba 100644 --- a/src/ShellSyntaxTree/Internal/Resolving/BashResolver.cs +++ b/src/ShellSyntaxTree/Internal/Resolving/BashResolver.cs @@ -69,7 +69,14 @@ internal static class BashResolver /// internal static (ArgKind Kind, string? Resolved, bool IsPath) Resolve( string raw, bool treatAsPath, BashParserOptions options) => - Resolve(raw, treatAsPath, options, workingDirectoryUnknown: false); + Resolve(raw, treatAsPath, options, workingDirectoryUnknown: false, isLiteralBytes: false); + + internal static (ArgKind Kind, string? Resolved, bool IsPath) Resolve( + string raw, + bool treatAsPath, + BashParserOptions options, + bool workingDirectoryUnknown) => + Resolve(raw, treatAsPath, options, workingDirectoryUnknown, isLiteralBytes: false); /// /// Internal extended-resolver entry point. PR 5 adds the @@ -78,13 +85,18 @@ internal static (ArgKind Kind, string? Resolved, bool IsPath) Resolve( /// preceding clause did cd $VAR, we statically don't know the /// working directory of subsequent clauses, so relative-path args /// resolve to DynamicSkip instead of falling back to the - /// daemon cwd. + /// daemon cwd. v0.1.2 adds so the + /// parser can tell the resolver "this token came from a single-quoted + /// string — treat its bytes as opaque literals per SPEC §5"; that + /// suppresses tilde / $HOME / $VAR / glob / + /// filesystem:: handling so '$HOME' no longer expands. /// internal static (ArgKind Kind, string? Resolved, bool IsPath) Resolve( string raw, bool treatAsPath, BashParserOptions options, - bool workingDirectoryUnknown) + bool workingDirectoryUnknown, + bool isLiteralBytes) { if (raw is null) { @@ -93,6 +105,24 @@ internal static (ArgKind Kind, string? Resolved, bool IsPath) Resolve( return (ArgKind.Literal, null, false); } + if (isLiteralBytes) + { + // Single-quoted token per SPEC §5: contents are literal bytes. + // No tilde / $HOME / $VAR / glob / filesystem:: handling. If + // this slot is a path AND the literal value happens to look + // like one (e.g. `cat '/etc/passwd'`), still normalize it; the + // user typed an absolute path inside single quotes. + if (!treatAsPath) + { + return (ArgKind.Literal, null, false); + } + + var resolvedLiteral = TryResolveAbsolutePath(raw, options, workingDirectoryUnknown); + return resolvedLiteral is null + ? (ArgKind.DynamicSkip, null, false) + : (ArgKind.Literal, resolvedLiteral, true); + } + // Step 1: filesystem::/path prefix stripping. Some agent tools emit // `filesystem::/path/to/file` (e.g. MCP filesystem servers). Strip // the prefix and continue with the remainder. We *do not* set a @@ -242,8 +272,20 @@ internal static bool LooksLikePath(string token) return true; } - // Any directory separator. - if (token.IndexOf('/') >= 0 || token.IndexOf('\\') >= 0) + // Forward slash anywhere counts (trailing `/` is a meaningful + // bash directory hint, e.g. `cd dir/`). + if (token.IndexOf('/') >= 0) + { + return true; + } + + // Backslash counts when it appears at a non-trailing position. A + // lone trailing `\` is typically a double-quote escape-collapse + // artifact (e.g. lexed `"foo\\"` → Value `foo\`), not a real + // path signal — accepting it would falsely classify + // `echo "trailing\\"` as a path. + var backslash = token.IndexOf('\\'); + if (backslash >= 0 && backslash < token.Length - 1) { return true; } diff --git a/tests/ShellSyntaxTree.Tests/Corpus/bash/104_echo_single_quoted_literal_home.json b/tests/ShellSyntaxTree.Tests/Corpus/bash/104_echo_single_quoted_literal_home.json index d224779..0e2e5c8 100644 --- a/tests/ShellSyntaxTree.Tests/Corpus/bash/104_echo_single_quoted_literal_home.json +++ b/tests/ShellSyntaxTree.Tests/Corpus/bash/104_echo_single_quoted_literal_home.json @@ -1,5 +1,5 @@ { - "name": "Quote: echo 'literal $HOME' (single quotes — but resolver still substitutes $HOME)", + "name": "Quote: echo 'literal $HOME' (single quotes — literal, no expansion)", "input": "echo 'literal $HOME'", "expected": { "isUnparseable": false, @@ -8,7 +8,7 @@ "operator": "None", "verb": ["echo"], "args": [ - { "raw": "'literal $HOME'", "kind": "Tilde", "isPath": false, "resolved": "__NULL__" } + { "raw": "'literal $HOME'", "kind": "Literal", "isPath": false, "resolved": "__NULL__" } ], "redirects": [], "isSubshell": false, @@ -16,5 +16,5 @@ } ] }, - "notes": "Single-quote lexer preservation (SPEC §5) keeps '$HOME' as literal bytes in the Value, but BashResolver runs uniformly on the Value and substitutes $HOME → Kind=Tilde. This diverges from real bash (which wouldn't expand inside single quotes). Documented v0.1 behavior; consumer gets Kind=Tilde, no IsPath signal." + "notes": "v0.1.2 / B2: single-quoted tokens are literal bytes per SPEC §5 — the resolver bypasses tilde / $HOME / $VAR / glob / filesystem:: handling when BashToken.IsSingleQuoted is true. Kind stays Literal; $HOME stays inside the Raw verbatim and nothing is resolved." } diff --git a/tests/ShellSyntaxTree.Tests/Corpus/bash/109_echo_trailing_backslash_escaped.json b/tests/ShellSyntaxTree.Tests/Corpus/bash/109_echo_trailing_backslash_escaped.json index b318226..9fc9251 100644 --- a/tests/ShellSyntaxTree.Tests/Corpus/bash/109_echo_trailing_backslash_escaped.json +++ b/tests/ShellSyntaxTree.Tests/Corpus/bash/109_echo_trailing_backslash_escaped.json @@ -1,5 +1,5 @@ { - "name": "Quote: echo \"trailing backslash\\\\\" (escaped backslash inside double-quote)", + "name": "Quote: echo \"trailing backslash\\\\\" (trailing-only backslash isn't a path)", "input": "echo \"trailing backslash\\\\\"", "expected": { "isUnparseable": false, @@ -8,7 +8,7 @@ "operator": "None", "verb": ["echo"], "args": [ - { "raw": "\"trailing backslash\\\\\"", "kind": "Literal", "isPath": true, "resolved": "/work/trailing backslash" } + { "raw": "\"trailing backslash\\\\\"", "kind": "Literal", "isPath": false, "resolved": "__NULL__" } ], "redirects": [], "isSubshell": false, @@ -16,5 +16,5 @@ } ] }, - "notes": "Inside double quotes \\\\ collapses to a single literal \\. The post-escape Value is `trailing backslash\\` — which contains a directory-separator character (backslash) so LooksLikePath returns true even though echo isn't a FileVerb. The resolver normalizes the trailing \\ away during path normalization → /work/trailing backslash. Documented v0.1 behavior; SPEC clarification candidate: should LooksLikePath ignore trailing-only backslashes?" + "notes": "v0.1.2 / B3: inside double quotes \\\\ collapses to a single literal \\. The post-escape Value is `trailing backslash\\` — but LooksLikePath now requires a backslash at a non-trailing position, so the lone trailing artifact no longer falsely classifies this as a path. Kind=Literal, IsPath=false, Resolved=null." } diff --git a/tests/ShellSyntaxTree.Tests/Corpus/bash/119_cat_single_quoted_absolute_path.json b/tests/ShellSyntaxTree.Tests/Corpus/bash/119_cat_single_quoted_absolute_path.json new file mode 100644 index 0000000..4a93328 --- /dev/null +++ b/tests/ShellSyntaxTree.Tests/Corpus/bash/119_cat_single_quoted_absolute_path.json @@ -0,0 +1,20 @@ +{ + "name": "Quote: cat '/etc/passwd' (single-quoted absolute path still resolves)", + "input": "cat '/etc/passwd'", + "expected": { + "isUnparseable": false, + "clauses": [ + { + "operator": "None", + "verb": ["cat"], + "args": [ + { "raw": "'/etc/passwd'", "kind": "Literal", "isPath": true, "resolved": "/etc/passwd" } + ], + "redirects": [], + "isSubshell": false, + "isBashCWrapped": false + } + ] + }, + "notes": "v0.1.2 / B2: single-quoting suppresses meta-character expansion (tilde / $VAR / glob) but a single-quoted absolute path still resolves through the path-arg slot. cat is a FileVerb so the positional is a path slot; the literal bytes happen to be a rooted absolute path." +} diff --git a/tests/ShellSyntaxTree.Tests/Corpus/bash/120_cd_trailing_forward_slash.json b/tests/ShellSyntaxTree.Tests/Corpus/bash/120_cd_trailing_forward_slash.json new file mode 100644 index 0000000..edf7ca0 --- /dev/null +++ b/tests/ShellSyntaxTree.Tests/Corpus/bash/120_cd_trailing_forward_slash.json @@ -0,0 +1,20 @@ +{ + "name": "Path: cd dir/ (trailing forward slash still classifies as path)", + "input": "cd dir/", + "expected": { + "isUnparseable": false, + "clauses": [ + { + "operator": "None", + "verb": ["cd"], + "args": [ + { "raw": "dir/", "kind": "Literal", "isPath": true, "resolved": "/work/dir" } + ], + "redirects": [], + "isSubshell": false, + "isBashCWrapped": false + } + ] + }, + "notes": "v0.1.2 / B3 regression guard: the LooksLikePath heuristic now treats a lone trailing backslash as a non-signal, but a trailing forward slash is a meaningful bash directory hint (`cd dir/` is common). Confirm `dir/` still resolves as a path." +} diff --git a/tests/ShellSyntaxTree.Tests/Corpus/bash/121_rm_single_quoted_var_pattern.json b/tests/ShellSyntaxTree.Tests/Corpus/bash/121_rm_single_quoted_var_pattern.json new file mode 100644 index 0000000..8facc61 --- /dev/null +++ b/tests/ShellSyntaxTree.Tests/Corpus/bash/121_rm_single_quoted_var_pattern.json @@ -0,0 +1,20 @@ +{ + "name": "Quote: rm '$VAR/file.txt' (single-quoted env var stays literal)", + "input": "rm '$VAR/file.txt'", + "expected": { + "isUnparseable": false, + "clauses": [ + { + "operator": "None", + "verb": ["rm"], + "args": [ + { "raw": "'$VAR/file.txt'", "kind": "Literal", "isPath": true, "resolved": "/work/$VAR/file.txt" } + ], + "redirects": [], + "isSubshell": false, + "isBashCWrapped": false + } + ] + }, + "notes": "v0.1.2 / B2: inside single quotes the `$VAR` token is literal — the resolver does NOT trigger the DynamicSkip path for unresolved env vars. The literal bytes `$VAR/file.txt` resolve under cwd as `/work/$VAR/file.txt` and Kind stays Literal. This matches bash semantics: `rm '$VAR/file.txt'` deletes the file literally named `$VAR/file.txt`." +}