diff --git a/convox.yml b/convox.yml index 5f22cf96..61a4c784 100644 --- a/convox.yml +++ b/convox.yml @@ -30,7 +30,7 @@ services: # Pinned to an immutable release tag. scripts/bump-version.sh sets it to the new # version (:vX.Y.Z); the release workflow publishes that tag when the v* git tag is pushed. # :e327add is the v0.1.1 release (identical to :latest when this was pinned). - image: docker.io/docspringcom/rack-gateway:v0.1.30 + image: docker.io/docspringcom/rack-gateway:v0.1.31 command: ./scripts/start-gateway.sh environment: - PORT=8080 @@ -54,7 +54,7 @@ services: admin: # Uses same image as gateway service (kept in sync by scripts/bump-version.sh) - image: docker.io/docspringcom/rack-gateway:v0.1.30 + image: docker.io/docspringcom/rack-gateway:v0.1.31 command: echo "Use this service to run migrations, database admin tasks, etc." environment: - PORT=8080 diff --git a/docs/src/content/docs/user-guide/cli/commands.mdx b/docs/src/content/docs/user-guide/cli/commands.mdx index 8984cef5..71c411e1 100644 --- a/docs/src/content/docs/user-guide/cli/commands.mdx +++ b/docs/src/content/docs/user-guide/cli/commands.mdx @@ -265,8 +265,7 @@ rack-gateway exec -a - `--justification` - Why the command is run and what it does (see [Justifications](#justifications)) - `--env`, `--all-env-vars`, `--write` - The env vars the command gets (see [Env vars](#env-vars)) - `--script` - A local script file for the command to run (see [Scripts](#scripts)) -- `--dry-run` - Check the command and review its script, then show what would be sent, without asking for MFA or - sending anything to the rack +- `--skip-syntax-check` - Don't check a `.rb` script's syntax with your local Ruby first (see [Scripts](#scripts)) - `--entrypoint` - Override the entrypoint **Example:** @@ -336,8 +335,7 @@ rack-gateway run -a - `--justification` - Why the command is run and what it does (see [Justifications](#justifications)) - `--env`, `--all-env-vars`, `--write` - The env vars the command gets (see [Env vars](#env-vars)) - `--script` - A local script file for the command to run (see [Scripts](#scripts)) -- `--dry-run` - Check the command and review its script, then show what would be sent, without asking for MFA or - sending anything to the rack +- `--skip-syntax-check` - Don't check a `.rb` script's syntax with your local Ruby first (see [Scripts](#scripts)) **Example:** ```bash @@ -382,8 +380,7 @@ rack-gateway run web "rake db:migrate" -a myapp --write \ The PIN dialog says which env vars the command gets: how many of the app's `command_env_vars`, the ones it asked for and the read-only ones, such as `Env vars: the 98 in the app's command_env_vars, plus SENTRY_DSN; read-only -DATABASE_URL (from READ_ONLY_DATABASE_URL)`. `--dry-run` prints the same line, and the Slack notification says the -same. The dialog warns in red when `--write` lets the command change data, when `--all-env-vars` gives it every env +DATABASE_URL (from READ_ONLY_DATABASE_URL)`. The Slack notification says the same. The dialog warns in red when `--write` lets the command change data, when `--all-env-vars` gives it every env var in an app that limits them, and for every `exec` in such an app (see below). The touch prompt says when the command can change data. The audit log records every env var name the command got, the ones it asked for, and whether it ran with `--write`. @@ -415,6 +412,10 @@ This runs `rails runner /tmp//count_pro_accounts.rb` in a new `web` process it and a justification. It must be a UTF-8 text file without NUL bytes, and the command and script together must fit in the audit log (50 KB). +- **A `.rb` script's syntax is checked first.** The CLI has your local Ruby parse it, without running any of it, and + refuses a script with a syntax error before anyone is asked to review it. A script goes unchecked when there's no + `ruby` that runs on your machine. If your local Ruby refuses syntax the rack's Ruby understands, because it's older, + add `--skip-syntax-check`. - **You review the script before anything is sent.** The CLI shows where it runs, the command, the justification and the script with numbered lines and syntax highlighting (the language comes from the file's extension, else from the command, e.g. `rails runner` is Ruby), in the terminal, or in a dialog on macOS when there's no terminal (for @@ -429,8 +430,6 @@ fit in the audit log (50 KB). without a PIN, the dialog only asks for approval, and the command asks for MFA as usual. - **The PIN dialog shows the script's name, size and SHA-256**, so you can check the touch is for the script you reviewed. When the review dialog took the PIN, there's no PIN dialog: the review dialog showed the same lines. -- **`--dry-run` stops after the review** and prints what the PIN dialog would show and how many bytes would be - sent, without asking for MFA or sending anything. Use it to check a script before running it for real. - **The audit log records the script readably:** the command as you'd run it locally (`rails runner count_pro_accounts.rb`), the script's text with credentials redacted, its name, and the SHA-256 of what was sent, which a copy of the script can be checked against. The web UI shows the script with numbered lines. diff --git a/internal/cli/AGENTS.md b/internal/cli/AGENTS.md index c0ef6755..39b28226 100644 --- a/internal/cli/AGENTS.md +++ b/internal/cli/AGENTS.md @@ -291,8 +291,13 @@ The touch prompt says "can change data" when `--write` gets a read-only env var' `--script FILE` (`convox_process_command.go`) sends a local script with the command words: `scriptrun.Build` wraps them in a fixed `sh` runner that decodes the base64 script into a temp directory and runs the words with the file's path appended. The file is readable by every user, since the words often drop to another user (`gosu`). The gateway reads it back with `scriptrun.Parse` for the audit log, so the runner, -`SafeName` and `Parse` must stay in step. A script needs command words and `--justification`. Before any MFA -prompt, `scriptreview.Review` shows it with numbered lines, hidden characters as `` and chroma syntax +`SafeName` and `Parse` must stay in step. A script needs command words and `--justification`. A `.rb` script's syntax +is checked first (`scriptsyntax.Check`, `checkScriptSyntax`): `ruby --disable=all -c -` from the PATH, with the bytes +that will be sent on stdin and any `#!` line blanked, since nobody has reviewed the script yet and none of it may run +(`RUBYOPT`, `BEGIN` blocks, `#!` switches: `TestCheckRunsNothingInTheScript`). A Ruby that doesn't run, or fails any +other way than by exiting with an error, leaves the script unchecked. `--skip-syntax-check` is for syntax a local +Ruby is too old for. Before any MFA prompt, `scriptreview.Review` shows it with numbered lines, hidden characters as +`` and chroma syntax highlighting (`highlight.go`: styled spans, ANSI colors in a terminal, macOS system colors in the dialog; a lexer that fails or takes over 2 seconds leaves the script plain): a y/N prompt on a terminal, else a macOS JXA dialog (Cancel is the default; the texts go in as arguments, never into the JXA source), @@ -302,8 +307,15 @@ picks WebAuthn) and exactly one key is plugged in and has a PIN (`webauthn.Singl default. `webauthn.CheckPIN` checks each PIN without a touch (`CredentialsInfo`; a key without credential management leaves it to the assertion), a wrong one re-shows the dialog with the retries left, and the PIN goes to `newProcessClient`, so the assertion only needs the touch. The PIN dialog shows the script's name, line count and SHA-256, not its -text. `--dry-run` stops after the checks and the review (`runProcessCommand`): it prints the PIN dialog summary and -the command's size, with no MFA prompt and no request to the rack. +text. + +`--dry-run` is hidden from `--help` and the docs site, and is only for developing and testing these features. It stops +after the checks and the review (`runProcessCommand`): it prints the PIN dialog summary, the env var warnings and the +command's size, with no MFA prompt and no request to the rack. Use it to see a change to the checks or the summary +without a security key, and, with `--script` and no terminal on macOS, to see a change to the review dialog (without +its PIN field; Run there runs nothing). Keep it out of the user docs and runbooks, and don't run it before a real +command: a real run does the same checks before it asks anyone anything, so a dry run with a script only makes the +person approve the same dialog twice. ### Terminal output from the gateway diff --git a/internal/cli/convox_process_command.go b/internal/cli/convox_process_command.go index 5094c87a..637e1b25 100644 --- a/internal/cli/convox_process_command.go +++ b/internal/cli/convox_process_command.go @@ -19,21 +19,28 @@ import ( // scriptFlag names a local script file to run with the command (see scriptrun). const scriptFlag = "script" -// dryRunFlag stops a command once it's checked and its script reviewed: nothing is sent to the rack. +// dryRunFlag stops a command once it's checked and its script reviewed: nothing is sent to the rack. It's hidden: it +// helps when developing the script review, and a real run does the same checks first (internal/cli/AGENTS.md). const dryRunFlag = "dry-run" +// skipSyntaxCheckFlag sends a .rb script without checking its syntax with the local Ruby (see scriptsyntax). +const skipSyntaxCheckFlag = "skip-syntax-check" + // maxScriptFileBytes caps the script file read: the audit log keeps at most 50 KB of command and script anyway. const maxScriptFileBytes = 1 << 20 -// addProcessCommandFlags adds the flags exec and run share: --justification, --script, --dry-run and the env var -// flags (--env, --all-env-vars and --write). +// addProcessCommandFlags adds the flags exec and run share: --justification, --script, --skip-syntax-check, --dry-run +// and the env var flags (--env, --all-env-vars and --write). func addProcessCommandFlags(cmd *cobra.Command) { addJustificationFlag(cmd) addEnvFlags(cmd) cmd.Flags().String(scriptFlag, "", "a local script file to run: the command gets the file's path as its last "+ "argument (e.g. rails runner), and you review the script before it runs. Needs --justification") + cmd.Flags().Bool(skipSyntaxCheckFlag, false, "don't check a .rb script's syntax with your local Ruby first: for "+ + "syntax the rack's Ruby understands and yours doesn't") cmd.Flags().Bool(dryRunFlag, false, "check the command and review its script, then show what would be sent, "+ "without asking for MFA or sending anything to the rack") + cmd.Flags().Lookup(dryRunFlag).Hidden = true } // processCommand is an exec or run command as it's sent: where, the command, its justification and its script. @@ -116,6 +123,9 @@ func prepareProcessCommand(cobraCmd *cobra.Command, args []string, action string if process.dryRun, err = cobraCmd.Flags().GetBool(dryRunFlag); err != nil { return process, err } + if err := checkScriptSyntax(cobraCmd, process); err != nil { + return process, err + } process.pin, err = reviewScript(process) return process, err } diff --git a/internal/cli/convox_process_command_test.go b/internal/cli/convox_process_command_test.go index 1f1b7ae6..0fdc08d6 100644 --- a/internal/cli/convox_process_command_test.go +++ b/internal/cli/convox_process_command_test.go @@ -2,6 +2,7 @@ package cli import ( "bytes" + "context" "errors" "fmt" "io" @@ -138,9 +139,18 @@ func reviewedScripts(t *testing.T, err error) *[]processCommand { return "", err } t.Cleanup(func() { reviewScriptWith = previous }) + acceptScriptSyntax(t) return &reviewed } +// acceptScriptSyntax has the syntax check accept every script, so that tests don't depend on this machine's Ruby. +func acceptScriptSyntax(t *testing.T) { + t.Helper() + previous := checkScriptSyntaxWith + checkScriptSyntaxWith = func(context.Context, string, []byte) error { return nil } + t.Cleanup(func() { checkScriptSyntaxWith = previous }) +} + // With --script, the command sent is the script command (scriptrun.Build) made from the command words and the // file, once the script is approved. The rest of the CLI gets it as the command's only word. func TestProcessCommandSendsAnApprovedScript(t *testing.T) { @@ -248,3 +258,52 @@ func TestProcessCommandDryRun(t *testing.T) { require.ErrorIs(t, err, scriptreview.ErrDeclined) require.Len(t, *declined, 1) } + +// --dry-run works, but neither run nor exec shows it in their help: the real command does the same checks first. +func TestDryRunIsHiddenFromTheHelp(t *testing.T) { + for _, cmd := range []*cobra.Command{RunCommand(), ExecCommand()} { + require.NotContains(t, cmd.UsageString(), dryRunFlag, cmd.Name()) + require.NotNil(t, cmd.Flags().Lookup(dryRunFlag), cmd.Name()) + require.Contains(t, cmd.UsageString(), skipSyntaxCheckFlag, cmd.Name()) + } +} + +// A script with a syntax error is refused before anyone is asked to review it, and nothing reaches the rack. The +// check gets the script's name and exact text. +func TestProcessCommandChecksScriptSyntaxBeforeTheReview(t *testing.T) { + gateway := newEnvChangeGateway(t) + reviewed := reviewedScripts(t, nil) + var checked []string + previous := checkScriptSyntaxWith + checkScriptSyntaxWith = func(_ context.Context, name string, text []byte) error { + checked = append(checked, name, string(text)) + return errors.New("count.rb has a syntax error") + } + t.Cleanup(func() { checkScriptSyntaxWith = previous }) + text := "#!/usr/bin/env ruby\nputs (1\n" + flags := map[string]string{scriptFlag: writeScript(t, "count.rb", text), justificationFlag: testJustification} + + cmd := processCobraCommand(t, RunCommand(), flags) + err := runProcessCommand(cmd, []string{"web", "rails", "runner"}, "run", nil) + + require.ErrorContains(t, err, "count.rb has a syntax error\nNothing was sent. Fix the script") + require.ErrorContains(t, err, "--"+skipSyntaxCheckFlag) + require.Equal(t, []string{"count.rb", text}, checked) + require.Empty(t, *reviewed, "nobody is asked to review it") + require.Empty(t, gateway.requests()) + + // --skip-syntax-check goes straight to the review. A dry run is checked like any other command. + checked = nil + flags[skipSyntaxCheckFlag], flags[dryRunFlag] = "true", "true" + cmd = processCobraCommand(t, RunCommand(), flags) + cmd.SetOut(io.Discard) + require.NoError(t, runProcessCommand(cmd, []string{"web", "rails", "runner"}, "run", nil)) + require.Empty(t, checked) + require.Len(t, *reviewed, 1) + + delete(flags, skipSyntaxCheckFlag) + cmd = processCobraCommand(t, RunCommand(), flags) + err = runProcessCommand(cmd, []string{"web", "rails", "runner"}, "run", nil) + require.ErrorContains(t, err, "syntax error") + require.Len(t, *reviewed, 1) +} diff --git a/internal/cli/convox_process_review.go b/internal/cli/convox_process_review.go index 59637d4b..e6cfa365 100644 --- a/internal/cli/convox_process_review.go +++ b/internal/cli/convox_process_review.go @@ -1,14 +1,40 @@ package cli import ( + "context" "errors" + "fmt" "strings" + "github.com/spf13/cobra" + "github.com/DocSpring/rack-gateway/internal/cli/scriptreview" + "github.com/DocSpring/rack-gateway/internal/cli/scriptsyntax" "github.com/DocSpring/rack-gateway/internal/cli/terminal" "github.com/DocSpring/rack-gateway/internal/cli/webauthn" ) +// checkScriptSyntaxWith checks a script's syntax. Tests replace it. +var checkScriptSyntaxWith = scriptsyntax.Check + +// checkScriptSyntax refuses a script with a syntax error (scriptsyntax.Check) before anyone is asked to review it, +// unless --skip-syntax-check says the local Ruby can't judge it. +func checkScriptSyntax(cobraCmd *cobra.Command, process processCommand) error { + if process.script == nil { + return nil + } + skip, err := cobraCmd.Flags().GetBool(skipSyntaxCheckFlag) + if err != nil || skip { + return err + } + err = checkScriptSyntaxWith(context.Background(), process.script.Name, []byte(process.script.Text)) + if err != nil { + return fmt.Errorf("%w\nNothing was sent. Fix the script, or if the rack's Ruby understands this syntax and "+ + "yours doesn't, add --%s", err, skipSyntaxCheckFlag) + } + return nil +} + // reviewScript has the person running the command approve its script before anything is sent, and returns the // security key PIN they entered with the approval, or "". API tokens run unattended: there's no one to ask. func reviewScript(process processCommand) (string, error) { diff --git a/internal/cli/scriptsyntax/scriptsyntax.go b/internal/cli/scriptsyntax/scriptsyntax.go new file mode 100644 index 00000000..91789fb0 --- /dev/null +++ b/internal/cli/scriptsyntax/scriptsyntax.go @@ -0,0 +1,100 @@ +// Package scriptsyntax checks a script's syntax locally before the CLI sends it, so that a typo is refused before +// anyone reviews the script or touches a security key. +package scriptsyntax + +import ( + "bytes" + "context" + "errors" + "fmt" + "os/exec" + "path/filepath" + "regexp" + "strings" + "time" +) + +const ( + // checkTimeout bounds the check: a Ruby that doesn't answer in time leaves the script unchecked. + checkTimeout = 10 * time.Second + // killWait bounds the wait for a killed Ruby's output, which a process it started can hold open. + killWait = time.Second + // maxOutputLines caps Ruby's output in the error: a script can have many syntax errors. + maxOutputLines = 30 +) + +// stdinLocation is how Ruby names the script it read from stdin at the start of a line, with the line number and +// whatever comes before it: the path of the Ruby that's running (3.4), "-: " (3.3), or no line number on the +// "-: compile error" line that ends the output (3.2). +var stdinLocation = regexp.MustCompile(`(?m)^(?:\S*ruby\S*: )?(?:-: )?-(:\d+:|: )`) + +// Check returns an error when the local Ruby refuses the syntax of text, the script in the file called name. It +// only checks .rb files. A machine without a Ruby that runs leaves the script unchecked, as does a Ruby that fails +// any other way than by exiting with an error. +// +// This runs before anyone has reviewed the script, so nothing in it may run: -c only parses, BEGIN blocks included. +// --disable=all ignores RUBYOPT and leaves out the gems, and the script goes in on stdin, as the exact bytes that +// will be sent, with any #! line blanked (see withoutShebang). +func Check(ctx context.Context, name string, text []byte) error { + if filepath.Ext(name) != ".rb" { + return nil + } + ctx, cancel := context.WithTimeout(ctx, checkTimeout) + defer cancel() + // A version manager's shim also exits with an error when no version is set, so this comes first: that isn't a + // syntax error. + version, ok := rubyVersion(ctx) + if !ok { + return nil + } + cmd := rubyCommand(ctx, "--disable=all", "-c", "-") + cmd.Stdin = bytes.NewReader(withoutShebang(text)) + output, err := cmd.CombinedOutput() + var exit *exec.ExitError + if ctx.Err() != nil || !errors.As(err, &exit) || exit.ExitCode() <= 0 { + return nil + } + return fmt.Errorf("%s has a syntax error, according to %s:\n%s", name, version, describe(output, name)) +} + +// rubyVersion returns the local Ruby's name and version ("ruby 3.4.1"), or false if it doesn't run. +func rubyVersion(ctx context.Context) (string, bool) { + output, err := rubyCommand(ctx, "--version").Output() + if err != nil { + return "", false + } + fields := strings.Fields(string(output)) + if len(fields) < 2 { + return "", false + } + return fields[0] + " " + fields[1], true +} + +func rubyCommand(ctx context.Context, args ...string) *exec.Cmd { + cmd := exec.CommandContext(ctx, "ruby", args...) + cmd.WaitDelay = killWait + return cmd +} + +// withoutShebang blanks a #! first line and keeps the line numbers. Ruby takes switches from that line, and when it +// names another interpreter Ruby looks for a Ruby script further down, and only parsing is wanted here. +func withoutShebang(text []byte) []byte { + if !bytes.HasPrefix(text, []byte("#!")) { + return text + } + if lineEnd := bytes.IndexByte(text, '\n'); lineEnd >= 0 { + return text[lineEnd:] + } + return nil +} + +// describe is Ruby's output as the error shows it: naming the script, and cut to maxOutputLines. name is safe to use +// in a replacement: scriptrun.SafeName leaves no $. +func describe(output []byte, name string) string { + message := stdinLocation.ReplaceAllString(strings.TrimSpace(string(output)), name+"${1}") + lines := strings.Split(message, "\n") + if extra := len(lines) - maxOutputLines; extra > 0 { + lines = append(lines[:maxOutputLines], fmt.Sprintf("(%d more lines)", extra)) + } + return strings.Join(lines, "\n") +} diff --git a/internal/cli/scriptsyntax/scriptsyntax_test.go b/internal/cli/scriptsyntax/scriptsyntax_test.go new file mode 100644 index 00000000..28255898 --- /dev/null +++ b/internal/cli/scriptsyntax/scriptsyntax_test.go @@ -0,0 +1,178 @@ +package scriptsyntax + +import ( + "context" + "fmt" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/stretchr/testify/require" +) + +// fakeRuby puts a ruby on the PATH that answers --version and runs body for anything else. It records its arguments +// and stdin in the directory it returns. +func fakeRuby(t *testing.T, versionExit int, body string) string { + t.Helper() + dir := t.TempDir() + script := fmt.Sprintf(`#!/bin/sh +if [ "$1" = "--version" ]; then + echo "ruby 3.4.1 (2024-12-25 revision 48d4efcb85) +PRISM [arm64-darwin24]" + exit %d +fi +echo "$@" > %q/args +cat > %q/stdin +%s +`, versionExit, dir, dir, body) + executable := os.FileMode(0o755) + path := filepath.Join(dir, "ruby") + require.NoError(t, os.WriteFile(path, []byte(script), 0o600)) + require.NoError(t, os.Chmod(path, executable)) + t.Setenv("PATH", dir+":/usr/bin:/bin") + return dir +} + +func TestCheckRefusesWhatRubyRefuses(t *testing.T) { + dir := fakeRuby(t, 0, `echo '/opt/rubies/3.4.1/bin/ruby: -:3: syntax errors found (SyntaxError)' >&2 +echo '> 3 | end' >&2 +echo '/usr/bin/ruby3.4: -:4: a packaged Ruby' >&2 +echo '-: -:5: ruby 3.3' >&2 +echo '-:6: ruby 3.2' >&2 +echo 'puts -:7: code Ruby echoes' >&2 +echo '-: compile error (SyntaxError)' >&2 +exit 1`) + + err := Check(context.Background(), "count.rb", []byte("#!/usr/bin/env -S ruby -s\nputs 1\n")) + + require.EqualError(t, err, `count.rb has a syntax error, according to ruby 3.4.1: +count.rb:3: syntax errors found (SyntaxError) +> 3 | end +count.rb:4: a packaged Ruby +count.rb:5: ruby 3.3 +count.rb:6: ruby 3.2 +puts -:7: code Ruby echoes +count.rb: compile error (SyntaxError)`) + args, readErr := os.ReadFile(filepath.Join(dir, "args")) // #nosec G304 -- the test's own directory + require.NoError(t, readErr) + require.Equal(t, "--disable=all -c -\n", string(args)) + stdin, readErr := os.ReadFile(filepath.Join(dir, "stdin")) // #nosec G304 -- the test's own directory + require.NoError(t, readErr) + require.Equal(t, "\nputs 1\n", string(stdin), "the #! line is blanked and the line numbers stay") +} + +func TestCheckCutsLongOutput(t *testing.T) { + fakeRuby(t, 0, `i=0; while [ $i -lt 40 ]; do echo "error $i" >&2; i=$((i+1)); done; exit 1`) + + err := Check(context.Background(), "count.rb", []byte("puts 1\n")) + + require.ErrorContains(t, err, "error 29\n(10 more lines)") + require.NotContains(t, err.Error(), "error 30") +} + +func TestCheckLeavesTheScriptUnchecked(t *testing.T) { + refuses := "echo refused >&2; exit 1" + + t.Run("a Ruby that accepts it", func(t *testing.T) { + fakeRuby(t, 0, "exit 0") + require.NoError(t, Check(context.Background(), "count.rb", []byte("puts 1\n"))) + }) + t.Run("another language", func(t *testing.T) { + dir := fakeRuby(t, 0, refuses) + for _, name := range []string{"count.py", "count", "rb", "count.rb.txt", "count.RB"} { + require.NoError(t, Check(context.Background(), name, []byte("(((\n"))) + } + require.NoFileExists(t, filepath.Join(dir, "args"), "Ruby never runs") + }) + t.Run("no Ruby", func(t *testing.T) { + t.Setenv("PATH", t.TempDir()) + require.NoError(t, Check(context.Background(), "count.rb", []byte("(((\n"))) + }) + t.Run("a version manager's shim with no version set", func(t *testing.T) { + dir := fakeRuby(t, 1, refuses) + require.NoError(t, Check(context.Background(), "count.rb", []byte("(((\n"))) + require.NoFileExists(t, filepath.Join(dir, "args"), "the check itself never runs") + }) + t.Run("a Ruby killed by a signal", func(t *testing.T) { + fakeRuby(t, 0, "kill -9 $$") + require.NoError(t, Check(context.Background(), "count.rb", []byte("puts 1\n"))) + }) + t.Run("a Ruby that hangs", func(t *testing.T) { + // Not exec: the sleep outlives the killed shell and holds its output open. + fakeRuby(t, 0, "sleep 30; exit 1") + ctx, cancel := context.WithTimeout(context.Background(), 200*time.Millisecond) + defer cancel() + started := time.Now() + require.NoError(t, Check(ctx, "count.rb", []byte("puts 1\n"))) + require.Less(t, time.Since(started), 10*time.Second) + }) +} + +func TestWithoutShebang(t *testing.T) { + require.Equal(t, "puts 1\n", string(withoutShebang([]byte("puts 1\n")))) + require.Equal(t, "\nputs 1\n", string(withoutShebang([]byte("#!/bin/sh\nputs 1\n")))) + require.Empty(t, withoutShebang([]byte("#!/bin/sh"))) + require.Equal(t, " #!/bin/sh\n", string(withoutShebang([]byte(" #!/bin/sh\n")))) +} + +// realRuby skips a test when this machine has no Ruby that runs, and otherwise runs it in an empty directory. +func realRuby(t *testing.T) string { + t.Helper() + dir := t.TempDir() + t.Chdir(dir) + if output, err := exec.CommandContext(context.Background(), "ruby", "--version").Output(); err != nil || + len(strings.Fields(string(output))) < 2 { + t.Skip("no Ruby that runs on this machine") + } + return dir +} + +func TestCheckWithARealRuby(t *testing.T) { + realRuby(t) + + require.NoError(t, Check(context.Background(), "count.rb", []byte("puts Account.where(id: 1).count\n")), + "only the syntax is checked: Account doesn't need to exist") + require.NoError(t, Check(context.Background(), "count.rb", []byte("puts 1\n__END__\n(((\n")), + "data after __END__ isn't code") + require.NoError(t, Check(context.Background(), "count.rb", []byte("#!/bin/sh\nputs 1\n")), + "a #! line naming another interpreter doesn't stop the check") + + err := Check(context.Background(), "count.rb", []byte("#!/usr/bin/env ruby\nputs 1\ndef count\n puts (1\nend\n")) + require.ErrorContains(t, err, "count.rb has a syntax error, according to ruby ") + require.ErrorContains(t, err, "count.rb:") + require.NotContains(t, err.Error(), "-:", "names the script, not stdin") +} + +// The check runs before anyone has reviewed the script, so nothing in it may run. +func TestCheckRunsNothingInTheScript(t *testing.T) { + dir := realRuby(t) + marker := filepath.Join(dir, "ran") + write := fmt.Sprintf("File.write(%q, 'x')\n", marker) + require.NoError(t, os.WriteFile(filepath.Join(dir, "planted.rb"), []byte(write), 0o600)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "rubygems.rb"), []byte(write), 0o600)) + t.Setenv("RUBYOPT", "-r./planted") + t.Setenv("RUBYLIB", dir) + + // Under this env the check does reach Ruby, and running a script does leave the marker: the checks below can fail. + require.ErrorContains(t, Check(context.Background(), "count.rb", []byte("puts (1\n")), "syntax error") + require.NoFileExists(t, marker) + run := exec.CommandContext(context.Background(), "ruby", "--disable=all", "planted.rb") + require.NoError(t, run.Run()) + require.FileExists(t, marker) + require.NoError(t, os.Remove(marker)) + + for name, script := range map[string]string{ + "the top level": write, + "a BEGIN block": "BEGIN { " + write + " }\n", + "an END block": "END { " + write + " }\n", + "a #! -r switch": "#!ruby -r./planted\nputs 1\n", + "a #! -e switch": "#!ruby -e " + strings.TrimSpace(write) + "\nputs 1\n", + } { + t.Run(name, func(t *testing.T) { + require.NoError(t, Check(context.Background(), "count.rb", []byte(script))) + require.NoFileExists(t, marker) + }) + } +} diff --git a/web/package.json b/web/package.json index 077c1b59..8ceb8cd3 100644 --- a/web/package.json +++ b/web/package.json @@ -1,6 +1,6 @@ { "name": "web", - "version": "0.1.30", + "version": "0.1.31", "private": true, "type": "module", "scripts": {