From 43443043284e2b5b692391797223ff3a05c1b9c7 Mon Sep 17 00:00:00 2001 From: Aleksandergreg Date: Sat, 10 Oct 2026 10:39:59 +0200 Subject: [PATCH] Command lists (;, &&, ||) - They behave like sh: && and || bind equally from left to right, and ls /nope || echo fallback shows the error and then fallback. - Each command expands variables and globs when its turn comes, so export X=1; echo $X prints 1 and cd logs && ls * lists the new directory. - The whole line is checked before anything runs, so a syntax error late in the line changes nothing. vi can't be used inside a list. - Lists also work in scripts. A failure handled by || is shown with its script line and the script carries on. - Exit status was needed to make this correct. grep with no match now fails silently, so grep -q ERROR log && echo alert works. I added grep -q, true and false; the last two don't count toward the Command Collector achievement. A plain grep with no match on its own line behaves exactly as before. - When a list's last command fails, the mission is still checked, because earlier commands may already have changed things. sort f > f empties the file, as on Linux. One deliberate difference: if the command fails, the file is put back. An existing safety test requires that a rejected command never destroys data, and AGENTS.md says not to weaken that, so I kept it. Ranks: I added Staff SRE (1,600 XP), Principal SRE (2,150) and Distinguished Engineer (2,700) above Senior SRE. The existing thresholds didn't change, so nobody gets demoted. Your own profile at 1,201 XP stays Senior SRE and is 399 XP from the next rank. A new test checks that finishing Linux reaches Staff SRE and the full catalog reaches the top rank. Smaller fixes: - ls: named files are listed first, as typed. Directories come after with headers, -a shows . and .., and a hidden file you name explicitly is shown. - Globs: * no longer matches hidden files, though .* does. find still matches them, as on Linux. - cut: field lists like -f1,3, 2- and -2 work. Lines without the delimiter pass through whole, -s is new, and multi-character delimiters are rejected. - find: repeated tests now all have to match. -o and ! give a clear "not supported" error. --- README.md | 2 +- docs/play/controls-and-commands.md | 18 +- docs/play/missions-and-progress.md | 7 +- docs/technical/sandbox-and-safety.md | 19 +- internal/game/environment.go | 11 + internal/game/progression.go | 6 +- internal/game/progression_test.go | 25 ++ internal/game/sandbox_environment.go | 3 + internal/game/session_execute.go | 23 +- internal/game/session_lifecycle_test.go | 30 +++ internal/profile/profile_test.go | 8 +- internal/profile/rank.go | 7 + internal/sandbox/command_list.go | 207 +++++++++++++++++ internal/sandbox/command_list_test.go | 215 ++++++++++++++++++ internal/sandbox/commands_columns.go | 111 ++++++++- internal/sandbox/commands_files.go | 71 +++--- internal/sandbox/commands_find.go | 70 +++--- .../sandbox/commands_gnu_fidelity_test.go | 118 ++++++++++ internal/sandbox/commands_grep.go | 21 +- internal/sandbox/commands_script.go | 20 ++ internal/sandbox/commands_script_test.go | 6 +- internal/sandbox/dispatch.go | 7 +- internal/sandbox/filesystem_queries.go | 19 +- internal/sandbox/manuals.go | 17 +- internal/sandbox/shell.go | 136 ++++++++++- internal/sandbox/syntax.go | 15 +- internal/sandbox/syntax_test.go | 9 +- 27 files changed, 1081 insertions(+), 120 deletions(-) create mode 100644 internal/sandbox/command_list.go create mode 100644 internal/sandbox/command_list_test.go diff --git a/README.md b/README.md index fe86061..95e88ce 100644 --- a/README.md +++ b/README.md @@ -32,7 +32,7 @@ OpsQuest validates the result, not a prescribed command. Equivalent supported so - 23 Linux missions across four ordered learning worlds - 16 optional, disposable Docker missions across three worlds: It Works on My Machine, Container Triage, and Network Plumbing - An isolated in-memory filesystem, environment, process table, and archives -- Quote-aware globs and variables, pipelines, redirection, and command history +- Quote-aware globs and variables, pipelines, redirection, `;`/`&&`/`||` command lists, and command history - A compact virtual `vi` and bounded virtual shell scripts - Progressive hints, free command guidance, XP, ranks, and six achievements - An optional loopback-only web companion for mission guidance and live progress diff --git a/docs/play/controls-and-commands.md b/docs/play/controls-and-commands.md index e1bc48b..7471fb0 100644 --- a/docs/play/controls-and-commands.md +++ b/docs/play/controls-and-commands.md @@ -29,14 +29,24 @@ Completion reads only the active environment's command vocabulary and virtual fi ## Teaching-shell command set ```text -awk basename cat cd chmod chown clear cp cut dirname du echo env export find +awk basename cat cd chmod chown clear cp cut dirname du echo env export false find grep gzip gunzip head help history kill less ls man mkdir mv printf ps pwd rm -rmdir sed sh sort stat tail tar touch tr uniq vi wc whoami +rmdir sed sh sort stat tail tar touch tr true uniq vi wc whoami ``` The shell also supports quote-aware variables and globs, pipelines (`|`), input redirection (`<`), and output redirection (`>` and `>>`). It intentionally implements a teaching subset of each command. Use `help COMMAND` for the exact supported flags and examples. -Enter one command per line. Command lists (`;`, `&&`, `||`), background jobs (`&`), subshells, command substitution, special parameters such as `$?`, and file-descriptor redirection such as `2>` are rejected with an explanation before anything runs. Quote or escape these characters to use them as data. +Command lists join commands on one line, as in `sh`: + +- `a; b` always runs `b`. +- `a && b` runs `b` only when `a` succeeds. +- `a || b` runs `b` only when `a` fails. + +`&&` and `||` bind equally from left to right, so `a && b || c` runs `c` when either `a` or `b` fails. Each command expands variables and globs when its turn comes, so `cd logs && ls *` lists the new directory. A failure is shown, then the list continues as its operators allow. `grep` without a match fails silently, which makes `grep -q ERROR app.log && echo alert` work; `true` and `false` set a status and nothing else. + +Output redirection with `>` empties its file before the command runs, so `sort notes.txt > notes.txt` leaves an empty file, just as on Linux. If the command fails, the file is put back. Wildcards skip hidden names unless the pattern starts with a dot: `*` ignores `.env`, while `.*` matches it. + +Background jobs (`&`), subshells, command substitution, special parameters such as `$?`, and file-descriptor redirection such as `2>` are rejected with an explanation before anything runs. Quote or escape these characters to use them as data. ## Mission navigation @@ -74,6 +84,6 @@ opsquest:/workspace$ ./report.sh `sh FILE` does not require executable permission. Direct paths require an executable mode and a supported shell shebang. Files, archives, and mission processes retain changes, while a script's working directory and exported environment are restored when it returns. -This is not a complete POSIX shell. Loops, conditionals, functions, substitutions, background jobs, external programs, positional arguments, `sh -c`, stdin-fed source, and interactive editor calls from scripts are rejected. +Command lists work in scripts too: a failure handled by `||` is reported with its script location and the script continues, and a script's exit status is its last command's. This is not a complete POSIX shell. Loops, `if` conditionals, functions, substitutions, background jobs, external programs, positional arguments, `sh -c`, stdin-fed source, and interactive editor calls from scripts are rejected. For implementation limits and trust boundaries, see [Sandbox and safety](../technical/sandbox-and-safety.md). diff --git a/docs/play/missions-and-progress.md b/docs/play/missions-and-progress.md index 9276068..a5edae9 100644 --- a/docs/play/missions-and-progress.md +++ b/docs/play/missions-and-progress.md @@ -64,7 +64,7 @@ may unlock achievements. Replays retain the original completion and reward. Every successful teaching command counts toward command mastery as soon as it runs, including during replays. Lab utilities such as `help`, `man`, `clear`, -and `history` are free to use and do not count. +`history`, `true`, and `false` are free to use and do not count. The displayed level is derived from total XP. Ranks progress through: @@ -73,7 +73,10 @@ The displayed level is derived from total XP. Ranks progress through: 3. Junior Sysadmin 4. Sysadmin 5. SRE -6. Senior SRE +6. Senior SRE (1,100 XP) +7. Staff SRE (1,600 XP, about the whole Linux track) +8. Principal SRE (2,150 XP) +9. Distinguished Engineer (2,700 XP, most of the Docker track as well) Achievements recognize the first fix, a three-command pipeline, ten practiced commands, five hint-free completions, an advanced incident, and completion of diff --git a/docs/technical/sandbox-and-safety.md b/docs/technical/sandbox-and-safety.md index ed4cc3b..6260113 100644 --- a/docs/technical/sandbox-and-safety.md +++ b/docs/technical/sandbox-and-safety.md @@ -58,18 +58,19 @@ Editable source: [`command-execution-pipeline.mmd`](diagrams/command-execution-p [`Sandbox.Execute`](https://github.com/Aleksandergreg/go-cli-tool/blob/main/internal/sandbox/shell.go) processes a line in explicit stages: 1. **Bound input:** reject a command line over 64 KiB before adding it to the attempt's 100-entry history. -2. **Lex:** reject unquoted shell syntax outside the teaching subset (command lists, background jobs, subshells, substitutions, special parameters, and file-descriptor redirection), then recognize words, quotes, escapes, comments, variables, pipes, and `<`, `>`, or `>>`. Interactive and script lines share these rules. Expansion reads only the sandbox environment. -3. **Parse:** build pipeline stages and attach at most one input and output redirection to each stage. -4. **Expand:** resolve eligible globs against the virtual filesystem and enforce expanded token and argument budgets. -5. **Preflight compositions:** reject unsupported interactive-editor or script placement before an earlier pipeline stage can mutate state. -6. **Dispatch:** call a Go method from the supported-command switch. Nested `find -exec` and scripts share dispatch budgets. -7. **Move virtual data:** pipeline output becomes the next stage's input; redirection reads or writes only virtual files. -8. **Return learning metadata:** output, successful command names, maximum pipeline width, or a virtual editor request flow back to `game.Session`. -9. **Observe outcomes:** output validators compare the returned text; state validators query the active environment. +2. **Split lists:** separate unquoted `;`, `&&`, and `||` into pipelines and check every pipeline's syntax before the first runs. Each pipeline then passes through the remaining stages only when its turn comes, so expansion sees earlier `cd` and `export` effects. +3. **Lex:** reject unquoted shell syntax outside the teaching subset (background jobs, subshells, substitutions, special parameters, and file-descriptor redirection), then recognize words, quotes, escapes, comments, variables, pipes, and `<`, `>`, or `>>`. Interactive and script lines share these rules. Expansion reads only the sandbox environment. +4. **Parse:** build pipeline stages and attach at most one input and output redirection to each stage. +5. **Expand:** resolve eligible globs against the virtual filesystem and enforce expanded token and argument budgets. +6. **Preflight compositions:** reject unsupported interactive-editor or script placement before an earlier pipeline stage can mutate state. +7. **Dispatch:** call a Go method from the supported-command switch. Every pipeline of a list, nested `find -exec`, and scripts share one dispatch budget, and a list's combined output shares the 2 MiB output limit. +8. **Move virtual data:** pipeline output becomes the next stage's input; redirection reads or writes only virtual files. A stage's `>` target is emptied just before it runs and restored if that stage fails. +9. **Return learning metadata:** output, successful command names, maximum pipeline width, an ordered output-and-error transcript for lists, or a virtual editor request flow back to `game.Session`. +10. **Observe outcomes:** output validators compare the returned text; state validators query the active environment. Unsupported commands fail at dispatch. The shell implements a teaching subset, not process lookup, so a name absent from the dispatcher can never fall through to the host. -A full pipeline is not one transaction: an ordinary earlier stage may mutate virtual state before a later stage fails. Safety-sensitive operations preflight their own affected state, and compositions known to be unsupported (`vi` in a pipeline, or a script receiving pipeline/file input) are rejected before any stage runs. +A full pipeline is not one transaction: an ordinary earlier stage may mutate virtual state before a later stage fails. Safety-sensitive operations preflight their own affected state, a failed stage restores the redirect target it emptied, and compositions known to be unsupported (`vi` in a pipeline or list, or a script receiving pipeline/file input) are rejected before any stage runs. A command list is likewise not a transaction: commands that ran before a failure keep their effects. ## Virtual state model diff --git a/internal/game/environment.go b/internal/game/environment.go index d008744..048c861 100644 --- a/internal/game/environment.go +++ b/internal/game/environment.go @@ -18,6 +18,17 @@ type Execution struct { PracticedCommands []string PipelineWidth int Interactive *InteractiveAction + // Transcript orders output and error messages when one line ran several + // commands (a; b, a && b, a || b). When it is empty, Output is the whole + // display. Output always holds only standard output for validators. + Transcript []TranscriptEntry +} + +// TranscriptEntry is command output, or an error message shown on the +// player's error stream. +type TranscriptEntry struct { + Text string + Error bool } // InteractiveAction lets an environment hand terminal-only work back to the diff --git a/internal/game/progression.go b/internal/game/progression.go index 6474921..c5007da 100644 --- a/internal/game/progression.go +++ b/internal/game/progression.go @@ -56,9 +56,9 @@ func ReconcileAchievements(player *profile.Profile, catalog mission.Catalog, now return unlocked } -// metaCommands explain or tidy the lab rather than practice an operations -// skill, so they never earn command mastery. -var metaCommands = map[string]bool{"clear": true, "help": true, "history": true, "man": true} +// metaCommands explain or tidy the lab, or only set an exit status, rather +// than practice an operations skill, so they never earn command mastery. +var metaCommands = map[string]bool{"clear": true, "false": true, "help": true, "history": true, "man": true, "true": true} // CountsAsPractice reports whether a successful command earns command mastery. func CountsAsPractice(command string) bool { return !metaCommands[command] } diff --git a/internal/game/progression_test.go b/internal/game/progression_test.go index 6916694..94e33f1 100644 --- a/internal/game/progression_test.go +++ b/internal/game/progression_test.go @@ -148,3 +148,28 @@ func TestSessionDoesNotRecordMetaCommandsAsPractice(t *testing.T) { t.Fatalf("completion did not list only teaching commands:\n%s", out.String()) } } + +// TestRanksSpanTheCatalog keeps rank thresholds meaningful as missions are +// added or removed: finishing Linux reaches Staff SRE, and only the Docker +// track leads to the top rank. +func TestRanksSpanTheCatalog(t *testing.T) { + catalog, err := mission.LoadCatalog() + if err != nil { + t.Fatal(err) + } + totals := map[string]int{} + for _, item := range catalog.All() { + totals[item.EffectiveTrack()] += item.Rewards.XP + } + linux := profile.Profile{XP: totals[mission.TrackLinux]} + if rank := linux.Rank(); rank != "Staff SRE" { + t.Errorf("Linux track total %d XP reaches %q, want Staff SRE", linux.XP, rank) + } + everything := profile.Profile{XP: totals[mission.TrackLinux] + totals[mission.TrackDocker]} + if rank := everything.Rank(); rank != "Distinguished Engineer" { + t.Errorf("catalog total %d XP reaches %q, want Distinguished Engineer", everything.XP, rank) + } + if _, _, hasNext := everything.NextRank(); hasNext { + t.Error("a rank remains beyond the whole catalog's XP") + } +} diff --git a/internal/game/sandbox_environment.go b/internal/game/sandbox_environment.go index e8c096d..d1e88f0 100644 --- a/internal/game/sandbox_environment.go +++ b/internal/game/sandbox_environment.go @@ -64,6 +64,9 @@ func (e *sandboxEnvironment) Execute(ctx context.Context, line string) (Executio PracticedCommands: slices.Clone(result.Commands), PipelineWidth: result.PipelineWidth, } + for _, chunk := range result.Transcript { + execution.Transcript = append(execution.Transcript, TranscriptEntry{Text: chunk.Text, Error: chunk.Error}) + } if result.Editor != nil { request := *result.Editor execution.Interactive = &InteractiveAction{ diff --git a/internal/game/session_execute.go b/internal/game/session_execute.go index 8f1e586..cf31e02 100644 --- a/internal/game/session_execute.go +++ b/internal/game/session_execute.go @@ -16,11 +16,16 @@ func (a *attempt) execute(line string) (step, error) { if err != nil || !ran { return step{}, err } - if result.Output != "" { - fmt.Fprint(a.Out, result.Output) - if !strings.HasSuffix(result.Output, "\n") { - fmt.Fprintln(a.Out) + if len(result.Transcript) > 0 { + for _, entry := range result.Transcript { + if entry.Error { + a.fail(entry.Text) + } else { + a.printOutput(entry.Text) + } } + } else { + a.printOutput(result.Output) } a.lastOutput = result.Output unlocked, err := a.recordPractice(result) @@ -48,6 +53,16 @@ func (a *attempt) execute(line string) (step, error) { return finish(completed), nil } +func (a *attempt) printOutput(output string) { + if output == "" { + return + } + fmt.Fprint(a.Out, output) + if !strings.HasSuffix(output, "\n") { + fmt.Fprintln(a.Out) + } +} + // runCommand executes line and any interactive follow-up. ran is false when // a recoverable command error was already shown to the player. func (a *attempt) runCommand(line string) (Execution, bool, error) { diff --git a/internal/game/session_lifecycle_test.go b/internal/game/session_lifecycle_test.go index c72ee90..7a3b3b2 100644 --- a/internal/game/session_lifecycle_test.go +++ b/internal/game/session_lifecycle_test.go @@ -354,3 +354,33 @@ func TestSessionStaysInCurrentLabWhenSwitchTargetIsUnavailable(t *testing.T) { } } } + +func TestCommandListsPrintInOrderAndStillValidate(t *testing.T) { + catalog, item := seamCatalogMission(t, "linux-orientation") + terminal := &bytes.Buffer{} + player := profile.New("tester") + session := Session{ + Mission: item, + Player: &player, + Saver: profile.NewStore(filepath.Join(t.TempDir(), "profile.json"), "tester"), + Out: terminal, + ErrOut: terminal, + Reader: &seamReader{lines: []string{"ls /missing; pwd && ls /missing"}}, + Catalog: catalog, + } + + result, err := session.Run() + if err != nil || !result.Completed { + t.Fatalf("Run() = %#v, %v; want the pwd output to complete the mission", result, err) + } + transcript := terminal.String() + first := strings.Index(transcript, "ls: /missing: no such file or directory") + output := strings.Index(transcript, "/home/operator\n") + last := strings.LastIndex(transcript, "ls: /missing: no such file or directory") + if first < 0 || output < first || last < output { + t.Fatalf("list output is out of order:\n%s", transcript) + } + if player.Commands["pwd"] != 1 || player.Commands["ls"] != 0 { + t.Errorf("practice = %#v, want only the successful pwd", player.Commands) + } +} diff --git a/internal/profile/profile_test.go b/internal/profile/profile_test.go index 02150af..e5b2cad 100644 --- a/internal/profile/profile_test.go +++ b/internal/profile/profile_test.go @@ -288,7 +288,13 @@ func TestRankAndNextRankShareThresholds(t *testing.T) { {xp: 0, rank: "Intern", next: "Operator", needed: 100, hasNext: true}, {xp: 100, rank: "Operator", next: "Junior Sysadmin", needed: 150, hasNext: true}, {xp: 649, rank: "Sysadmin", next: "SRE", needed: 1, hasNext: true}, - {xp: 1100, rank: "Senior SRE", hasNext: false}, + {xp: 1100, rank: "Senior SRE", next: "Staff SRE", needed: 500, hasNext: true}, + {xp: 1599, rank: "Senior SRE", next: "Staff SRE", needed: 1, hasNext: true}, + {xp: 1600, rank: "Staff SRE", next: "Principal SRE", needed: 550, hasNext: true}, + {xp: 2150, rank: "Principal SRE", next: "Distinguished Engineer", needed: 550, hasNext: true}, + {xp: 2699, rank: "Principal SRE", next: "Distinguished Engineer", needed: 1, hasNext: true}, + {xp: 2700, rank: "Distinguished Engineer", hasNext: false}, + {xp: 5000, rank: "Distinguished Engineer", hasNext: false}, } for _, test := range tests { player := New("alex") diff --git a/internal/profile/rank.go b/internal/profile/rank.go index 4bb08f9..129d6f1 100644 --- a/internal/profile/rank.go +++ b/internal/profile/rank.go @@ -1,5 +1,9 @@ package profile +// rankThresholds span the whole catalog: Linux alone (1,755 XP) reaches +// Staff SRE, and the top rank needs most of the Docker track (3,085 XP in +// total) without requiring a hint-free run. Thresholds up to Senior SRE are +// unchanged from earlier releases, so no existing profile loses its rank. var rankThresholds = []struct { name string xp int @@ -10,6 +14,9 @@ var rankThresholds = []struct { {name: "Sysadmin", xp: 450}, {name: "SRE", xp: 650}, {name: "Senior SRE", xp: 1100}, + {name: "Staff SRE", xp: 1600}, + {name: "Principal SRE", xp: 2150}, + {name: "Distinguished Engineer", xp: 2700}, } func (p Profile) Level() int { return p.XP/100 + 1 } diff --git a/internal/sandbox/command_list.go b/internal/sandbox/command_list.go new file mode 100644 index 0000000..2add081 --- /dev/null +++ b/internal/sandbox/command_list.go @@ -0,0 +1,207 @@ +package sandbox + +import ( + "fmt" + "slices" + "strings" + "unicode" +) + +// commandListItem is one pipeline of a command list and the operator that +// joins it to the previous pipeline ("" for the first). +type commandListItem struct { + operator string + line string +} + +// splitCommandList splits line at unquoted ;, &&, and || into pipelines. +// Quotes, escapes, and comments follow the lexer, so `echo 'a; b'` stays one +// pipeline. As in sh, a trailing ; is allowed but a trailing && or || is not. +func splitCommandList(line string) ([]commandListItem, error) { + runes := []rune(line) + var items []commandListItem + operator := "" + start := 0 + var quote rune + escaped, tokenStarted := false, false + for index := 0; index < len(runes); index++ { + char := runes[index] + if escaped { + escaped = false + tokenStarted = true + continue + } + if quote == '\'' { + if char == quote { + quote = 0 + } + continue + } + if char == '\\' { + escaped = true + continue + } + if quote == '"' { + if char == quote { + quote = 0 + } + continue + } + if unicode.IsSpace(char) { + tokenStarted = false + continue + } + if char == '#' && !tokenStarted { + break + } + if char == '\'' || char == '"' { + quote = char + tokenStarted = true + continue + } + next := "" + switch { + case char == ';': + next = ";" + case char == '&' && index+1 < len(runes) && runes[index+1] == '&': + next = "&&" + case char == '|' && index+1 < len(runes) && runes[index+1] == '|': + next = "||" + } + if next == "" { + // A pipe or redirection ends a word, so a following # is a comment. + tokenStarted = !strings.ContainsRune("|<>", char) + continue + } + segment := string(runes[start:index]) + if strings.TrimSpace(segment) == "" { + return nil, fmt.Errorf("syntax error near unexpected %q", next) + } + items = append(items, commandListItem{operator: operator, line: segment}) + operator = next + index += len(next) - 1 + start = index + 1 + tokenStarted = false + } + if len(items) == 0 { + return []commandListItem{{line: line}}, nil + } + rest := string(runes[start:]) + if trimmed := strings.TrimSpace(rest); trimmed == "" || strings.HasPrefix(trimmed, "#") { + if operator != ";" { + return nil, fmt.Errorf("command list cannot end with %q", operator) + } + return items, nil + } + return append(items, commandListItem{operator: operator, line: rest}), nil +} + +// executeLine runs one input line: a single pipeline or a command list. Each +// pipeline expands variables and globs only when its turn comes, so +// `export X=1; echo $X` prints 1 and `cd dir && ls` lists dir. As in sh, +// a && b runs b only after a succeeds, a || b runs b only after a fails, and +// the two operators bind equally from left to right. +// +// For a list, the result's Transcript holds every output and handled error +// in order, a failed pipeline contributes no practiced commands, and the +// returned error is the last pipeline's failure, if any. +func (s *Sandbox) executeLine(line string, context *executionContext, allowInteractive bool) (Result, error) { + items, err := splitCommandList(line) + if err != nil { + return Result{}, err + } + if len(items) == 1 { + return s.executePipeline(items[0].line, context, allowInteractive) + } + if err := s.preflightCommandList(items, allowInteractive); err != nil { + return Result{}, err + } + + result := Result{list: true} + var output strings.Builder + transcriptSize := 0 + add := func(chunk OutputChunk) error { + if chunk.Text == "" { + return nil + } + if len(chunk.Text) > maxCommandOutputBytes-transcriptSize { + return commandOutputLimitError() + } + transcriptSize += len(chunk.Text) + result.Transcript = append(result.Transcript, chunk) + if !chunk.Error { + output.WriteString(chunk.Text) + } + return nil + } + // pending holds a failure until the next pipeline runs: then the failure + // was handled and is reported in order; otherwise it is the list's result. + var pending error + failed := false + for index, item := range items { + if index > 0 && (item.operator == "&&" && failed || item.operator == "||" && !failed) { + continue + } + if pending != nil { + if err := add(OutputChunk{Text: pending.Error(), Error: true}); err != nil { + return Result{}, err + } + pending = nil + } + commandsBefore, stderrBefore := len(context.commands), len(context.stderr) + pipeline, err := s.executePipeline(item.line, context, allowInteractive) + diagnostics := slices.Clone(context.stderr[stderrBefore:]) + context.stderr = context.stderr[:stderrBefore] + for _, message := range diagnostics { + if err := add(OutputChunk{Text: message, Error: true}); err != nil { + return Result{}, err + } + } + if err != nil { + context.commands = context.commands[:commandsBefore] + pending, failed = err, true + continue + } + if err := add(OutputChunk{Text: pipeline.Output}); err != nil { + return Result{}, err + } + failed = pipeline.statusFailed + } + result.Output = output.String() + result.statusFailed = failed && pending == nil + return result, pending +} + +// preflightCommandList checks every pipeline before the first one runs, so a +// syntax error late in a list changes nothing, as in sh. +func (s *Sandbox) preflightCommandList(items []commandListItem, allowInteractive bool) error { + for _, item := range items { + if err := validateShellSyntax(item.line); err != nil { + return err + } + tokens, err := lex(item.line, s.Env) + if err != nil { + return err + } + parsed, err := parseCommandLine(tokens) + if err != nil { + return err + } + if !allowInteractive { + if err := validateScriptCommands(parsed); err != nil { + return err + } + } + // The editor runs after Execute returns, so it cannot sit in a list. + for _, stage := range parsed.stages { + if stage.args[0].value != "vi" { + continue + } + if !allowInteractive { + return fmt.Errorf("vi: interactive commands are not supported in scripts") + } + return fmt.Errorf("vi: interactive commands cannot be combined with ;, &&, or ||") + } + } + return nil +} diff --git a/internal/sandbox/command_list_test.go b/internal/sandbox/command_list_test.go new file mode 100644 index 0000000..9b95118 --- /dev/null +++ b/internal/sandbox/command_list_test.go @@ -0,0 +1,215 @@ +package sandbox + +import ( + "slices" + "strings" + "testing" +) + +func out(text string) OutputChunk { return OutputChunk{Text: text} } + +func errText(text string) OutputChunk { return OutputChunk{Text: text, Error: true} } + +func TestCommandListsFollowShellControlFlow(t *testing.T) { + missing := "ls: /missing: no such file or directory" + tests := []struct { + name string + line string + wantOutput string + wantChunks []OutputChunk + wantCommands []string + }{ + {name: "sequence", line: "echo a; echo b", wantOutput: "a\nb\n", wantChunks: []OutputChunk{out("a\n"), out("b\n")}, wantCommands: []string{"echo", "echo"}}, + {name: "sequence continues after failure", line: "ls /missing; echo after", wantOutput: "after\n", wantChunks: []OutputChunk{errText(missing), out("after\n")}, wantCommands: []string{"echo"}}, + {name: "and runs after success", line: "echo first && echo second", wantOutput: "first\nsecond\n", wantChunks: []OutputChunk{out("first\n"), out("second\n")}, wantCommands: []string{"echo", "echo"}}, + {name: "and skips after failure", line: "ls /missing && echo never", wantChunks: []OutputChunk{errText(missing)}, wantCommands: nil}, + {name: "or runs after failure", line: "ls /missing || echo fallback", wantOutput: "fallback\n", wantChunks: []OutputChunk{errText(missing), out("fallback\n")}, wantCommands: []string{"echo"}}, + {name: "or skips after success", line: "echo ok || echo never", wantOutput: "ok\n", wantChunks: []OutputChunk{out("ok\n")}, wantCommands: []string{"echo"}}, + {name: "equal precedence left to right", line: "false && echo x || echo y", wantOutput: "y\n", wantChunks: []OutputChunk{out("y\n")}, wantCommands: []string{"false", "echo"}}, + {name: "skipped and keeps the earlier status", line: "ls /missing && echo x || echo recovered", wantOutput: "recovered\n", wantChunks: []OutputChunk{errText(missing), out("recovered\n")}, wantCommands: []string{"echo"}}, + {name: "silent false drives or", line: "false || echo fallback", wantOutput: "fallback\n", wantChunks: []OutputChunk{out("fallback\n")}, wantCommands: []string{"false", "echo"}}, + {name: "true drives and", line: "true && echo yes", wantOutput: "yes\n", wantChunks: []OutputChunk{out("yes\n")}, wantCommands: []string{"true", "echo"}}, + {name: "grep match status", line: "grep -q ERROR events.log && echo found", wantOutput: "found\n", wantChunks: []OutputChunk{out("found\n")}, wantCommands: []string{"grep", "echo"}}, + {name: "grep no match status", line: "grep -q PANIC events.log || echo clean", wantOutput: "clean\n", wantChunks: []OutputChunk{out("clean\n")}, wantCommands: []string{"grep", "echo"}}, + {name: "grep count still prints zero", line: "grep -c PANIC events.log || echo none", wantOutput: "0\nnone\n", wantChunks: []OutputChunk{out("0\n"), out("none\n")}, wantCommands: []string{"grep", "echo"}}, + {name: "pipeline status is its last stage", line: "grep PANIC events.log | wc -l && echo counted", wantOutput: "0\ncounted\n", wantChunks: []OutputChunk{out("0\n"), out("counted\n")}, wantCommands: []string{"grep", "wc", "echo"}}, + {name: "trailing semicolon", line: "echo done;", wantOutput: "done\n", wantCommands: []string{"echo"}}, + {name: "trailing comment after semicolon", line: "echo a; echo b; # ; && ||", wantOutput: "a\nb\n", wantChunks: []OutputChunk{out("a\n"), out("b\n")}, wantCommands: []string{"echo", "echo"}}, + {name: "quoted and escaped operators are data", line: `echo 'a; b' "c && d" e\;f g\|\|h`, wantOutput: "a; b c && d e;f g||h\n", wantCommands: []string{"echo"}}, + {name: "operators inside a comment", line: "echo a # ; echo b", wantOutput: "a\n", wantCommands: []string{"echo"}}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + box := testSandbox(t) + result, err := box.Execute(test.line) + if err != nil { + t.Fatalf("Execute(%q) error = %v", test.line, err) + } + if result.Output != test.wantOutput { + t.Errorf("Output = %q, want %q", result.Output, test.wantOutput) + } + if !slices.Equal(result.Transcript, test.wantChunks) { + t.Errorf("Transcript = %+v, want %+v", result.Transcript, test.wantChunks) + } + if !slices.Equal(result.Commands, test.wantCommands) { + t.Errorf("Commands = %q, want %q", result.Commands, test.wantCommands) + } + }) + } +} + +func TestCommandListsExpandEachPipelineWhenItRuns(t *testing.T) { + box := testSandbox(t) + steps := []struct{ line, want string }{ + {line: `export STAGE=deploy; echo "$STAGE"`, want: "deploy\n"}, + {line: "cd /out && pwd", want: "/out\n"}, + {line: "touch a.txt b.txt; echo *.txt", want: "a.txt b.txt\n"}, + {line: "cd /missing && touch never.txt; pwd", want: "/out\n"}, + } + for _, step := range steps { + result, err := box.Execute(step.line) + if err != nil { + t.Fatalf("Execute(%q) error = %v", step.line, err) + } + if result.Output != step.want { + t.Errorf("Execute(%q) output = %q, want %q", step.line, result.Output, step.want) + } + } + if box.FS.Exists("/out/never.txt") { + t.Error("&& ran a command after cd failed") + } + if len(box.History) != len(steps) { + t.Errorf("history = %q, want one entry per line", box.History) + } +} + +func TestSingleCommandsKeepTheirFailureContract(t *testing.T) { + box := testSandbox(t) + for _, line := range []string{"grep PANIC events.log", "false", "true"} { + result, err := box.Execute(line) + if err != nil || result.Output != "" || result.Transcript != nil { + t.Errorf("Execute(%q) = %#v, %v; want silent success without a transcript", line, result, err) + } + } + result, err := box.Execute("ls /missing") + if err == nil || result.Transcript != nil { + t.Fatalf("single failure = %#v, %v; want a plain error", result, err) + } +} + +func TestCommandListsBoundOutputAndDispatch(t *testing.T) { + box := testSandbox(t) + if err := box.FS.WriteFile("/work/big.txt", strings.Repeat("x", maxCommandOutputBytes/2+1), 0o644); err != nil { + t.Fatal(err) + } + if _, err := box.Execute("cat big.txt; cat big.txt"); err == nil || !strings.Contains(err.Error(), "output") { + t.Fatalf("combined list output error = %v, want the output limit", err) + } + + result, err := box.Execute(strings.Repeat("true; ", maxExecutionDispatchSteps+10)) + if err != nil { + t.Fatalf("long list error = %v", err) + } + if len(result.Commands) != maxExecutionDispatchSteps { + t.Errorf("dispatched commands = %d, want the %d-step budget", len(result.Commands), maxExecutionDispatchSteps) + } + last := result.Transcript[len(result.Transcript)-1] + if !last.Error || !strings.Contains(last.Text, "dispatch limit") { + t.Errorf("last transcript entry = %#v, want the dispatch limit", last) + } +} + +func TestScriptsSupportCommandLists(t *testing.T) { + t.Run("handled failures are reported with their location", func(t *testing.T) { + box := testSandbox(t) + writeTestScript(t, box, "report.sh", "echo start\ncat missing.log || echo fallback\necho done\n", 0o644) + result, err := box.Execute("sh report.sh") + if err != nil { + t.Fatalf("Execute() error = %v", err) + } + want := []OutputChunk{ + errText("/work/report.sh:2: cat: /work/missing.log: no such file"), + out("start\nfallback\ndone\n"), + } + if !slices.Equal(result.Transcript, want) { + t.Fatalf("Transcript = %+v, want %+v", result.Transcript, want) + } + if result.Output != "start\nfallback\ndone\n" { + t.Errorf("Output = %q", result.Output) + } + }) + + t.Run("an unhandled failure still stops the script", func(t *testing.T) { + box := testSandbox(t) + writeTestScript(t, box, "stop.sh", "echo start && cat missing.log\necho never\n", 0o644) + _, err := box.Execute("sh stop.sh") + if err == nil || !strings.Contains(err.Error(), "/work/stop.sh:1: cat:") { + t.Fatalf("Execute() error = %v, want the located cat failure", err) + } + }) + + t.Run("exit status is the last command's", func(t *testing.T) { + box := testSandbox(t) + writeTestScript(t, box, "check.sh", "echo checking\ngrep -q PANIC events.log\n", 0o644) + result, err := box.Execute("sh check.sh || echo clean") + if err != nil || result.Output != "checking\nclean\n" { + t.Fatalf("Execute() = %q, %v; want the script's failing status to trigger ||", result.Output, err) + } + }) + + t.Run("nested scripts chain locations", func(t *testing.T) { + box := testSandbox(t) + writeTestScript(t, box, "inner.sh", "cat missing.log || echo inner\n", 0o644) + writeTestScript(t, box, "outer.sh", "sh inner.sh\necho outer\n", 0o644) + result, err := box.Execute("sh outer.sh") + if err != nil { + t.Fatalf("Execute() error = %v", err) + } + if len(result.Transcript) == 0 || result.Transcript[0].Text != "/work/outer.sh:1: /work/inner.sh:1: cat: /work/missing.log: no such file" { + t.Fatalf("Transcript = %+v, want the outer and inner locations", result.Transcript) + } + }) + + t.Run("a single failing command with diagnostics earns no practice", func(t *testing.T) { + box := testSandbox(t) + writeTestScript(t, box, "partial.sh", "cat missing.log || echo fallback\ncat missing.log\n", 0o644) + result, err := box.Execute("sh partial.sh") + if err != nil { + t.Fatalf("Execute() error = %v", err) + } + if result.Commands != nil { + t.Errorf("Commands = %q, want none for a failed command", result.Commands) + } + last := result.Transcript[len(result.Transcript)-1] + if !last.Error || !strings.Contains(last.Text, "partial.sh:2:") { + t.Errorf("last transcript entry = %#v, want the script failure", last) + } + }) +} + +func TestSplitCommandList(t *testing.T) { + items, err := splitCommandList(`a 'b;c' && d "e||f" || g\;h; i # j && k`) + if err != nil { + t.Fatal(err) + } + want := []commandListItem{ + {operator: "", line: `a 'b;c' `}, + {operator: "&&", line: ` d "e||f" `}, + {operator: "||", line: ` g\;h`}, + {operator: ";", line: ` i # j && k`}, + } + if !slices.Equal(items, want) { + t.Fatalf("splitCommandList() = %q, want %q", items, want) + } + for line, wantErr := range map[string]string{ + "; echo": `unexpected ";"`, + "echo a ;; b": `unexpected ";"`, + "echo a && || b": `unexpected "||"`, + "echo a &&": `cannot end with "&&"`, + "echo a || # x": `cannot end with "||"`, + } { + if _, err := splitCommandList(line); err == nil || !strings.Contains(err.Error(), wantErr) { + t.Errorf("splitCommandList(%q) error = %v, want %q", line, err, wantErr) + } + } +} diff --git a/internal/sandbox/commands_columns.go b/internal/sandbox/commands_columns.go index 80532b2..11286a7 100644 --- a/internal/sandbox/commands_columns.go +++ b/internal/sandbox/commands_columns.go @@ -5,6 +5,7 @@ import ( "regexp" "strconv" "strings" + "unicode/utf8" ) var awkPrintField = regexp.MustCompile(`^\{\s*print\s+\$([0-9]+)\s*\}$`) @@ -30,7 +31,8 @@ func (s *Sandbox) cmdAwk(args []string, stdin string) (string, error) { func (s *Sandbox) cmdCut(args []string, stdin string) (string, error) { delimiter := "\t" - field := 0 + fieldList := "" + onlyDelimited := false var files []string for index := 0; index < len(args); index++ { switch { @@ -44,12 +46,14 @@ func (s *Sandbox) cmdCut(args []string, stdin string) (string, error) { delimiter = strings.TrimPrefix(args[index], "-d") case args[index] == "-f": if index+1 >= len(args) { - return "", fmt.Errorf("-f requires a field") + return "", fmt.Errorf("-f requires a field list") } index++ - field, _ = strconv.Atoi(args[index]) + fieldList = args[index] case strings.HasPrefix(args[index], "-f"): - field, _ = strconv.Atoi(strings.TrimPrefix(args[index], "-f")) + fieldList = strings.TrimPrefix(args[index], "-f") + case args[index] == "-s": + onlyDelimited = true default: if strings.HasPrefix(args[index], "-") { return "", fmt.Errorf("unknown option %s", args[index]) @@ -57,17 +61,106 @@ func (s *Sandbox) cmdCut(args []string, stdin string) (string, error) { files = append(files, args[index]) } } - if field < 1 { - return "", fmt.Errorf("select a positive field with -f") + if fieldList == "" { + return "", fmt.Errorf("select fields with -f, such as -f 2, -f 1,3, or -f 2-") } - if delimiter == "" { - return "", fmt.Errorf("delimiter cannot be empty") + fields, err := parseFieldList(fieldList) + if err != nil { + return "", err + } + if utf8.RuneCountInString(delimiter) != 1 { + return "", fmt.Errorf("the delimiter must be a single character") } inputs, err := s.readInputs(files, stdin) if err != nil { return "", err } - return selectField(inputs, field, func(line string) []string { return strings.Split(line, delimiter) }) + var output commandOutputBuffer + for _, input := range inputs { + for _, line := range textLines(input.text) { + // Like GNU cut, a line without the delimiter passes through whole + // unless -s asks for delimited lines only. + if !strings.Contains(line, delimiter) { + if !onlyDelimited { + output.WriteString(line + "\n") + } + continue + } + var selected []string + for position, field := range strings.Split(line, delimiter) { + if fields.contains(position + 1) { + selected = append(selected, field) + } + } + output.WriteString(strings.Join(selected, delimiter) + "\n") + } + } + return output.Result() +} + +// fieldRange is an inclusive range of 1-based fields; high 0 is open-ended. +type fieldRange struct{ low, high int } + +type fieldList []fieldRange + +func (l fieldList) contains(field int) bool { + for _, item := range l { + if field >= item.low && (item.high == 0 || field <= item.high) { + return true + } + } + return false +} + +// parseFieldList parses cut's LIST: comma-separated N, N-M, N-, or -M. +// Ranges are kept as bounds, so -f 1-999999999 allocates nothing per field. +func parseFieldList(list string) (fieldList, error) { + invalid := fmt.Errorf("invalid field list %q; use N, N-M, N-, or -M separated by commas", list) + number := func(value string) (int, error) { + if value == "" || strings.Trim(value, "0123456789") != "" { + return 0, invalid + } + parsed, err := strconv.Atoi(value) + if err != nil { + return 0, invalid + } + if parsed < 1 { + return 0, fmt.Errorf("fields are numbered from 1") + } + return parsed, nil + } + var fields fieldList + for _, item := range strings.Split(list, ",") { + lowText, highText, isRange := strings.Cut(item, "-") + if !isRange { + field, err := number(item) + if err != nil { + return nil, err + } + fields = append(fields, fieldRange{low: field, high: field}) + continue + } + if lowText == "" && highText == "" { + return nil, invalid + } + bounds := fieldRange{low: 1} + var err error + if lowText != "" { + if bounds.low, err = number(lowText); err != nil { + return nil, err + } + } + if highText != "" { + if bounds.high, err = number(highText); err != nil { + return nil, err + } + if bounds.high < bounds.low { + return nil, fmt.Errorf("invalid decreasing range %q in field list", item) + } + } + fields = append(fields, bounds) + } + return fields, nil } func selectField(inputs []namedText, field int, split func(string) []string) (string, error) { diff --git a/internal/sandbox/commands_files.go b/internal/sandbox/commands_files.go index fb16065..2ff1720 100644 --- a/internal/sandbox/commands_files.go +++ b/internal/sandbox/commands_files.go @@ -3,6 +3,7 @@ package sandbox import ( "fmt" "path" + "sort" "strconv" "strings" ) @@ -63,38 +64,56 @@ func (s *Sandbox) cmdLS(args []string) (string, error) { if len(names) == 0 { names = []string{"."} } - var output commandOutputBuffer - for index, name := range names { - resolved := s.Resolve(name) - entry, exists := s.FS.Entry(resolved) + // Like GNU ls, list file operands first under the names given, then each + // directory's contents, with a header when there are several operands. + var files, directories []string + for _, name := range names { + entry, exists := s.FS.Entry(s.Resolve(name)) if !exists { return "", fmt.Errorf("%s: no such file or directory", name) } - if len(names) > 1 && entry.Kind == Directory { - if index > 0 { - output.WriteByte('\n') - } + if entry.Kind == Directory { + directories = append(directories, name) + } else { + files = append(files, name) + } + } + sort.Strings(files) + sort.Strings(directories) + + var output commandOutputBuffer + writeEntry := func(itemPath, display string) { + item, _ := s.FS.Entry(itemPath) + if !long { + output.WriteString(display + "\n") + return + } + kind := '-' + if item.Kind == Directory { + kind = 'd' + } + output.WriteString(fmt.Sprintf("%c%s %-8s %6d %s\n", kind, permissionString(item.Mode), item.Owner, len(item.Content), display)) + } + for _, name := range files { + writeEntry(s.Resolve(name), name) + } + for index, name := range directories { + if len(files) > 0 || index > 0 { + output.WriteByte('\n') + } + if len(names) > 1 { output.WriteString(name + ":\n") } - items := []string{resolved} - if entry.Kind == Directory { - children, _ := s.FS.Children(resolved) - items = children + resolved := s.Resolve(name) + if showAll { + writeEntry(resolved, ".") + writeEntry(path.Dir(resolved), "..") } - for _, itemPath := range items { - item, _ := s.FS.Entry(itemPath) - base := path.Base(itemPath) - if !showAll && strings.HasPrefix(base, ".") { - continue - } - if long { - kind := '-' - if item.Kind == Directory { - kind = 'd' - } - output.WriteString(fmt.Sprintf("%c%s %-8s %6d %s\n", kind, permissionString(item.Mode), item.Owner, len(item.Content), base)) - } else { - output.WriteString(base + "\n") + children, _ := s.FS.Children(resolved) + for _, child := range children { + base := path.Base(child) + if showAll || !strings.HasPrefix(base, ".") { + writeEntry(child, base) } } } diff --git a/internal/sandbox/commands_find.go b/internal/sandbox/commands_find.go index 0ce0f5c..ee950e5 100644 --- a/internal/sandbox/commands_find.go +++ b/internal/sandbox/commands_find.go @@ -1,6 +1,7 @@ package sandbox import ( + "errors" "fmt" "path" "slices" @@ -18,31 +19,55 @@ func (s *Sandbox) cmdFind(context *executionContext, args []string) (string, err roots = []string{"."} } - namePattern := "" - caseInsensitive := false - entryType := "" + // Tests combine with an implicit AND, as in find: every -name, -iname, + // and -type must hold for an entry to be printed or passed to -exec. + var tests []func(candidate string, entry *Entry) (bool, error) var execArgs []string for index < len(args) { switch args[index] { case "-name", "-iname": - caseInsensitive = args[index] == "-iname" + caseInsensitive := args[index] == "-iname" if index+1 >= len(args) { return "", fmt.Errorf("%s requires a pattern", args[index]) } index++ - namePattern = args[index] + pattern := args[index] + if caseInsensitive { + pattern = strings.ToLower(pattern) + } + tests = append(tests, func(candidate string, _ *Entry) (bool, error) { + name := path.Base(candidate) + if caseInsensitive { + name = strings.ToLower(name) + } + matched, err := matchShellPattern(pattern, name) + if err != nil { + return false, fmt.Errorf("invalid name pattern: %w", err) + } + return matched, nil + }) case "-type": if index+1 >= len(args) { return "", fmt.Errorf("-type requires f or d") } index++ - entryType = args[index] - if entryType != "f" && entryType != "d" { - return "", fmt.Errorf("unsupported type %q", entryType) + kind := Regular + switch args[index] { + case "f": + case "d": + kind = Directory + default: + return "", fmt.Errorf("unsupported type %q", args[index]) } - case "-print": - // Printing is the default action. + tests = append(tests, func(_ string, entry *Entry) (bool, error) { + return entry.Kind == kind, nil + }) + case "-a", "-and", "-print": + // AND is already implicit, and printing is the default action. case "-exec": + if execArgs != nil { + return "", fmt.Errorf("this lab supports one -exec action per find") + } index++ start := index for index < len(args) && args[index] != ";" { @@ -56,7 +81,7 @@ func (s *Sandbox) cmdFind(context *executionContext, args []string) (string, err return "", fmt.Errorf("-exec requires a command") } default: - return "", fmt.Errorf("unsupported expression %s", args[index]) + return "", fmt.Errorf("unsupported expression %s; tests combine with an implicit AND, and -o, !, and parentheses are not supported", args[index]) } index++ } @@ -68,25 +93,16 @@ func (s *Sandbox) cmdFind(context *executionContext, args []string) (string, err if err != nil { return "", err } + candidates: for _, candidate := range items { entry, _ := s.FS.Entry(candidate) - if entryType == "f" && entry.Kind != Regular { - continue - } - if entryType == "d" && entry.Kind != Directory { - continue - } - if namePattern != "" { - candidateName, pattern := path.Base(candidate), namePattern - if caseInsensitive { - candidateName, pattern = strings.ToLower(candidateName), strings.ToLower(pattern) - } - matched, err := matchShellPattern(pattern, candidateName) + for _, test := range tests { + matched, err := test(candidate, entry) if err != nil { - return "", fmt.Errorf("invalid name pattern: %w", err) + return "", err } if !matched { - continue + continue candidates } } display := displayFindPath(root, rootAbs, candidate) @@ -103,8 +119,10 @@ func (s *Sandbox) cmdFind(context *executionContext, args []string) (string, err command[position] = display } } + // A failing status (grep without a match) only makes this -exec + // test false; find continues with the next entry. result, err := s.run(context, command, "") - if err != nil { + if err != nil && !errors.Is(err, errFailureStatus) { return "", fmt.Errorf("-exec %s: %w", command[0], err) } output.WriteString(result) diff --git a/internal/sandbox/commands_gnu_fidelity_test.go b/internal/sandbox/commands_gnu_fidelity_test.go index d6a7e26..76459d1 100644 --- a/internal/sandbox/commands_gnu_fidelity_test.go +++ b/internal/sandbox/commands_gnu_fidelity_test.go @@ -262,3 +262,121 @@ func TestTranslateBasicRegex(t *testing.T) { } } } + +func TestOutputRedirectionEmptiesItsTargetBeforeTheCommandRuns(t *testing.T) { + runGNUFidelityCases(t, []gnuFidelityCase{ + {name: "sort onto its input", setup: []string{fourLines, "sort /out/four.txt > /out/four.txt"}, line: "wc -c /out/four.txt", want: "0 /out/four.txt\n"}, + {name: "input redirection from the target", setup: []string{fourLines, "cat < /out/four.txt > /out/four.txt"}, line: "wc -c /out/four.txt", want: "0 /out/four.txt\n"}, + {name: "append keeps its input", setup: []string{fourLines, "head -n 1 /out/four.txt >> /out/four.txt"}, line: "tail -n 2 /out/four.txt", want: "l4\nh1\n"}, + {name: "directory target fails before running", line: "touch /out/new.txt > /out", wantErr: "redirect"}, + }) + + box := testSandbox(t) + if _, err := box.Execute("touch /out/new.txt > /out"); err == nil || box.FS.Exists("/out/new.txt") { + t.Fatalf("a rejected redirection ran its command: err %v", err) + } +} + +func TestFailedStageRestoresItsRedirectTarget(t *testing.T) { + box := testSandbox(t) + if err := box.FS.WriteFile("/out/report", "keep\n", 0o640); err != nil { + t.Fatal(err) + } + if err := box.FS.Chown("/out/report", "reviewer"); err != nil { + t.Fatal(err) + } + for _, line := range []string{ + "cat missing.log > /out/report", + "cat missing.log > /out/created", + "grep ERROR < missing.log > /out/report", + } { + if _, err := box.Execute(line); err == nil { + t.Fatalf("Execute(%q) unexpectedly succeeded", line) + } + } + entry, exists := box.FS.Entry("/out/report") + if !exists || entry.Content != "keep\n" || entry.Mode != 0o640 || entry.Owner != "reviewer" { + t.Errorf("failed command changed its destination: %#v", entry) + } + if box.FS.Exists("/out/created") { + t.Error("failed command left a destination it created") + } + + // Earlier stages that finished keep their effects, as before. + if _, err := box.Execute("echo kept > /out/first.txt | cat missing.log"); err == nil { + t.Fatal("pipeline unexpectedly succeeded") + } + if content, _ := box.FS.ReadFile("/out/first.txt"); content != "kept\n" { + t.Errorf("completed stage output = %q, want kept", content) + } + + // Archive metadata comes back with a restored archive. + if _, err := box.Execute("tar -cf /out/logs.tar events.log"); err != nil { + t.Fatal(err) + } + if _, err := box.Execute("cat missing.log > /out/logs.tar"); err == nil { + t.Fatal("failing redirect unexpectedly succeeded") + } + if result, err := box.Execute("tar -tf /out/logs.tar"); err != nil || !strings.Contains(result.Output, "events.log") { + t.Errorf("restored archive listing = %q, %v", result.Output, err) + } +} + +func TestListingFollowsGNULayout(t *testing.T) { + tree := []string{"mkdir -p /out/d1/d2", "touch /out/d1/f /out/d1/g /out/d1/.env /out/top /out/.hidden"} + runGNUFidelityCases(t, []gnuFidelityCase{ + {name: "file operand keeps its path", setup: tree, line: "ls /out/d1/f", want: "/out/d1/f\n"}, + {name: "files before directories", setup: tree, line: "ls /out/d1 /out/top", want: "/out/top\n\n/out/d1:\nd2\nf\ng\n"}, + {name: "directories sorted with headers", setup: tree, line: "ls /out/d1/d2 /out/d1", want: "/out/d1:\nd2\nf\ng\n\n/out/d1/d2:\n"}, + {name: "single directory has no header", setup: tree, line: "ls /out/d1", want: "d2\nf\ng\n"}, + {name: "all shows dot entries", setup: tree, line: "ls -a /out/d1", want: ".\n..\n.env\nd2\nf\ng\n"}, + {name: "named hidden file is shown", setup: tree, line: "ls /out/.hidden", want: "/out/.hidden\n"}, + {name: "long file operand", setup: tree, line: "ls -l /out/top", want: "-rw-r--r-- operator 0 /out/top\n"}, + }) +} + +func TestGlobsSkipHiddenNamesUnlessTheDotIsExplicit(t *testing.T) { + tree := []string{"mkdir -p /out/d", "touch /out/a /out/.env /out/.profile /out/d/b /out/d/.secret"} + runGNUFidelityCases(t, []gnuFidelityCase{ + {name: "star", setup: tree, line: "echo /out/*", want: "/out/a /out/d\n"}, + {name: "dot star", setup: tree, line: "echo /out/.*", want: "/out/.env /out/.profile\n"}, + {name: "dot prefix", setup: tree, line: "echo /out/.e*", want: "/out/.env\n"}, + {name: "nested star", setup: tree, line: "echo /out/*/*", want: "/out/d/b\n"}, + {name: "question mark", setup: tree, line: "echo /out/?", want: "/out/a /out/d\n"}, + {name: "rm star keeps hidden files", setup: append(tree, "rm /out/a"), line: "ls -a /out", want: ".\n..\n.env\n.profile\nd\n"}, + {name: "find still matches hidden names", setup: tree, line: "find /out/d -name '*' -type f", want: "/out/d/.secret\n/out/d/b\n"}, + }) +} + +func TestCutFieldListsAndUndelimitedLines(t *testing.T) { + rows := `printf 'a:b:c:d\nplain\n' > /out/rows.txt` + runGNUFidelityCases(t, []gnuFidelityCase{ + {name: "list", setup: []string{rows}, line: "cut -d: -f1,3 /out/rows.txt", want: "a:c\nplain\n"}, + {name: "open range", setup: []string{rows}, line: "cut -d : -f 3- /out/rows.txt", want: "c:d\nplain\n"}, + {name: "leading range", setup: []string{rows}, line: "cut -d: -f-2 /out/rows.txt", want: "a:b\nplain\n"}, + {name: "overlapping ranges keep input order", setup: []string{rows}, line: "cut -d: -f4,1-2,2 /out/rows.txt", want: "a:b:d\nplain\n"}, + {name: "fields past the end", setup: []string{rows}, line: "cut -d: -f9 /out/rows.txt", want: "\nplain\n"}, + {name: "huge range", setup: []string{rows}, line: "cut -d: -f2-999999999 /out/rows.txt", want: "b:c:d\nplain\n"}, + {name: "only delimited", setup: []string{rows}, line: "cut -s -d: -f2 /out/rows.txt", want: "b\n"}, + {name: "tab default", line: `printf 'x\ty\n' | cut -f2`, want: "y\n"}, + {name: "multi-character delimiter", setup: []string{rows}, line: "cut -d ', ' -f1 /out/rows.txt", wantErr: "single character"}, + {name: "zero field", setup: []string{rows}, line: "cut -d: -f0 /out/rows.txt", wantErr: "numbered from 1"}, + {name: "decreasing range", setup: []string{rows}, line: "cut -d: -f3-1 /out/rows.txt", wantErr: "decreasing range"}, + {name: "malformed list", setup: []string{rows}, line: "cut -d: -f1,x /out/rows.txt", wantErr: "invalid field list"}, + {name: "missing list", setup: []string{rows}, line: "cut -d: /out/rows.txt", wantErr: "select fields with -f"}, + }) +} + +func TestFindCombinesTestsWithAnd(t *testing.T) { + tree := []string{"mkdir -p /out/logs", "touch /out/logs/app.log /out/logs/api.txt /out/access.log"} + runGNUFidelityCases(t, []gnuFidelityCase{ + {name: "two names", setup: tree, line: "find /out -name 'a*' -name '*.log'", want: "/out/access.log\n/out/logs/app.log\n"}, + {name: "contradictory types", setup: tree, line: "find /out -type f -type d", want: ""}, + {name: "explicit and", setup: tree, line: "find /out -type f -a -name '*.txt'", want: "/out/logs/api.txt\n"}, + {name: "name and iname", setup: tree, line: "find /out -iname 'API*' -name '*.txt'", want: "/out/logs/api.txt\n"}, + {name: "exec only on matches", setup: tree, line: `find /out -name '*.log' -name 'app*' -exec basename {} \;`, want: "app.log\n"}, + {name: "exec status does not stop find", line: `find . -type f -exec grep -l ERROR {} \;`, want: "./events.log\n"}, + {name: "or is rejected", setup: tree, line: "find /out -name a -o -name b", wantErr: "implicit AND"}, + {name: "second exec is rejected", setup: tree, line: `find /out -exec basename {} \; -exec basename {} \;`, wantErr: "one -exec"}, + }) +} diff --git a/internal/sandbox/commands_grep.go b/internal/sandbox/commands_grep.go index 32def70..f3d2981 100644 --- a/internal/sandbox/commands_grep.go +++ b/internal/sandbox/commands_grep.go @@ -9,7 +9,7 @@ import ( func (s *Sandbox) cmdGrep(args []string, stdin string) (string, error) { recursive, namesOnly, lineNumbers := false, false, false - insensitive, invert, fixed, countOnly, wholeWord, extended := false, false, false, false, false, false + insensitive, invert, fixed, countOnly, wholeWord, extended, quiet := false, false, false, false, false, false, false var operands []string optionsDone := false for _, arg := range args { @@ -38,6 +38,8 @@ func (s *Sandbox) cmdGrep(args []string, stdin string) (string, error) { wholeWord = true case 'E': extended = true + case 'q': + quiet = true default: return "", fmt.Errorf("unknown option -%c", option) } @@ -107,6 +109,7 @@ func (s *Sandbox) cmdGrep(args []string, stdin string) (string, error) { showNames := len(inputs) > 1 || recursive var output commandOutputBuffer + selectedAny := false for _, input := range inputs { matchedFile := false matchCount := 0 @@ -119,6 +122,10 @@ func (s *Sandbox) cmdGrep(args []string, stdin string) (string, error) { continue } matchCount++ + selectedAny = true + if quiet { + break + } if namesOnly { if input.name != "" && !matchedFile { output.WriteString(input.name + "\n") @@ -137,12 +144,20 @@ func (s *Sandbox) cmdGrep(args []string, stdin string) (string, error) { } output.WriteString(line + "\n") } - if countOnly && !namesOnly { + if quiet && selectedAny { + return "", nil + } + if countOnly && !namesOnly && !quiet { if showNames && input.name != "" { output.WriteString(input.name + ":") } output.WriteString(strconv.Itoa(matchCount) + "\n") } } - return output.Result() + result, err := output.Result() + if err == nil && !selectedAny { + // Like grep's exit status 1: nothing matched, which && and || observe. + return result, errFailureStatus + } + return result, err } diff --git a/internal/sandbox/commands_script.go b/internal/sandbox/commands_script.go index fea172c..b1bd7d2 100644 --- a/internal/sandbox/commands_script.go +++ b/internal/sandbox/commands_script.go @@ -95,6 +95,7 @@ func (s *Sandbox) executeScript(context *executionContext, scriptPath string, re }() var output strings.Builder + statusFailed := false for index, rawLine := range lines { lineNumber := index + 1 line := strings.TrimSuffix(rawLine, "\r") @@ -105,7 +106,21 @@ func (s *Sandbox) executeScript(context *executionContext, scriptPath string, re if trimmed == "" || strings.HasPrefix(trimmed, "#") { continue } + stderrBefore := len(context.stderr) result, err := s.executeLine(line, context, false) + // Failures that a command list handled (`a || b`), on this line or in a + // nested script, still reach the caller's terminal ahead of the + // script's output, located like the errors that stop a script. + diagnostics := slices.Clone(context.stderr[stderrBefore:]) + context.stderr = context.stderr[:stderrBefore] + for _, chunk := range result.Transcript { + if chunk.Error { + diagnostics = append(diagnostics, chunk.Text) + } + } + for _, message := range diagnostics { + context.stderr = append(context.stderr, fmt.Sprintf("%s:%d: %s", scriptPath, lineNumber, message)) + } if err != nil { return "", fmt.Errorf("%s:%d: %w", scriptPath, lineNumber, err) } @@ -113,6 +128,11 @@ func (s *Sandbox) executeScript(context *executionContext, scriptPath string, re return "", fmt.Errorf("%s:%d: script output exceeds the %d KiB limit", scriptPath, lineNumber, maxScriptOutputBytes/1024) } output.WriteString(result.Output) + statusFailed = result.statusFailed + } + if statusFailed { + // Like sh, a script's exit status is that of its last command. + return output.String(), errFailureStatus } return output.String(), nil } diff --git a/internal/sandbox/commands_script_test.go b/internal/sandbox/commands_script_test.go index 9faa321..af82f76 100644 --- a/internal/sandbox/commands_script_test.go +++ b/internal/sandbox/commands_script_test.go @@ -321,9 +321,9 @@ func TestShRejectsUnsupportedShellLanguage(t *testing.T) { line string want string }{ - {name: "semicolon", line: "echo one; echo two", want: "control syntax"}, - {name: "and", line: "echo one && echo two", want: "control syntax"}, - {name: "or", line: "echo one || echo two", want: "operator ||"}, + {name: "background job", line: "echo one &", want: "control syntax"}, + {name: "empty list item", line: "echo one ;; echo two", want: "syntax error near"}, + {name: "dangling and", line: "echo one &&", want: `cannot end with "&&"`}, {name: "parentheses", line: "(echo one)", want: "control syntax"}, {name: "backticks", line: "echo `pwd`", want: "command substitution"}, {name: "substitution", line: "echo $(pwd)", want: "command substitution"}, diff --git a/internal/sandbox/dispatch.go b/internal/sandbox/dispatch.go index dd19cb8..b0bcfd5 100644 --- a/internal/sandbox/dispatch.go +++ b/internal/sandbox/dispatch.go @@ -1,6 +1,7 @@ package sandbox import ( + "errors" "fmt" ) @@ -13,7 +14,7 @@ func (s *Sandbox) run(context *executionContext, args []string, stdin string) (o return "", fmt.Errorf("command dispatch limit of %d exceeded", maxExecutionDispatchSteps) } defer func() { - if err == nil && len(output) > maxCommandOutputBytes { + if (err == nil || errors.Is(err, errFailureStatus)) && len(output) > maxCommandOutputBytes { output = "" err = commandOutputLimitError() } @@ -30,6 +31,10 @@ func (s *Sandbox) run(context *executionContext, args []string, stdin string) (o } context.commands = append(context.commands, args[0]) switch args[0] { + case "true": + return "", nil + case "false": + return "", errFailureStatus case "pwd": return s.cmdPwd(args[1:]) case "cd": diff --git a/internal/sandbox/filesystem_queries.go b/internal/sandbox/filesystem_queries.go index 4b4ae99..608bca6 100644 --- a/internal/sandbox/filesystem_queries.go +++ b/internal/sandbox/filesystem_queries.go @@ -63,7 +63,7 @@ func (f *FileSystem) Glob(cwd, pattern string) []string { matches := make([]string, 0) for candidate := range f.entries { matched, err := matchShellPattern(absPattern, candidate) - if err == nil && matched { + if err == nil && matched && !hiddenFromGlob(absPattern, candidate) { if strings.HasPrefix(pattern, "/") { matches = append(matches, candidate) } else { @@ -79,6 +79,23 @@ func (f *FileSystem) Glob(cwd, pattern string) []string { return matches } +// hiddenFromGlob reports whether candidate has a dot-prefixed component that +// the matching pattern component does not spell out. As in sh, `*` skips +// .env while `.*` and `.e*` match it. +func hiddenFromGlob(pattern, candidate string) bool { + patternParts, candidateParts := strings.Split(pattern, "/"), strings.Split(candidate, "/") + if len(patternParts) != len(candidateParts) { + return false + } + for index, part := range candidateParts { + explicit := strings.HasPrefix(patternParts[index], ".") || strings.HasPrefix(patternParts[index], `\.`) + if strings.HasPrefix(part, ".") && !explicit { + return true + } + } + return false +} + // matchShellPattern is path.Match with the shell's [!...] negated bracket // spelling, which path.Match only understands as [^...]. func matchShellPattern(pattern, name string) (bool, error) { diff --git a/internal/sandbox/manuals.go b/internal/sandbox/manuals.go index ec42d03..2c7b013 100644 --- a/internal/sandbox/manuals.go +++ b/internal/sandbox/manuals.go @@ -19,7 +19,8 @@ func shellHelp(args []string) (string, error) { commands := CommandNames() return "Available lab commands:\n " + strings.Join(commands, " ") + "\n\nShell features: pipelines (|), input (<), output (>), and append (>>) redirection." + - "\nEnter one command per line; ;, &&, ||, &, 2>, and $(...) are not supported." + + "\nCommand lists: a; b runs both, a && b runs b only if a succeeds, and a || b only if a fails." + + "\nNot supported: background jobs (&), 2> and other descriptor redirection, and $(...) substitution." + fmt.Sprintf("\nSandbox limits: %d KiB per command line; %d KiB expanded tokens; %d expanded arguments; %d pipeline stages; %d command dispatches; %d MiB per file and command output; %d MiB filesystem content and %d MiB archive payload; %d filesystem entries and %d archive entries.", maxCommandLineBytes/1024, maxExpandedTokenBytes/1024, maxExpandedArguments, maxPipelineStages, maxExecutionDispatchSteps, maxVirtualFileBytes/(1024*1024), maxVirtualFileSystemBytes/(1024*1024), maxVirtualArchiveBytes/(1024*1024), maxVirtualEntries, maxVirtualArchiveEntries) + @@ -42,14 +43,15 @@ var commandManuals = map[string]string{ "chown": "chown OWNER FILE... — change a file owner; OWNER is limited to 256 bytes", "clear": "clear — clear output in a real terminal; it is a no-op in scripted labs", "cp": "cp [-r] SOURCE... DEST — copy files or directory trees", - "cut": "cut -d DELIMITER -f FIELD [FILE] — select a delimited field", + "cut": "cut [-s] [-d CHAR] -f LIST [FILE...] — select delimited fields; LIST is N, N-M, N-, or -M joined by commas", "dirname": "dirname PATH — print a path without its final component", "du": "du [-a|-s] [-b|-h] [PATH...] — show virtual file sizes", "echo": "echo [-n] [TEXT...] — print arguments", "env": "env — print the current environment", "export": "export NAME=value... — set shell environment variables", - "find": "find [PATH] [-name GLOB] [-type f|d] [-exec COMMAND {} \\;]", - "grep": "grep [-rilnvcFwE] PATTERN [FILE...] — print lines matching a basic regex; \\| \\+ \\( \\) are operators, or use -E for extended syntax", + "false": "false — fail silently without output; useful with && and ||", + "find": "find [PATH...] [-name GLOB] [-iname GLOB] [-type f|d] [-exec COMMAND {} \\;] — every test must match (implicit AND)", + "grep": "grep [-rilnvcqFwE] PATTERN [FILE...] — print lines matching a basic regex; -q prints nothing and only sets the status for && and ||; \\| \\+ \\( \\) are operators, or use -E for extended syntax", "gzip": "gzip FILE... — add the .gz suffix to virtual compressed files", "gunzip": "gunzip FILE.gz... — restore virtual compressed files", "head": "head [-n [-]COUNT] [FILE...] — print the first lines; -n -N prints all but the last N", @@ -57,7 +59,7 @@ var commandManuals = map[string]string{ "history": "history — show commands entered in this mission attempt", "kill": "kill [-9|-15] PID... — stop a mission process", "less": "less FILE... — display file content in the non-interactive lab", - "ls": "ls [-la] [PATH...] — list directory contents", + "ls": "ls [-la] [PATH...] — list files as named, then directory contents; -a includes hidden entries, . and ..", "man": "man COMMAND — show the same focused help as help COMMAND", "mkdir": "mkdir [-p] DIR... — create directories", "mv": "mv SOURCE... DEST — move or rename paths", @@ -68,9 +70,9 @@ var commandManuals = map[string]string{ "rmdir": "rmdir DIR... — remove empty directories", "sed": "sed [-inE] 's/REGEX/REPLACEMENT/[gp]' [FILE...] — substitute text; & is the match, \\1 a \\(group\\), and -E enables extended regex", "sh": "sh FILE — run a virtual UTF-8 script through the OpsQuest teaching shell\n" + - "Blank lines, comments, #!/bin/sh, existing commands, variables, pipelines, and redirection are supported.\n" + + "Blank lines, comments, #!/bin/sh, existing commands, variables, pipelines, redirection, and ; && || lists are supported.\n" + "Executable paths such as ./deploy.sh require a shebang and an executable mode; sh FILE does not.\n" + - "Scripts stop at the first error, restore their working directory and environment, and report virtual file/line locations.\n" + + "Scripts stop at the first unhandled error (a || b handles one), restore their working directory and environment, and report virtual file/line locations.\n" + "Limits: 64 KiB per script, 8 KiB per line, nesting depth 8, 256 dispatched commands, and 1 MiB output.\n" + "Options, arguments, stdin, loops, conditionals, functions, substitutions, background jobs, and external programs are unsupported.", "sort": "sort [-nru] [FILE...] — sort lines", @@ -78,6 +80,7 @@ var commandManuals = map[string]string{ "tail": "tail [-n [+]COUNT] [FILE...] — print the last lines; -n +N starts at line N", "tar": "tar -xf ARCHIVE [-C DIR] — extract; -C is extraction-only; -cf creates and -tf lists", "touch": "touch FILE... — create empty files when they do not exist", + "true": "true — succeed without output; useful with && and ||", "tr": "tr [-ds] SET1 [SET2] — translate, delete, or squeeze characters; sets accept ranges such as a-z and classes such as [:upper:]", "uniq": "uniq [-c] [FILE] — collapse adjacent duplicate lines", "vi": "vi FILE — edit one virtual UTF-8 text file up to 256 KiB interactively\n" + diff --git a/internal/sandbox/shell.go b/internal/sandbox/shell.go index 2084f7d..5421dd6 100644 --- a/internal/sandbox/shell.go +++ b/internal/sandbox/shell.go @@ -1,6 +1,7 @@ package sandbox import ( + "errors" "fmt" "slices" "strings" @@ -16,10 +17,26 @@ const ( ) type Result struct { + // Output is standard output only; output validators compare it. Output string Commands []string PipelineWidth int Editor *EditorRequest + // Transcript orders standard output and error messages for display when + // one line ran several commands (a; b, a && b, a || b) or a script handled + // a failure. Execute then returns a nil error and the last failure, if + // any, is the final entry. It is empty for a single command. + Transcript []OutputChunk + + list bool + statusFailed bool +} + +// OutputChunk is one piece of a Transcript: command output, or an error +// message the terminal shows on its error stream. +type OutputChunk struct { + Text string + Error bool } type executionContext struct { @@ -28,8 +45,16 @@ type executionContext struct { dispatchSteps int scriptStack []string scriptSteps int + // stderr collects messages from failures a command list handled inside a + // script, for the caller's terminal. + stderr []string } +// errFailureStatus is a failing exit status without an error message, such +// as grep finding no match or false. The command's output stays valid; the +// status matters only to && and || and to a script's own exit status. +var errFailureStatus = errors.New("command reported a failing status") + const ( wordToken tokenKind = iota pipeToken @@ -52,10 +77,35 @@ func (s *Sandbox) Execute(line string) (Result, error) { result, err := s.executeLine(line, context, true) result.Commands = slices.Clone(context.commands) result.PipelineWidth = context.maxPipelineWidth - return result, err + if !result.list && len(context.stderr) == 0 { + return result, err + } + // A command list reports each command's output and error in order rather + // than failing as a whole; earlier commands have already changed state. + transcript := errorChunks(context.stderr) + if result.list { + transcript = append(transcript, result.Transcript...) + } else if result.Output != "" { + transcript = append(transcript, OutputChunk{Text: result.Output}) + } + if err != nil { + transcript = append(transcript, OutputChunk{Text: err.Error(), Error: true}) + if !result.list { + // The single command failed as a whole, so none of it is practice. + result.Commands = nil + } + } + if transcriptBytes(transcript) > maxCommandOutputBytes { + return Result{}, commandOutputLimitError() + } + result.Transcript = transcript + return result, nil } -func (s *Sandbox) executeLine(line string, context *executionContext, allowInteractive bool) (Result, error) { +// executePipeline runs one pipeline: validate, lex, parse, expand, open +// output redirections, and run each stage. statusFailed in the result reports +// the last stage's silent failing status. +func (s *Sandbox) executePipeline(line string, context *executionContext, allowInteractive bool) (Result, error) { if err := validateShellSyntax(line); err != nil { return Result{}, err } @@ -120,17 +170,39 @@ func (s *Sandbox) executeLine(line string, context *executionContext, allowInter } stdin := "" + statusFailed := false for _, stage := range expanded.stages { + // Like sh, > empties its file before the command runs, so `sort f > f` + // reads nothing. If this stage then fails, the destination is restored: + // a rejected command never destroys data. + restore := func() error { return nil } + if stage.outputPath != "" && !stage.append { + restore, err = s.truncateRedirectTarget(s.Resolve(stage.outputPath)) + if err != nil { + return Result{}, fmt.Errorf("redirect: %w", err) + } + } + fail := func(err error) (Result, error) { + if restoreErr := restore(); restoreErr != nil { + err = errors.Join(err, fmt.Errorf("restore %s: %w", stage.outputPath, restoreErr)) + } + return Result{}, err + } if stage.inputPath != "" { stdin, err = s.FS.ReadFile(s.Resolve(stage.inputPath)) if err != nil { - return Result{}, fmt.Errorf("redirect: %w", err) + return fail(fmt.Errorf("redirect: %w", err)) } } name := stage.args[0] stdin, err = s.run(context, stage.args, stdin) + // A pipeline's status is its last stage's, as in sh without pipefail. + statusFailed = errors.Is(err, errFailureStatus) + if statusFailed { + err = nil + } if err != nil { - return Result{}, fmt.Errorf("%s: %w", name, err) + return fail(fmt.Errorf("%s: %w", name, err)) } if stage.outputPath != "" { target := s.Resolve(stage.outputPath) @@ -140,11 +212,63 @@ func (s *Sandbox) executeLine(line string, context *executionContext, allowInter err = s.FS.WriteFile(target, stdin, 0) } if err != nil { - return Result{}, fmt.Errorf("redirect: %w", err) + return fail(fmt.Errorf("redirect: %w", err)) } s.removeArchiveMetadata(target) stdin = "" } } - return Result{Output: stdin}, nil + return Result{Output: stdin, statusFailed: statusFailed}, nil +} + +// truncateRedirectTarget empties or creates target for > and returns a +// function that puts back its previous content, mode, owner, and archive +// metadata, or removes a file the truncation created. +func (s *Sandbox) truncateRedirectTarget(target string) (func() error, error) { + previous, existed := s.FS.Entry(target) + var saved Entry + if existed { + saved = *previous + } + archive, hadArchive := s.Archives[target] + if err := s.FS.WriteFile(target, "", 0); err != nil { + return nil, err + } + s.removeArchiveMetadata(target) + return func() error { + if !existed { + return s.FS.Remove(target, false, true) + } + if err := s.FS.WriteFile(target, saved.Content, saved.Mode); err != nil { + return err + } + if err := s.FS.Chown(target, saved.Owner); err != nil { + return err + } + if !hadArchive { + return nil + } + archives, err := s.planArchiveReplacement(target, archive) + if err != nil { + return err + } + s.Archives = archives + return nil + }, nil +} + +func errorChunks(messages []string) []OutputChunk { + chunks := make([]OutputChunk, 0, len(messages)) + for _, message := range messages { + chunks = append(chunks, OutputChunk{Text: message, Error: true}) + } + return chunks +} + +func transcriptBytes(chunks []OutputChunk) int { + total := 0 + for _, chunk := range chunks { + total += len(chunk.Text) + } + return total } diff --git a/internal/sandbox/syntax.go b/internal/sandbox/syntax.go index 4f0910a..2fcbe81 100644 --- a/internal/sandbox/syntax.go +++ b/internal/sandbox/syntax.go @@ -7,9 +7,10 @@ import ( ) // validateShellSyntax rejects shell language outside the teaching subset -// before lexing. Interactive lines and script lines share these rules, so -// syntax such as `a; b`, `a && b`, or `cmd 2>/dev/null` fails with a clear -// message instead of silently becoming literal arguments. +// before one pipeline is lexed. Interactive lines and script lines share these +// rules, so syntax such as `cmd &` or `cmd 2>/dev/null` fails with a clear +// message instead of silently becoming literal arguments. Command-list +// operators (;, &&, ||) are split off earlier by splitCommandList. func validateShellSyntax(line string) error { runes := []rune(line) var quote rune @@ -66,8 +67,6 @@ func validateShellSyntax(line string) error { next = runes[index+1] } switch char { - case ';': - return fmt.Errorf("shell control syntax %q is not supported; enter each command on its own line", ";") case '&': if index > 0 && (runes[index-1] == '>' || runes[index-1] == '<') { return fdRedirectionError(string(runes[index-1 : index+1])) @@ -75,18 +74,12 @@ func validateShellSyntax(line string) error { if next == '>' { return fdRedirectionError("&>") } - if next == '&' { - return fmt.Errorf("shell control syntax %q is not supported; enter each command on its own line", "&&") - } return fmt.Errorf("shell control syntax %q is not supported; background jobs are not available", "&") case '(', ')': return fmt.Errorf("shell control syntax %q is not supported", string(char)) case '`': return fmt.Errorf("command substitution is not supported") case '|': - if next == '|' { - return fmt.Errorf("control operator || is not supported; enter each command on its own line") - } tokenStarted = false continue case '<', '>': diff --git a/internal/sandbox/syntax_test.go b/internal/sandbox/syntax_test.go index 2355135..8b28d2d 100644 --- a/internal/sandbox/syntax_test.go +++ b/internal/sandbox/syntax_test.go @@ -11,9 +11,12 @@ func TestInteractiveLinesRejectUnsupportedShellSyntaxBeforeRunning(t *testing.T) line string want string }{ - {name: "semicolon", line: "touch /out/first.txt; touch /out/second.txt", want: `shell control syntax ";"`}, - {name: "and list", line: "touch /out/first.txt && touch /out/second.txt", want: `shell control syntax "&&"`}, - {name: "or list", line: "touch /out/first.txt || touch /out/second.txt", want: "control operator ||"}, + {name: "empty list item", line: "touch /out/first.txt ;; touch /out/second.txt", want: `syntax error near unexpected ";"`}, + {name: "leading operator", line: "&& touch /out/first.txt", want: `syntax error near unexpected "&&"`}, + {name: "dangling or", line: "touch /out/first.txt ||", want: `cannot end with "||"`}, + {name: "unsupported syntax later in a list", line: "touch /out/first.txt; echo $(pwd)", want: "command substitution"}, + {name: "parse error later in a list", line: "touch /out/first.txt && | touch /out/second.txt", want: "unexpected pipe"}, + {name: "editor in a list", line: "touch /out/first.txt; vi events.log", want: "cannot be combined"}, {name: "background job", line: "touch /out/first.txt &", want: "background jobs"}, {name: "subshell", line: "(touch /out/first.txt)", want: `shell control syntax "("`}, {name: "stderr redirection", line: "touch /out/first.txt 2>/out/errors.txt", want: `redirection such as "2>"`},