diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3b8aa10..db78e7a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -15,9 +15,10 @@ concurrency: cancel-in-progress: ${{ github.event_name == 'pull_request' }} env: - # CI tool, not a Go dependency: installed with go install at this version, - # which verifies every module against the Go checksum database. + # CI tools, not Go dependencies. Each is installed with go install at this + # version, which verifies every module against the Go checksum database. GOLANGCI_LINT_VERSION: v2.15.0 + GOVULNCHECK_VERSION: v1.9.0 jobs: checks: @@ -54,6 +55,12 @@ jobs: go install "github.com/golangci/golangci-lint/v2/cmd/golangci-lint@${GOLANGCI_LINT_VERSION}" golangci-lint run ./... golangci-lint run --build-tags webdist ./... + - name: govulncheck + # Fails when the code calls a function with a known vulnerability, + # in a module or in the Go standard library. + run: | + go install "golang.org/x/vuln/cmd/govulncheck@${GOVULNCHECK_VERSION}" + govulncheck ./... - name: shellcheck run: | shellcheck --version diff --git a/README.md b/README.md index c043c2c..5fe0fd9 100644 --- a/README.md +++ b/README.md @@ -181,7 +181,7 @@ GitHub Actions runs five workflows. Every action is pinned by commit SHA, and ev | Workflow | Does | |---|---| -| `ci.yml` | gofmt, go vet, golangci-lint, shellcheck, i18n parity, the dashboard build, and every Go test with Postgres and Docker. It also checks that no script puts a credential on a command line and that the sandbox admission policy keeps every rule | +| `ci.yml` | gofmt, go vet, golangci-lint, govulncheck (known vulnerabilities), shellcheck, i18n parity, the dashboard build, and every Go test with Postgres and Docker. It also checks that no script puts a credential on a command line and that the sandbox admission policy keeps every rule | | `exercises.yml` | Validates every changed exercise with 3 seeds | | `dbt.yml` | Runs the dbt build and its tests against Postgres | | `commit-policy.yml` | Checks that every commit is signed and follows the commit rules | diff --git a/SECURITY.md b/SECURITY.md index 41283cb..6804672 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -17,4 +17,4 @@ Helpful things to include are what you found, how to reproduce it, and which mod ## Out of scope -The limits already listed in ADR 0019, such as a learner faking a check from inside their own sandbox, or k3d's privileged nodes. +The limits already listed in ADR 0019, such as a learner faking or reading a check from inside their own sandbox, filling their own disk from a sandbox, or k3d's privileged nodes. diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 14a2b87..e8f9fb7 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -134,7 +134,7 @@ variants: - Exits 0 for a pass and 1 for a fail. Exit 2 means the check could not run, which is an infrastructure error and never counts against the learner. - Prints one line of JSON, such as `{"passed": false, "feedback": "site_unreachable"}`. The feedback key maps to a text that describes the symptom, never the fix. - Accepts every valid fix and rejects shortcuts. Each check has at least one assertion against shortcuts. For example, the config must be valid and the original site content must be served, so replacing the config with a default one fails. -- Is copied in at check time and deleted afterwards, so the criteria cannot be read during the session. +- Is copied in at check time and deleted afterwards, so it is not lying around during the session. While it runs, the learner is root in the same sandbox and could still read it and its environment (section 13.3). - With `runs_from: runner` it runs in a separate runner container instead. That is a fresh copy of the exercise image on the internal network, with no added capabilities, destroyed after the check. `RC_SANDBOX_HOST` holds the sandbox's host name. **solution.sh** @@ -191,7 +191,7 @@ Placeholders are filled from variant values and derived values. Both language fi - Checks are unlimited, and a failed check costs nothing. - A reset gives a fresh copy of the same variant with no penalty. Revealed hints stay revealed. - Giving up becomes possible once hint 3 is revealed. -- Infrastructure errors (check exit 2, a crashed sandbox) void the session, so it does not count. +- Infrastructure errors (check exit 2, or a sandbox that crashed or was removed) void the session, so it does not count. - An expired session counts as a fail only if at least one check was run. ## 7. Adaptation @@ -380,6 +380,7 @@ How it is built. - The dashboard follows the CLI by polling, the open session every 2 seconds and the overview every few seconds and on focus. The two front ends share only the database. - The session view never contains the seed, the variant values or the derived values. Hints appear once revealed, and the walkthrough once the session is solved or given up. - Everything the dashboard needs is in the binary, including its fonts (SIL OFL, with the licences next to the files) and its icons. It loads nothing from elsewhere. The terminal's code loads only on a session page. +- An error the dashboard has no text for shows a short translated message, and its technical detail goes to the terminal that runs `rootcause serve`, never to the browser. Static files are served without folder listings. - `scripts/ui-check.sh` checks the dashboard in a real browser. It covers the first-run intro, the collapsed rail, centred pages on wide screens, the reset dialog, axe-core accessibility rules, layout shift, the keyboard rules, the Tab order, terminal latency, the CSP, and screenshots in both languages. It is not part of CI. ## 12. Languages @@ -427,6 +428,8 @@ How it is built. - In `init: systemd` sandboxes, journald and tmp.mount are masked, because they need capabilities outside the allowlist. Service output is not kept in a journal. - `init: systemd` exercises need a host with cgroup v2, which is the WSL2 default. The systemd in the exercise images does not boot on cgroup v1, so the CLI and the selftest refuse such an exercise there with a clear error. - Sandboxes on the internal network can reach each other. With one learner and at most two sandboxes, this is accepted. +- check.sh and the seed and variant values it receives are copied in only for the check, but while the check runs, the learner is root in the same sandbox and could read them. That only spoils the learner's own practice, and the scripts are public in this repository anyway. +- A sandbox's writable filesystem has no size limit, so a learner (or a runaway command) could fill the host's disk. Docker can only limit it on XFS with project quotas. The tmpfs volumes an exercise declares are limited. **In Kubernetes mode** - k3d runs its node containers privileged and in the host's user namespace, so node root is host root, whatever Docker's userns-remap says. This is accepted for a local, single-user cluster (ADR 0011). Sandbox pods run in their own user namespaces, enforced by the admission policy for every requester, and system pods run as non-root users in their own user namespaces as well. Rootless k3d would fix this, but it needs sudo and k3d's support for it is still experimental. @@ -440,7 +443,7 @@ How it is built. | Workflow | Runs on | Does | |---|---|---| -| `ci.yml` | Every pull request and push to main | gofmt, go vet, golangci-lint, go test, shellcheck on all scripts, the frontend type check and build, and i18n key parity. Postgres comes from `rootcause init`, and any skipped test fails the run | +| `ci.yml` | Every pull request and push to main | gofmt, go vet, golangci-lint, govulncheck, go test, shellcheck on all scripts, the frontend type check and build, and i18n key parity. Postgres comes from `rootcause init`, and any skipped test fails the run | | `exercises.yml` | Changes under `exercises/` | For each changed exercise and 3 seeds, it builds the image, checks that break then check fails, and that break, solution, check passes | | `dbt.yml` | Changes under `dbt/` | A dbt build against a Postgres service container, seeded with a scripted series of sessions played through the engine | | `commit-policy.yml` | Every pull request | Rejects Co-authored-by trailers, tool attribution lines, author emails other than the author's, and unsigned commits | diff --git a/docs/adr/0005-bash-exercise-contract.md b/docs/adr/0005-bash-exercise-contract.md index 36bc3c7..18072cb 100644 --- a/docs/adr/0005-bash-exercise-contract.md +++ b/docs/adr/0005-bash-exercise-contract.md @@ -4,7 +4,7 @@ Accepted on 2026-10-27. ## Context -Exercises are written by hand, so they must be easy to read and review. The learner must never be able to read the fault or the check. +Exercises are written by hand, so they must be easy to read and review. The learner should not be handed the fault or the check. ## Decision @@ -16,4 +16,5 @@ Exercises are written by hand, so they must be easy to read and review. The lear ## Consequences - The image holds only what a real server would have. +- This keeps the scripts out of sight, not secret. While a check runs, the learner is root in the same sandbox and could read it (ADR 0019). - Every check has an assertion against shortcuts. For 3 seeds, CI runs break then check, which must fail, and break, solution, check, which must pass. diff --git a/docs/adr/0014-ci-github-actions.md b/docs/adr/0014-ci-github-actions.md index 560c078..511aa15 100644 --- a/docs/adr/0014-ci-github-actions.md +++ b/docs/adr/0014-ci-github-actions.md @@ -10,6 +10,7 @@ Every change must be checked the same way, within the free plan's Actions minute - The workflows are `ci.yml`, `exercises.yml`, `dbt.yml`, `commit-policy.yml` and `release.yml` (ARCHITECTURE.md section 14). The scripts only CI uses live in `.github/scripts`. - Every action is pinned by commit SHA. Workflows have no permissions by default. Each job gets read access to the repository, and package access only where it needs it. +- govulncheck fails CI when the code calls anything with a known vulnerability, in a module or in the Go standard library. - Any skipped Go test fails CI. Only changed exercises are validated on a pull request, and all of them before a release. - Workflows start on every pull request and decide inside the job what to run, so they can all be required checks. - Scripts never put a credential on a command line, and `.github/scripts/ci-check-credential-args.sh` checks that. diff --git a/docs/adr/0019-known-limits.md b/docs/adr/0019-known-limits.md index 79a2adc..4fcc1cb 100644 --- a/docs/adr/0019-known-limits.md +++ b/docs/adr/0019-known-limits.md @@ -13,6 +13,8 @@ These are accepted for a single-user practice tool that runs locally. - `check.before: restart` restarts the container. Files stay, but only PID 1 starts again and setup.sh does not run again. Services that must start at boot use `init: systemd`. - In Kubernetes mode there is no restart that keeps files, and `init: systemd` exercises (svc-02) are refused, because systemd cannot boot in a pod with its own user namespace. - A check runner can reach any sandbox on the internal network, not only its own. With at most two sessions this is accepted. +- While a check runs, the learner could read check.sh and the seed and variant values it receives, since both run as root in the same sandbox. It only spoils the learner's own practice, and the scripts are public anyway. +- A sandbox's writable filesystem has no size limit, so a learner could fill the host's disk. Docker limits it only on XFS with project quotas, which most hosts do not use. - k3d's nodes are privileged and run as host root (ADR 0011). - The cluster's dbt CronJob relies on dbt's own exit code, without the node count check. - The workflows and the commit check run from a pull request's own commits, so a pull request could weaken the checks that judge it. Changes to `.github/`, `scripts/` and `.githooks/` therefore need careful review. diff --git a/go.mod b/go.mod index 1f82da4..7eb40dd 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/DIZAMU/RootCause -go 1.27.2 +go 1.27.3 require ( github.com/gorilla/websocket v1.5.4-0.20250319132907-e064f32e3674 @@ -43,12 +43,12 @@ require ( github.com/spf13/pflag v1.0.10 // indirect github.com/x448/float16 v0.8.4 // indirect go.yaml.in/yaml/v2 v2.4.4 // indirect - golang.org/x/net v0.57.0 // indirect + golang.org/x/net v0.60.0 // indirect golang.org/x/oauth2 v0.36.0 // indirect - golang.org/x/sync v0.22.0 // indirect - golang.org/x/sys v0.47.0 // indirect - golang.org/x/term v0.45.0 // indirect - golang.org/x/text v0.40.0 // indirect + golang.org/x/sync v0.23.0 // indirect + golang.org/x/sys v0.48.0 // indirect + golang.org/x/term v0.46.0 // indirect + golang.org/x/text v0.42.0 // indirect golang.org/x/time v0.15.0 // indirect google.golang.org/protobuf v1.36.12-0.20260120151049-f2248ac996af // indirect gopkg.in/evanphx/json-patch.v4 v4.13.0 // indirect diff --git a/go.sum b/go.sum index 627ba30..4865d08 100644 --- a/go.sum +++ b/go.sum @@ -91,18 +91,18 @@ go.yaml.in/yaml/v2 v2.4.4 h1:tuyd0P+2Ont/d6e2rl3be67goVK4R6deVxCUX5vyPaQ= go.yaml.in/yaml/v2 v2.4.4/go.mod h1:gMZqIpDtDqOfM0uNfy0SkpRhvUryYH0Z6wdMYcacYXQ= go.yaml.in/yaml/v3 v3.0.5 h1:N6y/pJk8buWs9NY5ERU2HSMfm+IuD/OtfdAnq6kESPw= go.yaml.in/yaml/v3 v3.0.5/go.mod h1:HVTZu1O7/Vkt2N+BFy8Zza+lnLsABggaTM2ZpNIGuKg= -golang.org/x/net v0.57.0 h1:K5+3DljvIuDG9/Jv9rvyMywYNFCQ9RSUY6OOTTkT+tE= -golang.org/x/net v0.57.0/go.mod h1:KpXc8iv+r3XplLAG/f7Jsf9RPszJzdR0f58q9vGOuEU= +golang.org/x/net v0.60.0 h1:79p50tfZlm0J9YfoDsSi639qSXNGVwEzOPLCxM2FsYU= +golang.org/x/net v0.60.0/go.mod h1:2DA/G1UfVbCpQPeWTmMPGY7Cs2PkBkwu743bVX5PIVg= golang.org/x/oauth2 v0.36.0 h1:peZ/1z27fi9hUOFCAZaHyrpWG5lwe0RJEEEeH0ThlIs= golang.org/x/oauth2 v0.36.0/go.mod h1:YDBUJMTkDnJS+A4BP4eZBjCqtokkg1hODuPjwiGPO7Q= -golang.org/x/sync v0.22.0 h1:SZjpbeLmrCk4xhRSZFNZW5gFUeCeFgjekvI/+gfScek= -golang.org/x/sync v0.22.0/go.mod h1:9xrNwdLfx4jkKbNva9FpL6vEN7evnE43NNNJQ2LF3+0= -golang.org/x/sys v0.47.0 h1:o7XGOvZQCADBQQ4Y7VNq2dRWQR7JmOUW8Kxx4ZsNgWs= -golang.org/x/sys v0.47.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= -golang.org/x/term v0.45.0 h1:NwWyBmoJCbfTHpxrWoZ9C6/VxOf7ic219I8xZZFdrf0= -golang.org/x/term v0.45.0/go.mod h1:9aqxs0blBcrm/n0L9QW0aRVD+ktan8ssZromtqJC43w= -golang.org/x/text v0.40.0 h1:Ub2Z6/xjgF1WrYQz2nuITOEegKFtiIy+rieRJ5lHZKs= -golang.org/x/text v0.40.0/go.mod h1:hpnzDAfGV753zIKo+wk3u1bVKCGPbrnF7+7LBF/UHVY= +golang.org/x/sync v0.23.0 h1:KameEIfc1IkluZyXWLn39Wd4tURc6GbCiISGiZm2bQk= +golang.org/x/sync v0.23.0/go.mod h1:sUUOizhqBxiL6pEWpqNLUiaJn1ShEbZ6BBqskPbjZm0= +golang.org/x/sys v0.48.0 h1:bbX/i/6MgT9BVLM9RT1thmxL04yeTAhbEz4SyadbXoo= +golang.org/x/sys v0.48.0/go.mod h1:hNLxWAXmnKAxqDtdwIYC4bM9oQPEecfsnNMuSxOs3og= +golang.org/x/term v0.46.0 h1:3+OXuTbaKDgwk8jTi3aSLHRlmWqHEUDUtxnbFigO4YE= +golang.org/x/term v0.46.0/go.mod h1:+K02xbkittuwc0Am4abfA3Fc+XRGXkvBXNO88NCXPoc= +golang.org/x/text v0.42.0 h1:JbOZXgfeCPU9gacVtYliJqOhD+zhrEqK4LfdpmlUZqI= +golang.org/x/text v0.42.0/go.mod h1:ojzP1Z+2QtioaF8DTtO8K5q7JWVVYwZKenzujK0Zd0E= golang.org/x/time v0.15.0 h1:bbrp8t3bGUeFOx08pvsMYRTCVSMk89u4tKbNOZbp88U= golang.org/x/time v0.15.0/go.mod h1:Y4YMaQmXwGQZoFaVFk4YpCt4FLQMYKZe9oeV/f4MSno= google.golang.org/protobuf v1.36.12-0.20260120151049-f2248ac996af h1:+5/Sw3GsDNlEmu7TfklWKPdQ0Ykja5VEmq2i817+jbI= diff --git a/internal/cli/errors.go b/internal/cli/errors.go index ce84b96..5ce2306 100644 --- a/internal/cli/errors.go +++ b/internal/cli/errors.go @@ -2,6 +2,8 @@ package cli import ( "errors" + "fmt" + "io" "github.com/DIZAMU/RootCause/internal/i18n" "github.com/DIZAMU/RootCause/internal/runtime" @@ -40,25 +42,49 @@ func failWith(key string, vars map[string]string, cause error) error { // followed by its technical detail; that detail comes from Go, Docker or // Postgres and is not translated. func Message(b *i18n.Bundle, lang string, err error) string { + var ue *userError + if errors.As(err, &ue) && ue.cause != nil { + return b.T(lang, ue.key, ue.vars) + "\n" + b.T(lang, "error.detail", map[string]string{"detail": ue.cause.Error()}) + } + if msg, ok := known(b, lang, err); ok { + return msg + } + return b.T(lang, "error.generic", map[string]string{"detail": err.Error()}) +} + +// WebMessage renders err for the dashboard. It is Message without the +// technical detail: the browser gets only translated text, and the detail +// goes to log, the terminal that runs rootcause serve, where the learner can +// read it and nothing else can. +func WebMessage(b *i18n.Bundle, lang string, err error, log io.Writer) string { + var ue *userError + if errors.As(err, &ue) && ue.cause != nil { + _, _ = fmt.Fprintf(log, "rootcause serve: %v\n", err) + } + if msg, ok := known(b, lang, err); ok { + return msg + } + _, _ = fmt.Fprintf(log, "rootcause serve: %v\n", err) + return b.T(lang, "error.generic_web", nil) +} + +// known renders the failures that have their own text, without any +// technical detail. +func known(b *i18n.Bundle, lang string, err error) (string, bool) { var ue *userError switch { case errors.As(err, &ue): - msg := b.T(lang, ue.key, ue.vars) - if ue.cause != nil { - msg += "\n" + b.T(lang, "error.detail", map[string]string{"detail": ue.cause.Error()}) - } - return msg + return b.T(lang, ue.key, ue.vars), true case errors.Is(err, session.ErrGiveUpTooEarly): - return b.T(lang, "giveup.locked", nil) + return b.T(lang, "giveup.locked", nil), true case errors.Is(err, session.ErrNoShell): - return b.T(lang, "error.no_shell", nil) + return b.T(lang, "error.no_shell", nil), true case errors.Is(err, runtime.ErrNeedsCgroupV2): - return b.T(lang, "error.cgroup_v2_required", nil) + return b.T(lang, "error.cgroup_v2_required", nil), true case errors.Is(err, runtime.ErrSystemdUnsupported): - return b.T(lang, "error.systemd_unsupported", nil) - default: - return b.T(lang, "error.generic", map[string]string{"detail": err.Error()}) + return b.T(lang, "error.systemd_unsupported", nil), true } + return "", false } // English renders an error in English, for the developer commands, which run diff --git a/internal/cli/errors_test.go b/internal/cli/errors_test.go index 315aa59..e214144 100644 --- a/internal/cli/errors_test.go +++ b/internal/cli/errors_test.go @@ -56,6 +56,40 @@ func TestMessage(t *testing.T) { } } +// TestWebMessage checks that the dashboard never gets technical detail: it +// gets the translated message, and the detail goes to the serve terminal. +func TestWebMessage(t *testing.T) { + b := loadBundle(t) + cases := []struct { + name string + err error + want string // substring of the dashboard text + logged bool // whether the detail reaches the log + private string // must not reach the dashboard + }{ + {"unknown error", errors.New("dial tcp 127.0.0.1:5432: connection refused"), + "terminal where rootcause serve runs", true, "5432"}, + {"known key with cause", fail("error.compose_failed", errors.New("exit status 1")), + "Could not start Postgres", true, "exit status 1"}, + {"known key without cause", fail("error.no_root", nil), + "Cannot find the RootCause folder", false, ""}, + {"sentinel", session.ErrGiveUpTooEarly, "", false, ""}, + } + for _, c := range cases { + var log strings.Builder + got := WebMessage(b, "en", c.err, &log) + if c.want != "" && !strings.Contains(got, c.want) { + t.Errorf("%s: WebMessage = %q, want it to contain %q", c.name, got, c.want) + } + if c.private != "" && strings.Contains(got, c.private) { + t.Errorf("%s: WebMessage = %q leaks %q", c.name, got, c.private) + } + if c.logged != strings.Contains(log.String(), c.err.Error()) { + t.Errorf("%s: log = %q, want the error logged: %v", c.name, log.String(), c.logged) + } + } +} + // TestUsedKeysExist fails if this package asks for a locale key that does not // exist. Bundle.T falls back to printing the raw key, so a typo would // otherwise only show up on screen. diff --git a/internal/cli/serve.go b/internal/cli/serve.go index d02404a..b292a4f 100644 --- a/internal/cli/serve.go +++ b/internal/cli/serve.go @@ -43,7 +43,7 @@ func (a *App) Serve(ctx context.Context, args []string) error { ExercisesDir: a.exercisesDir(), Catalog: func() ([]store.ExerciseRow, error) { return loadAllExercises(a.root) }, BuildStats: a.buildStats, - Describe: func(lang string, err error) string { return Message(a.bundle, lang, err) }, + Describe: func(lang string, err error) string { return WebMessage(a.bundle, lang, err, os.Stderr) }, Assets: web.Assets(), CanRebuild: os.Getenv(EnvDBTMode) != "cronjob", }) diff --git a/internal/server/server.go b/internal/server/server.go index 5699a0f..f09bab2 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -158,11 +158,23 @@ func (s *Server) Handler(port int) http.Handler { mux.HandleFunc("GET /{$}", s.handleIndex) mux.HandleFunc("GET /index.html", s.handleIndex) if s.cfg.Assets != nil { - mux.Handle("GET /assets/", http.FileServerFS(s.cfg.Assets)) + mux.Handle("GET /assets/", noListing(http.FileServerFS(s.cfg.Assets))) } return s.guard(port, mux) } +// noListing serves files only. http.FileServer lists a directory's contents +// when asked for it, which the dashboard never needs. +func noListing(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if strings.HasSuffix(r.URL.Path, "/") { + http.NotFound(w, r) + return + } + next.ServeHTTP(w, r) + }) +} + // guard applies the hardening described in the package comment, and the // security headers every response gets. func (s *Server) guard(port int, next http.Handler) http.Handler { diff --git a/internal/server/server_test.go b/internal/server/server_test.go index 2eafb8f..3ec8aa0 100644 --- a/internal/server/server_test.go +++ b/internal/server/server_test.go @@ -182,7 +182,11 @@ func TestGuard(t *testing.T) { if err != nil { t.Fatal(err) } - r := serve(t, server.Config{Bundle: bundle, Assets: fstest.MapFS{"index.html": {Data: []byte(indexHTML)}}}) + r := serve(t, server.Config{Bundle: bundle, Assets: fstest.MapFS{ + "index.html": {Data: []byte(indexHTML)}, + "assets/app.js": {Data: []byte("// app")}, + "assets/fonts/x.woff2": {Data: []byte("font")}, + }}) cases := []struct { name, method, path, host, token string @@ -205,6 +209,9 @@ func TestGuard(t *testing.T) { {"api, upper-case host", "GET", "/api/i18n/no", "LOCALHOST:" + r.port, r.token, 200}, {"index, localhost host", "GET", "/", "localhost:" + r.port, "", 200}, {"unknown language", "GET", "/api/i18n/de", "", r.token, 404}, + {"asset file", "GET", "/assets/app.js", "", "", 200}, + {"asset folder is not listed", "GET", "/assets/", "", "", 404}, + {"nested asset folder is not listed", "GET", "/assets/fonts/", "", "", 404}, } for _, c := range cases { res := r.do(c.method, c.path, c.host, nil, c.token) diff --git a/internal/session/engine.go b/internal/session/engine.go index 366183c..a12f5e7 100644 --- a/internal/session/engine.go +++ b/internal/session/engine.go @@ -314,6 +314,16 @@ func (e *Engine) Check(ctx context.Context, sessionID string) (CheckResult, erro spec := e.specFor(ex, sessionID) res, err := e.runCheck(ctx, ex, spec, sess.SandboxID, scriptEnv(sess.Seed, sess.VariantValues)) if err != nil { + // A sandbox that is gone (removed from outside, or crashed and + // stopped) can never be checked again. That is an infrastructure + // error, so the session is voided and does not count (ARCHITECTURE.md + // section 6), instead of staying active with nothing to work on. + if e.sandboxGone(ctx, sess.SandboxID) { + _ = e.st.AddEvent(ctx, sessionID, "voided", map[string]any{"reason": "sandbox_gone"}) + _ = e.st.EndSession(ctx, sessionID, "voided", nil) + _ = e.rt.Destroy(ctx, sess.SandboxID) + return CheckResult{Infra: true}, nil + } return CheckResult{}, fmt.Errorf("run check: %w", err) } @@ -358,6 +368,22 @@ func (e *Engine) Check(ctx context.Context, sessionID string) (CheckResult, erro return result, nil } +// sandboxGone reports whether the runtime no longer runs the sandbox. If the +// runtime cannot say, the sandbox is assumed to be there, so a passing +// problem never voids a session. +func (e *Engine) sandboxGone(ctx context.Context, sandboxID string) bool { + boxes, err := e.rt.List(ctx) + if err != nil { + return false + } + for _, b := range boxes { + if b.ID == sandboxID { + return false + } + } + return true +} + // readyTimeout bounds how long a restarted sandbox may take to accept commands. const readyTimeout = 30 * time.Second diff --git a/internal/session/reset_test.go b/internal/session/reset_test.go index 1838833..51b72e7 100644 --- a/internal/session/reset_test.go +++ b/internal/session/reset_test.go @@ -2,6 +2,7 @@ package session import ( "context" + "errors" "io" "path/filepath" "testing" @@ -96,3 +97,66 @@ func TestResetVoidsOnFailure(t *testing.T) { } } } + +// goneRuntime has lost every sandbox: it lists none, and copying a script +// into one fails, as after a docker rm -f from outside. +type goneRuntime struct{ brokenRuntime } + +func (g *goneRuntime) CopyIn(context.Context, string, string, []byte) error { + return errors.New("Error response from daemon: No such container") +} + +// TestCheckVoidsWhenSandboxGone checks that a check on a sandbox that no +// longer exists voids the session as an infrastructure error, instead of +// failing with a raw error and leaving the session active. It needs the test +// DB, not Docker. +func TestCheckVoidsWhenSandboxGone(t *testing.T) { + dsn := testenv.DSN(t) + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + st := testenv.Store(t, dsn) + rt := &goneRuntime{} + learnerID := testLearner(t, st, rt) + + exDir, _ := filepath.Abs("../../exercises") + if err := st.SyncExercises(ctx, []store.ExerciseRow{ + {ID: "svc-01-intranet-config", Version: 1, Topic: "services", Difficulty: 1}, + }); err != nil { + t.Fatalf("sync: %v", err) + } + id := newID() + if _, err := st.CreateSession(ctx, store.NewSession{ + ID: id, LearnerID: learnerID, ExerciseID: "svc-01-intranet-config", + ExerciseVersion: 1, Difficulty: 1, Seed: 1, TTL: time.Hour, + }); err != nil { + t.Fatalf("create session: %v", err) + } + if err := st.SetSandbox(ctx, id, "fake-gone"); err != nil { + t.Fatalf("set sandbox: %v", err) + } + + res, err := New(rt, nil, nil, st, exDir).Check(ctx, id) + if err != nil { + t.Fatalf("Check returned an error, want a voided result: %v", err) + } + if !res.Infra || res.Passed { + t.Errorf("result = %+v, want an infrastructure result", res) + } + sess, err := st.SessionByID(ctx, id) + if err != nil { + t.Fatalf("reload: %v", err) + } + if sess.Status != "voided" { + t.Errorf("status = %q, want voided", sess.Status) + } + events, _ := st.Events(ctx, id) + var reason any + for _, e := range events { + if e.Type == "voided" { + reason = e.Payload["reason"] + } + } + if reason != "sandbox_gone" { + t.Errorf("voided event reason = %v, want sandbox_gone (events %v)", reason, events) + } +} diff --git a/locales/en.json b/locales/en.json index 9d650dd..c4c2c62 100644 --- a/locales/en.json +++ b/locales/en.json @@ -33,6 +33,7 @@ "help.unknown_command": "There is no command called {command}.", "help.usage": "rootcause - hands-on IT troubleshooting practice\n\nPlay:\n init Start Postgres, run migrations, create the local profile\n next Start the recommended next exercise\n start Start a specific exercise\n ticket Show the active ticket again\n shell Open a shell in the active sandbox\n hint Reveal the next hint\n check Grade the active session\n reset [--all] Fresh copy of the same problem (with --all, delete all your data)\n giveup End the session and show the walkthrough (after hint 3)\n status Active session, topic levels, review queue and best stars\n list All exercises with best stars\n stats [--refresh] Statistics per exercise (--refresh rebuilds them first)\n serve [--port N] Start the local dashboard on http://localhost:7300\n lang en|no Set the interface language\n\nDeveloper:\n validate Validate an exercise definition\n vars --seed Print a seed's variant values\n selftest --seed Prove an exercise end to end in real sandboxes\n", "error.generic": "Something went wrong ({detail}).", + "error.generic_web": "Something went wrong. The details are in the terminal where rootcause serve runs.", "error.detail": "Details: {detail}", "error.no_root": "Cannot find the RootCause folder. Run rootcause from inside the repo, or set RC_ROOT to its path.", "error.no_env": "Cannot read deploy/compose/.env. Copy deploy/compose/.env.example to deploy/compose/.env and set a password, or set RC_DATABASE_URL.", diff --git a/locales/no.json b/locales/no.json index 27041a4..00cb0d9 100644 --- a/locales/no.json +++ b/locales/no.json @@ -33,6 +33,7 @@ "help.unknown_command": "Det finnes ingen kommando som heter {command}.", "help.usage": "rootcause - praktisk øving i IT-feilsøking\n\nØving:\n init Start Postgres, kjør migreringer, lag den lokale profilen\n next Start den anbefalte neste øvelsen\n start <øvelse-id> Start en bestemt øvelse\n ticket Vis den aktive saken igjen\n shell Åpne et skall i den aktive sandkassen\n hint Vis neste hint\n check Vurder den aktive økten\n reset [--all] Ny kopi av samme problem (med --all, slett alle dataene dine)\n giveup Avslutt økten og vis gjennomgangen (etter hint 3)\n status Aktiv økt, nivå per emne, repetisjonskø og beste stjerner\n list Alle øvelser med beste stjerner\n stats [--refresh] Statistikk per øvelse (--refresh bygger den på nytt først)\n serve [--port N] Start det lokale dashbordet på http://localhost:7300\n lang en|no Velg språk for grensesnittet\n\nUtvikler:\n validate Valider en øvelsesdefinisjon\n vars --seed Vis variantverdiene for en seed\n selftest --seed Bevis at en øvelse virker i ekte sandkasser\n", "error.generic": "Noe gikk galt ({detail}).", + "error.generic_web": "Noe gikk galt. Detaljene står i terminalen der rootcause serve kjører.", "error.detail": "Detaljer: {detail}", "error.no_root": "Finner ikke RootCause-mappen. Kjør rootcause fra repoet, eller sett RC_ROOT til stien.", "error.no_env": "Kan ikke lese deploy/compose/.env. Kopier deploy/compose/.env.example til deploy/compose/.env og sett et passord, eller sett RC_DATABASE_URL.",