diff --git a/checks/cli.go b/checks/cli.go index 8e09b24..58e5e5a 100644 --- a/checks/cli.go +++ b/checks/cli.go @@ -1,6 +1,9 @@ package checks import ( + "bytes" + "context" + "errors" "fmt" "maps" "os" @@ -8,36 +11,118 @@ import ( "regexp" "runtime" "strings" + "time" api "github.com/bootdotdev/bootdev/client" ) +const ( + cliCommandTimeout = 5 * time.Minute + maxCLIOutputBytesPerStream = 1024 * 1024 + commandWaitDelay = 2 * time.Second +) + +var errCLIOutputLimitExceeded = errors.New("CLI command output limit exceeded") + +type boundedBuffer struct { + buffer bytes.Buffer + limit int + truncated bool + onTruncate func() +} + +func newBoundedBuffer(limit int, onTruncate func()) *boundedBuffer { + return &boundedBuffer{ + limit: max(limit, 0), + onTruncate: onTruncate, + } +} + +func (b *boundedBuffer) Write(p []byte) (int, error) { + originalLength := len(p) + remaining := max(b.limit-b.buffer.Len(), 0) + toWrite := min(len(p), remaining) + if toWrite > 0 { + _, _ = b.buffer.Write(p[:toWrite]) + } + if toWrite < len(p) && !b.truncated { + b.truncated = true + if b.onTruncate != nil { + b.onTruncate() + } + } + return originalLength, nil +} + +func (b *boundedBuffer) String() string { + return b.buffer.String() +} + +func (b *boundedBuffer) Truncated() bool { + return b.truncated +} + func runCLICommand(command api.CLIStepCLICommand, variables map[string]string) (result api.CLICommandResult) { + return runCLICommandWithLimits(command, variables, cliCommandTimeout, maxCLIOutputBytesPerStream) +} + +func runCLICommandWithLimits( + command api.CLIStepCLICommand, + variables map[string]string, + timeout time.Duration, + maxOutputBytesPerStream int, +) (result api.CLICommandResult) { finalCommand := InterpolateVariables(command.Command, variables) result.FinalCommand = finalCommand result.Command = command + timeoutCtx, cancelTimeout := context.WithTimeout(context.Background(), timeout) + defer cancelTimeout() + ctx, cancelCommand := context.WithCancelCause(timeoutCtx) + defer cancelCommand(nil) + var cmd *exec.Cmd if runtime.GOOS == "windows" { - cmd = exec.Command("powershell", "-Command", finalCommand) + cmd = exec.CommandContext(ctx, "powershell", "-Command", finalCommand) } else { - cmd = exec.Command("sh", "-c", finalCommand) + cmd = exec.CommandContext(ctx, "sh", "-c", finalCommand) } cmd.Env = append(os.Environ(), "LANG=en_US.UTF-8") - b, err := cmd.CombinedOutput() + cmd.WaitDelay = commandWaitDelay + cancelForOutputLimit := func() { + cancelCommand(errCLIOutputLimitExceeded) + } + stdout := newBoundedBuffer(maxOutputBytesPerStream, cancelForOutputLimit) + stderr := newBoundedBuffer(maxOutputBytesPerStream, cancelForOutputLimit) + cmd.Stdout = stdout + cmd.Stderr = stderr + err := cmd.Run() if ee, ok := err.(*exec.ExitError); ok { result.ExitCode = ee.ExitCode() } else if err != nil { result.ExitCode = -2 } - result.Stdout = strings.TrimRight(string(b), " \n\t\r") + result.Stdout = strings.TrimRight(stdout.String(), " \n\t\r") + result.Stderr = strings.TrimRight(stderr.String(), " \n\t\r") if command.StdoutFilterTmdl != nil { result.Stdout = ExtractTmdlBlock(result.Stdout, *command.StdoutFilterTmdl) } - if err := parseStdoutVariables(result.Stdout, command.StdoutVariables, variables); err != nil { - result.Err = err.Error() + + switch { + case errors.Is(context.Cause(ctx), errCLIOutputLimitExceeded): + result.Err = fmt.Sprintf("command output exceeded the %d-byte per-stream limit", maxOutputBytesPerStream) + result.ExitCode = -2 + case errors.Is(context.Cause(ctx), context.DeadlineExceeded): + result.Err = fmt.Sprintf("command timed out after %s", timeout) + result.ExitCode = -2 + } + + if result.Err == "" { + if err := parseStdoutVariables(result.Stdout, command.StdoutVariables, variables); err != nil { + result.Err = err.Error() + } } result.Variables = maps.Clone(variables) return result diff --git a/checks/cli_test.go b/checks/cli_test.go index f094f98..d85c906 100644 --- a/checks/cli_test.go +++ b/checks/cli_test.go @@ -2,11 +2,102 @@ package checks import ( "runtime" + "strings" "testing" + "time" api "github.com/bootdotdev/bootdev/client" ) +func TestRunCLICommandTimesOut(t *testing.T) { + command := `while :; do :; done` + if runtime.GOOS == "windows" { + command = `while ($true) {}` + } + + start := time.Now() + result := runCLICommandWithLimits( + api.CLIStepCLICommand{Command: command}, + map[string]string{}, + 20*time.Millisecond, + 1024, + ) + elapsed := time.Since(start) + + if !strings.Contains(result.Err, "command timed out") { + t.Fatalf("command error = %q, want timeout error", result.Err) + } + if result.ExitCode >= 0 { + t.Fatalf("exit code = %d, want internal failure", result.ExitCode) + } + if elapsed > time.Second { + t.Fatalf("command took %v, want a prompt timeout", elapsed) + } +} + +func TestRunCLICommandCapsOutput(t *testing.T) { + command := `printf 'abcdefgh'; while :; do :; done` + if runtime.GOOS == "windows" { + command = `[Console]::Out.Write('abcdefgh'); while ($true) {}` + } + + variables := map[string]string{} + start := time.Now() + result := runCLICommandWithLimits( + api.CLIStepCLICommand{ + Command: command, + StdoutVariables: []api.CLICommandStdoutVariable{{ + Name: "partial", + Regex: `(abcd)`, + }}, + }, + variables, + 5*time.Second, + 4, + ) + elapsed := time.Since(start) + + if !strings.Contains(result.Err, "per-stream limit") { + t.Fatalf("command error = %q, want per-stream output limit error", result.Err) + } + if result.ExitCode >= 0 { + t.Fatalf("exit code = %d, want internal failure", result.ExitCode) + } + if result.Stdout != "abcd" { + t.Fatalf("stdout = %q, want capped output %q", result.Stdout, "abcd") + } + if _, ok := variables["partial"]; ok { + t.Fatal("truncated output unexpectedly populated a stdout variable") + } + if elapsed > time.Second { + t.Fatalf("command took %v, want cancellation immediately after exceeding the output limit", elapsed) + } +} + +func TestBoundedBufferDiscardsExcessBytes(t *testing.T) { + truncations := 0 + buffer := newBoundedBuffer(5, func() { + truncations++ + }) + written, err := buffer.Write([]byte("abcdefgh")) + if err != nil { + t.Fatalf("Write() error = %v", err) + } + if written != 8 { + t.Fatalf("Write() = %d, want 8", written) + } + if got := buffer.String(); got != "abcde" { + t.Fatalf("buffer = %q, want abcde", got) + } + if !buffer.Truncated() { + t.Fatal("buffer did not record truncation") + } + _, _ = buffer.Write([]byte("more")) + if truncations != 1 { + t.Fatalf("truncation callback invoked %d times, want once", truncations) + } +} + func TestRunCLICommandCapturesStdoutVariables(t *testing.T) { variables := map[string]string{} result := runCLICommand(api.CLIStepCLICommand{ @@ -28,6 +119,43 @@ func TestRunCLICommandCapturesStdoutVariables(t *testing.T) { } } +func TestRunCLICommandKeepsStderrSeparateFromStdoutChecks(t *testing.T) { + command := `printf 'stdout-value\n'; printf 'stderr-value\n' >&2` + if runtime.GOOS == "windows" { + command = `Write-Output 'stdout-value'; [Console]::Error.WriteLine('stderr-value')` + } + + variables := map[string]string{} + step := api.CLIStepCLICommand{ + Command: command, + StdoutVariables: []api.CLICommandStdoutVariable{{ + Name: "stderr_value", + Regex: `(stderr-value)`, + }}, + Tests: []api.CLICommandTest{{ + StdoutContainsAll: []string{"stderr-value"}, + }}, + } + + result := runCLICommand(step, variables) + + if result.Stdout != "stdout-value" { + t.Fatalf("stdout = %q, want stdout-value", result.Stdout) + } + if result.Stderr != "stderr-value" { + t.Fatalf("stderr = %q, want stderr-value", result.Stderr) + } + if strings.Contains(result.Stdout, "stderr-value") { + t.Fatalf("stdout unexpectedly contains stderr: %q", result.Stdout) + } + if _, ok := variables["stderr_value"]; ok { + t.Fatalf("stderr unexpectedly populated a stdout variable") + } + if failure := evaluateCLICommandTests(0, step, result); failure == nil { + t.Fatal("stderr unexpectedly satisfied a stdout check") + } +} + func TestRunCLICommandInterpolatesCapturedStdoutVariables(t *testing.T) { variables := map[string]string{} diff --git a/checks/http.go b/checks/http.go index 57bbfe5..7770656 100644 --- a/checks/http.go +++ b/checks/http.go @@ -17,6 +17,11 @@ import ( "github.com/spf13/cobra" ) +const ( + maxHTTPResponseBodyBytes = 1024 * 1024 + maxBinaryBodyBytes = 16 * 1024 +) + func runHTTPRequest( client *http.Client, baseURL string, @@ -31,12 +36,12 @@ func runHTTPRequest( var req *http.Request if requestStep.Request.BodyJSON != nil { - dat, err := json.Marshal(requestStep.Request.BodyJSON) + bodyJSON := interpolateJSONStrings(requestStep.Request.BodyJSON, variables) + dat, err := json.Marshal(bodyJSON) cobra.CheckErr(err) - interpolatedBodyJSONStr := InterpolateVariables(string(dat), variables) req, err = http.NewRequest( requestStep.Request.Method, completeURL, - bytes.NewBuffer([]byte(interpolatedBodyJSONStr)), + bytes.NewReader(dat), ) if err != nil { cobra.CheckErr("Failed to create request") @@ -93,7 +98,7 @@ func runHTTPRequest( } defer resp.Body.Close() - body, err := io.ReadAll(resp.Body) + body, err := io.ReadAll(io.LimitReader(resp.Body, maxHTTPResponseBodyBytes+1)) if err != nil { result = api.HTTPRequestResult{Err: "Failed to read response body"} return result @@ -109,7 +114,8 @@ func runHTTPRequest( trailers[k] = strings.Join(v, ",") } - if err := parseVariables(body, requestStep.ResponseVariables, variables); err != nil { + bodyString := truncateAndStringifyBody(body) + if err := parseVariables([]byte(bodyString), requestStep.ResponseVariables, variables); err != nil { return api.HTTPRequestResult{Err: fmt.Sprintf("Failed to parse response variable: %s", err)} } if err := parseHeaderVariables(headers, requestStep.ResponseHeaderVariables, variables); err != nil { @@ -120,13 +126,34 @@ func runHTTPRequest( StatusCode: resp.StatusCode, ResponseHeaders: headers, ResponseTrailers: trailers, - BodyString: truncateAndStringifyBody(body), + BodyString: bodyString, Variables: maps.Clone(variables), Request: requestStep, } return result } +func interpolateJSONStrings(value any, variables map[string]string) any { + switch value := value.(type) { + case string: + return InterpolateVariables(value, variables) + case []any: + interpolated := make([]any, len(value)) + for i, item := range value { + interpolated[i] = interpolateJSONStrings(item, variables) + } + return interpolated + case map[string]any: + interpolated := make(map[string]any, len(value)) + for key, item := range value { + interpolated[key] = interpolateJSONStrings(item, variables) + } + return interpolated + default: + return value + } +} + func prettyPrintHTTPTest(test api.HTTPRequestTest, variables map[string]string) string { if test.StatusCode != nil { return fmt.Sprintf("Expecting status code: %d", *test.StatusCode) @@ -187,9 +214,9 @@ func prettyPrintHTTPTest(test api.HTTPRequestTest, variables map[string]string) // embedded in binary (e.g. "moov" in MP4 files) remain searchable by downstream checks. // The result is not guaranteed to be valid UTF-8 or lossless. func truncateAndStringifyBody(body []byte) string { - maxBodyLength := 1024 * 1024 // 1 MiB + maxBodyLength := maxHTTPResponseBodyBytes if likelyBinary(body) { - maxBodyLength = 16 * 1024 // 16 KiB + maxBodyLength = maxBinaryBodyBytes } if len(body) > maxBodyLength { body = body[:maxBodyLength] diff --git a/checks/http_test.go b/checks/http_test.go index c552faa..1a58e19 100644 --- a/checks/http_test.go +++ b/checks/http_test.go @@ -7,10 +7,33 @@ import ( "net/http/httptest" "strings" "testing" + "time" api "github.com/bootdotdev/bootdev/client" ) +type httpRoundTripFunc func(*http.Request) (*http.Response, error) + +func (f httpRoundTripFunc) RoundTrip(r *http.Request) (*http.Response, error) { + return f(r) +} + +type endlessReadCloser struct { + bytesRead int +} + +func (r *endlessReadCloser) Read(p []byte) (int, error) { + for i := range p { + p[i] = 'a' + } + r.bytesRead += len(p) + return len(p), nil +} + +func (r *endlessReadCloser) Close() error { + return nil +} + func TestInterpolateVariables(t *testing.T) { got := InterpolateVariables( "${baseURL}/users/${id}?missing=${missing}", @@ -117,13 +140,136 @@ func TestRunHTTPRequestInterpolatesRequestAndCapturesResponseVariables(t *testin } } +func TestRunHTTPRequestSafelyInterpolatesNestedJSONStrings(t *testing.T) { + specialValue := "quote: \"hello\", slash: \\, newline:\nnext" + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + var payload struct { + Message string `json:"message"` + Nested struct { + Values []any `json:"values"` + Literal string `json:"${dynamicKey}"` + } `json:"nested"` + } + if err := json.NewDecoder(r.Body).Decode(&payload); err != nil { + t.Errorf("decode request JSON: %v", err) + return + } + if payload.Message != specialValue { + t.Errorf("message = %q, want %q", payload.Message, specialValue) + } + if got := payload.Nested.Values[0]; got != "prefix "+specialValue+" suffix" { + t.Errorf("nested string = %q, want interpolated special value", got) + } + if got := payload.Nested.Values[1]; got != float64(42) { + t.Errorf("nested number = %#v, want 42", got) + } + if got := payload.Nested.Values[2]; got != true { + t.Errorf("nested boolean = %#v, want true", got) + } + if payload.Nested.Literal != "key was not interpolated" { + t.Errorf("literal dynamic key value = %q, want preserved key", payload.Nested.Literal) + } + w.WriteHeader(http.StatusNoContent) + })) + defer server.Close() + + requestStep := api.CLIStepHTTPRequest{ + Request: api.HTTPRequest{ + Method: http.MethodPost, + FullURL: api.BaseURLPlaceholder, + BodyJSON: map[string]any{ + "message": "${value}", + "nested": map[string]any{ + "values": []any{"prefix ${value} suffix", 42, true, nil}, + "${dynamicKey}": "key was not interpolated", + }, + }, + }, + } + + result := runHTTPRequest(server.Client(), server.URL, map[string]string{ + "value": specialValue, + "dynamicKey": "changed", + }, requestStep) + if result.Err != "" { + t.Fatalf("unexpected request error: %s", result.Err) + } + if result.StatusCode != http.StatusNoContent { + t.Fatalf("status = %d, want %d", result.StatusCode, http.StatusNoContent) + } +} + func TestTruncateAndStringifyBodyCapsBinaryBody(t *testing.T) { body := []byte(strings.Repeat("a", 20*1024)) body[0] = 0 got := truncateAndStringifyBody(body) - if len(got) != 16*1024 { - t.Fatalf("len(truncateAndStringifyBody(binary)) = %d, want %d", len(got), 16*1024) + if len(got) != maxBinaryBodyBytes { + t.Fatalf("len(truncateAndStringifyBody(binary)) = %d, want %d", len(got), maxBinaryBodyBytes) + } +} + +func TestRunHTTPRequestCapsResponseBodyRead(t *testing.T) { + body := &endlessReadCloser{} + client := &http.Client{ + Transport: httpRoundTripFunc(func(r *http.Request) (*http.Response, error) { + return &http.Response{ + StatusCode: http.StatusOK, + Header: make(http.Header), + Body: body, + Request: r, + }, nil + }), + } + requestStep := api.CLIStepHTTPRequest{ + Request: api.HTTPRequest{ + Method: http.MethodGet, + FullURL: "http://example.test", + }, + } + + result := runHTTPRequest(client, "", map[string]string{}, requestStep) + if result.Err != "" { + t.Fatalf("runHTTPRequest() error = %q", result.Err) + } + if body.bytesRead != maxHTTPResponseBodyBytes+1 { + t.Fatalf("response bytes read = %d, want %d", body.bytesRead, maxHTTPResponseBodyBytes+1) + } + if len(result.BodyString) != maxHTTPResponseBodyBytes { + t.Fatalf("stored response body length = %d, want %d", len(result.BodyString), maxHTTPResponseBodyBytes) + } +} + +func TestRunHTTPRequestHonorsClientTimeout(t *testing.T) { + const timeout = 20 * time.Millisecond + client := &http.Client{ + Timeout: timeout, + Transport: httpRoundTripFunc(func(r *http.Request) (*http.Response, error) { + <-r.Context().Done() + return nil, r.Context().Err() + }), + } + requestStep := api.CLIStepHTTPRequest{ + Request: api.HTTPRequest{ + Method: http.MethodGet, + FullURL: "http://example.test", + }, + } + + start := time.Now() + result := runHTTPRequest(client, "", map[string]string{}, requestStep) + elapsed := time.Since(start) + if result.Err == "" { + t.Fatal("runHTTPRequest() unexpectedly succeeded") + } + if elapsed > time.Second { + t.Fatalf("runHTTPRequest() took %v, want a prompt timeout", elapsed) + } +} + +func TestLessonHTTPClientUsesConfiguredTimeout(t *testing.T) { + if got := newLessonHTTPClient().Timeout; got != lessonHTTPRequestTimeout { + t.Fatalf("lesson HTTP client timeout = %v, want %v", got, lessonHTTPRequestTimeout) } } diff --git a/checks/local.go b/checks/local.go index b6cc095..9c5f227 100644 --- a/checks/local.go +++ b/checks/local.go @@ -146,6 +146,42 @@ func evaluateHTTPRequestTests(stepIndex int, req api.CLIStepHTTPRequest, result } } + captureIndex := len(req.Tests) + for _, vardef := range req.ResponseVariables { + expected := map[string]string{} + if err := parseVariables([]byte(result.BodyString), []api.HTTPRequestResponseVariable{vardef}, expected); err != nil { + return localFailure(stepIndex, captureIndex, err.Error()) + } + + want, found := expected[vardef.Name] + if !found { + return localFailure(stepIndex, captureIndex, fmt.Sprintf("missing value for response variable %q", vardef.Name)) + } + got, captured := result.Variables[vardef.Name] + if !captured || got != want { + return localFailure(stepIndex, captureIndex, fmt.Sprintf("captured response variable %q did not match the response body", vardef.Name)) + } + } + + if len(req.ResponseVariables) > 0 { + captureIndex++ + } + for _, vardef := range req.ResponseHeaderVariables { + expected := map[string]string{} + if err := parseHeaderVariables(result.ResponseHeaders, []api.HTTPRequestResponseHeaderVariable{vardef}, expected); err != nil { + return localFailure(stepIndex, captureIndex, err.Error()) + } + + want, found := expected[vardef.Name] + if !found { + return localFailure(stepIndex, captureIndex, fmt.Sprintf("missing value for response header variable %q", vardef.Name)) + } + got, captured := result.Variables[vardef.Name] + if !captured || got != want { + return localFailure(stepIndex, captureIndex, fmt.Sprintf("captured response header variable %q did not match the response header", vardef.Name)) + } + } + return nil } diff --git a/checks/local_test.go b/checks/local_test.go index 424af40..82fa6cd 100644 --- a/checks/local_test.go +++ b/checks/local_test.go @@ -115,6 +115,81 @@ func TestEvaluateStdoutJq(t *testing.T) { } } +func TestLocalSubmissionEventRejectsMissingHTTPResponseCaptures(t *testing.T) { + tests := []struct { + name string + request api.CLIStepHTTPRequest + result api.HTTPRequestResult + }{ + { + name: "response body variable", + request: api.CLIStepHTTPRequest{ + Tests: []api.HTTPRequestTest{{StatusCode: intPtr(200)}}, + ResponseVariables: []api.HTTPRequestResponseVariable{{Name: "token", Path: ".token"}}, + }, + result: api.HTTPRequestResult{ + StatusCode: 200, + BodyString: `{"message":"missing token"}`, + Variables: map[string]string{"token": "stale-token"}, + }, + }, + { + name: "response header variable", + request: api.CLIStepHTTPRequest{ + Tests: []api.HTTPRequestTest{{StatusCode: intPtr(200)}}, + ResponseHeaderVariables: []api.HTTPRequestResponseHeaderVariable{{ + Name: "requestID", + Header: "X-Request-ID", + }}, + }, + result: api.HTTPRequestResult{ + StatusCode: 200, + ResponseHeaders: map[string]string{}, + Variables: map[string]string{"requestID": "stale-request-id"}, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + data := api.CLIData{Steps: []api.CLIStep{{HTTPRequest: &tt.request}}} + results := []api.CLIStepResult{{HTTPRequestResult: &tt.result}} + + event := LocalSubmissionEvent(data, results) + if event.ResultSlug != api.VerificationResultSlugFailure { + t.Fatalf("ResultSlug = %q, want failure", event.ResultSlug) + } + if event.StructuredErrCLI == nil { + t.Fatal("expected structured failure") + } + if event.StructuredErrCLI.FailedStepIndex != 0 || event.StructuredErrCLI.FailedTestIndex != 1 { + t.Fatalf("failure = %#v, want step 0 capture test 1", event.StructuredErrCLI) + } + }) + } +} + +func TestLocalSubmissionEventAcceptsEmptyHTTPResponseCapture(t *testing.T) { + request := api.CLIStepHTTPRequest{ + Tests: []api.HTTPRequestTest{{StatusCode: intPtr(200)}}, + ResponseVariables: []api.HTTPRequestResponseVariable{{ + Name: "nextPath", + BodyRegex: `next="([^"]*)"`, + }}, + } + data := api.CLIData{Steps: []api.CLIStep{{HTTPRequest: &request}}} + results := []api.CLIStepResult{{HTTPRequestResult: &api.HTTPRequestResult{ + StatusCode: 200, + BodyString: `next=""`, + Variables: map[string]string{"nextPath": ""}, + }}} + + event := LocalSubmissionEvent(data, results) + if event.ResultSlug != api.VerificationResultSlugSuccess { + t.Fatalf("ResultSlug = %q, want success; failure = %#v", event.ResultSlug, event.StructuredErrCLI) + } +} + func TestValuesEqualPreservesTypes(t *testing.T) { tests := []struct { name string diff --git a/checks/runner.go b/checks/runner.go index fc080d9..c41e152 100644 --- a/checks/runner.go +++ b/checks/runner.go @@ -11,8 +11,14 @@ import ( "github.com/spf13/cobra" ) +const lessonHTTPRequestTimeout = 30 * time.Second + +func newLessonHTTPClient() *http.Client { + return &http.Client{Timeout: lessonHTTPRequestTimeout} +} + func CLIChecks(cliData api.CLIData, overrideBaseURL string, ch chan tea.Msg) (results []api.CLIStepResult) { - client := &http.Client{} + client := newLessonHTTPClient() results = make([]api.CLIStepResult, len(cliData.Steps)) if cliData.BaseURLDefault == api.BaseURLOverrideRequired && overrideBaseURL == "" { diff --git a/client/auth.go b/client/auth.go index 000a946..30182f3 100644 --- a/client/auth.go +++ b/client/auth.go @@ -6,11 +6,18 @@ import ( "fmt" "io" "net/http" + "time" "github.com/goccy/go-json" "github.com/spf13/viper" ) +const apiRequestTimeout = 30 * time.Second + +var apiHTTPClient = &http.Client{ + Timeout: apiRequestTimeout, +} + type LoginRequest struct { Otp string `json:"otp"` } @@ -26,13 +33,12 @@ type CurrentUserResponse struct { func FetchAccessToken() (*LoginResponse, error) { apiURL := viper.GetString("api_url") - client := &http.Client{} r, err := http.NewRequest("POST", apiURL+"/v1/auth/refresh", bytes.NewBuffer([]byte{})) if err != nil { return nil, err } r.Header.Add("X-Refresh-Token", viper.GetString("refresh_token")) - resp, err := client.Do(r) + resp, err := apiHTTPClient.Do(r) if err != nil { return nil, err } @@ -59,7 +65,7 @@ func LoginWithCode(code string) (*LoginResponse, error) { return nil, err } - resp, err := http.Post(apiURL+"/v1/auth/otp/login", "application/json", bytes.NewReader(req)) + resp, err := apiHTTPClient.Post(apiURL+"/v1/auth/otp/login", "application/json", bytes.NewReader(req)) if err != nil { return nil, err } @@ -102,6 +108,23 @@ func FetchCurrentUser() (*CurrentUserResponse, error) { return &user, nil } +func Logout() error { + apiURL := viper.GetString("api_url") + r, err := http.NewRequest("POST", apiURL+"/v1/auth/logout", bytes.NewBuffer([]byte{})) + if err != nil { + return err + } + r.Header.Add("X-Refresh-Token", viper.GetString("refresh_token")) + + resp, err := apiHTTPClient.Do(r) + if err != nil { + return err + } + defer resp.Body.Close() + _, _ = io.Copy(io.Discard, resp.Body) + return nil +} + func fetchWithAuth(method string, url string) ([]byte, error) { body, code, err := fetchWithAuthAndPayload(method, url, []byte{}) if err != nil { @@ -118,14 +141,13 @@ func fetchWithAuth(method string, url string) ([]byte, error) { func fetchWithAuthAndPayload(method string, url string, payload []byte) ([]byte, int, error) { apiURL := viper.GetString("api_url") - client := &http.Client{} r, err := http.NewRequest(method, apiURL+url, bytes.NewBuffer(payload)) if err != nil { return nil, 0, err } r.Header.Add("Authorization", "Bearer "+viper.GetString("access_token")) - resp, err := client.Do(r) + resp, err := apiHTTPClient.Do(r) if err != nil { return nil, 0, err } diff --git a/client/auth_test.go b/client/auth_test.go new file mode 100644 index 0000000..4c5d7ce --- /dev/null +++ b/client/auth_test.go @@ -0,0 +1,60 @@ +package api + +import ( + "errors" + "net" + "net/http" + "testing" + "time" + + "github.com/spf13/viper" +) + +type roundTripFunc func(*http.Request) (*http.Response, error) + +func (f roundTripFunc) RoundTrip(r *http.Request) (*http.Response, error) { + return f(r) +} + +func TestFetchAccessTokenHasOverallTimeout(t *testing.T) { + originalClient := apiHTTPClient + originalAPIURL := viper.GetString("api_url") + originalRefreshToken := viper.GetString("refresh_token") + t.Cleanup(func() { + apiHTTPClient = originalClient + viper.Set("api_url", originalAPIURL) + viper.Set("refresh_token", originalRefreshToken) + }) + + const timeout = 20 * time.Millisecond + apiHTTPClient = &http.Client{ + Timeout: timeout, + Transport: roundTripFunc(func(r *http.Request) (*http.Response, error) { + <-r.Context().Done() + return nil, r.Context().Err() + }), + } + viper.Set("api_url", "http://api.example") + viper.Set("refresh_token", "refresh-token") + + start := time.Now() + _, err := FetchAccessToken() + elapsed := time.Since(start) + if err == nil { + t.Fatal("FetchAccessToken() unexpectedly succeeded") + } + + var netErr net.Error + if !errors.As(err, &netErr) || !netErr.Timeout() { + t.Fatalf("FetchAccessToken() error = %v, want timeout error", err) + } + if elapsed > time.Second { + t.Fatalf("FetchAccessToken() took %v, want a prompt timeout", elapsed) + } +} + +func TestAPIHTTPClientUsesConfiguredTimeout(t *testing.T) { + if apiHTTPClient.Timeout != apiRequestTimeout { + t.Fatalf("API client timeout = %v, want %v", apiHTTPClient.Timeout, apiRequestTimeout) + } +} diff --git a/client/lessons.go b/client/lessons.go index 481e452..4865887 100644 --- a/client/lessons.go +++ b/client/lessons.go @@ -185,6 +185,7 @@ type CLICommandResult struct { FinalCommand string `json:"-"` Command CLIStepCLICommand `json:"-"` Stdout string + Stderr string `json:"-"` Variables map[string]string JqOutputs []CLICommandJqOutput `json:"-"` } @@ -196,7 +197,7 @@ type CLICommandJqOutput struct { } type HTTPRequestResult struct { - Err string `json:"-"` + Err string `json:"FetchErr,omitempty"` StatusCode int ResponseHeaders map[string]string ResponseTrailers map[string]string diff --git a/client/lessons_test.go b/client/lessons_test.go new file mode 100644 index 0000000..7b6fd12 --- /dev/null +++ b/client/lessons_test.go @@ -0,0 +1,59 @@ +package api + +import ( + "strings" + "testing" + + "github.com/goccy/go-json" +) + +func TestCLICommandResultOmitsStderrFromJSON(t *testing.T) { + payload, err := json.Marshal(CLICommandResult{ + Stdout: "submitted output", + Stderr: "local diagnostic", + }) + if err != nil { + t.Fatalf("marshal CLI command result: %v", err) + } + + if strings.Contains(string(payload), "local diagnostic") { + t.Fatalf("submission payload unexpectedly contains stderr: %s", payload) + } + if !strings.Contains(string(payload), "submitted output") { + t.Fatalf("submission payload is missing stdout: %s", payload) + } +} + +func TestHTTPRequestResultSerializesFetchError(t *testing.T) { + const fetchErr = "Failed to fetch: connection refused" + + payload, err := json.Marshal(HTTPRequestResult{Err: fetchErr}) + if err != nil { + t.Fatalf("marshal HTTP request result: %v", err) + } + + var submitted struct { + FetchErr *string + } + if err := json.Unmarshal(payload, &submitted); err != nil { + t.Fatalf("unmarshal submission payload: %v", err) + } + if submitted.FetchErr == nil || *submitted.FetchErr != fetchErr { + t.Fatalf("FetchErr = %v, want %q; payload: %s", submitted.FetchErr, fetchErr, payload) + } +} + +func TestHTTPRequestResultOmitsEmptyFetchError(t *testing.T) { + payload, err := json.Marshal(HTTPRequestResult{}) + if err != nil { + t.Fatalf("marshal HTTP request result: %v", err) + } + + var submitted map[string]any + if err := json.Unmarshal(payload, &submitted); err != nil { + t.Fatalf("unmarshal submission payload: %v", err) + } + if _, ok := submitted["FetchErr"]; ok { + t.Fatalf("submission payload unexpectedly contains an empty FetchErr: %s", payload) + } +} diff --git a/cmd/login.go b/cmd/login.go index a83eed6..6366e62 100644 --- a/cmd/login.go +++ b/cmd/login.go @@ -2,13 +2,17 @@ package cmd import ( "bufio" + "context" _ "embed" "errors" "fmt" "io" + "net" "net/http" + "net/url" "os" "regexp" + "sync" "time" api "github.com/bootdotdev/bootdev/client" @@ -51,7 +55,7 @@ var loginCmd = &cobra.Command{ fmt.Println("Please navigate to:\n" + loginURL) - inputChan := make(chan string) + inputChan := make(chan string, 1) go func() { reader := bufio.NewReader(os.Stdin) @@ -60,9 +64,8 @@ var loginCmd = &cobra.Command{ inputChan <- text }() - go func() { - startHTTPServer(inputChan) - }() + stopHTTPServer := startHTTPServer(inputChan) + defer stopHTTPServer() // attempt to open the browser go func() { @@ -73,6 +76,7 @@ var loginCmd = &cobra.Command{ // race the web server against the user's input text := <-inputChan + stopHTTPServer() re := regexp.MustCompile(`[^A-Za-z0-9_-]`) text = re.ReplaceAllString(text, "") @@ -99,21 +103,64 @@ var loginCmd = &cobra.Command{ }, } -func cors(next http.Handler) http.Handler { +const maxLoginCodeBytes = 4 * 1024 + +func cors(allowedOrigin string, next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if allowedOrigin == "" || r.Header.Get("Origin") != allowedOrigin { + http.Error(w, "origin not allowed", http.StatusForbidden) + return + } + w.Header().Set("Access-Control-Allow-Methods", "GET, POST, OPTIONS") - w.Header().Set("Access-Control-Allow-Origin", "*") + w.Header().Set("Access-Control-Allow-Origin", allowedOrigin) + w.Header().Set("Access-Control-Allow-Headers", "Content-Type") + w.Header().Set("Vary", "Origin") + if r.Method == http.MethodOptions { + w.WriteHeader(http.StatusNoContent) + return + } next.ServeHTTP(w, r) }) } -func startHTTPServer(inputChan chan string) { +func frontendOrigin(frontendURL string) string { + parsed, err := url.Parse(frontendURL) + if err != nil || parsed.Scheme == "" || parsed.Host == "" { + return "" + } + return parsed.Scheme + "://" + parsed.Host +} + +func newLoginHTTPHandler(inputChan chan<- string, frontendURL string) http.Handler { + mux := http.NewServeMux() + allowedOrigin := frontendOrigin(frontendURL) + handleSubmit := func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodPost { + w.Header().Set("Allow", "POST, OPTIONS") + http.Error(w, "method not allowed", http.StatusMethodNotAllowed) + return + } + + r.Body = http.MaxBytesReader(w, r.Body, maxLoginCodeBytes) code, err := io.ReadAll(r.Body) if err != nil { + var maxBytesErr *http.MaxBytesError + if errors.As(err, &maxBytesErr) { + http.Error(w, "login code too large", http.StatusRequestEntityTooLarge) + return + } + http.Error(w, "failed to read login code", http.StatusBadRequest) return } - inputChan <- string(code) + select { + case inputChan <- string(code): + default: + http.Error(w, "login code already received", http.StatusConflict) + return + } + w.WriteHeader(http.StatusNoContent) // Clear current line fmt.Print("\n\033[1A\033[K") } @@ -123,16 +170,42 @@ func startHTTPServer(inputChan chan string) { } handleRedirect := func(w http.ResponseWriter, r *http.Request) { - loginURL := viper.GetString("frontend_url") + "/cli/login" + loginURL := frontendURL + "/cli/login" http.Redirect(w, r, loginURL, http.StatusSeeOther) } - http.Handle("POST /submit", cors(http.HandlerFunc(handleSubmit))) - http.Handle("/health", cors(http.HandlerFunc(handleHealth))) - http.Handle("/{$}", cors(http.HandlerFunc(handleRedirect))) + mux.Handle("/submit", cors(allowedOrigin, http.HandlerFunc(handleSubmit))) + mux.Handle("/health", cors(allowedOrigin, http.HandlerFunc(handleHealth))) + mux.Handle("/{$}", http.HandlerFunc(handleRedirect)) + return mux +} + +func startHTTPServer(inputChan chan<- string) func() { + server := &http.Server{ + Handler: newLoginHTTPHandler(inputChan, viper.GetString("frontend_url")), + ReadHeaderTimeout: 5 * time.Second, + ReadTimeout: 5 * time.Second, + WriteTimeout: 5 * time.Second, + IdleTimeout: 5 * time.Second, + } - // if we fail, oh well. we fall back to entering the code - _ = http.ListenAndServe("localhost:9417", nil) + listener, err := net.Listen("tcp", "localhost:9417") + if err != nil { + return func() {} + } + + go func() { + _ = server.Serve(listener) + }() + + var stopOnce sync.Once + return func() { + stopOnce.Do(func() { + ctx, cancel := context.WithTimeout(context.Background(), time.Second) + defer cancel() + _ = server.Shutdown(ctx) + }) + } } func init() { diff --git a/cmd/login_test.go b/cmd/login_test.go new file mode 100644 index 0000000..2c06016 --- /dev/null +++ b/cmd/login_test.go @@ -0,0 +1,82 @@ +package cmd + +import ( + "net/http" + "net/http/httptest" + "strings" + "testing" +) + +const testFrontendURL = "https://boot.dev" + +func TestLoginHTTPHandlerAcceptsConfiguredOrigin(t *testing.T) { + inputChan := make(chan string, 1) + handler := newLoginHTTPHandler(inputChan, testFrontendURL) + request := httptest.NewRequest(http.MethodPost, "/submit", strings.NewReader("valid-code")) + request.Header.Set("Origin", testFrontendURL) + response := httptest.NewRecorder() + + handler.ServeHTTP(response, request) + + if response.Code != http.StatusNoContent { + t.Fatalf("status = %d, want %d", response.Code, http.StatusNoContent) + } + if code := <-inputChan; code != "valid-code" { + t.Fatalf("login code = %q, want valid-code", code) + } + if origin := response.Header().Get("Access-Control-Allow-Origin"); origin != testFrontendURL { + t.Fatalf("allowed origin = %q, want %q", origin, testFrontendURL) + } +} + +func TestLoginHTTPHandlerRejectsUnexpectedOrigin(t *testing.T) { + inputChan := make(chan string, 1) + handler := newLoginHTTPHandler(inputChan, testFrontendURL) + request := httptest.NewRequest(http.MethodPost, "/submit", strings.NewReader("attacker-code")) + request.Header.Set("Origin", "https://example.com") + response := httptest.NewRecorder() + + handler.ServeHTTP(response, request) + + if response.Code != http.StatusForbidden { + t.Fatalf("status = %d, want %d", response.Code, http.StatusForbidden) + } + select { + case code := <-inputChan: + t.Fatalf("unexpected login code accepted: %q", code) + default: + } +} + +func TestLoginHTTPHandlerRejectsMissingOrigin(t *testing.T) { + inputChan := make(chan string, 1) + handler := newLoginHTTPHandler(inputChan, testFrontendURL) + request := httptest.NewRequest(http.MethodPost, "/submit", strings.NewReader("attacker-code")) + response := httptest.NewRecorder() + + handler.ServeHTTP(response, request) + + if response.Code != http.StatusForbidden { + t.Fatalf("status = %d, want %d", response.Code, http.StatusForbidden) + } +} + +func TestLoginHTTPHandlerLimitsCodeSize(t *testing.T) { + inputChan := make(chan string, 1) + handler := newLoginHTTPHandler(inputChan, testFrontendURL) + body := strings.NewReader(strings.Repeat("a", maxLoginCodeBytes+1)) + request := httptest.NewRequest(http.MethodPost, "/submit", body) + request.Header.Set("Origin", testFrontendURL) + response := httptest.NewRecorder() + + handler.ServeHTTP(response, request) + + if response.Code != http.StatusRequestEntityTooLarge { + t.Fatalf("status = %d, want %d", response.Code, http.StatusRequestEntityTooLarge) + } + select { + case code := <-inputChan: + t.Fatalf("unexpected login code accepted: %q", code) + default: + } +} diff --git a/cmd/logout.go b/cmd/logout.go index 2311470..4b9c433 100644 --- a/cmd/logout.go +++ b/cmd/logout.go @@ -1,39 +1,38 @@ package cmd import ( - "bytes" "fmt" - "net/http" - "time" + api "github.com/bootdotdev/bootdev/client" "github.com/spf13/cobra" "github.com/spf13/viper" ) -func logout() { - apiURL := viper.GetString("api_url") - client := &http.Client{} - // Best effort - logout should never fail - r, _ := http.NewRequest("POST", apiURL+"/v1/auth/logout", bytes.NewBuffer([]byte{})) - r.Header.Add("X-Refresh-Token", viper.GetString("refresh_token")) - client.Do(r) +func logout() error { + // Server logout is best effort. Local credentials should still be cleared + // when the token is expired or the API is unavailable. + if viper.GetString("refresh_token") != "" { + _ = api.Logout() + } viper.Set("access_token", "") viper.Set("refresh_token", "") - viper.Set("last_refresh", time.Now().Unix()) - viper.WriteConfig() + viper.Set("last_refresh", 0) + if err := viper.WriteConfig(); err != nil { + return fmt.Errorf("failed to clear stored credentials: %w", err) + } + fmt.Println("Logged out successfully.") + return nil } var logoutCmd = &cobra.Command{ Use: "logout", Aliases: []string{"signout"}, Short: "Disconnect the CLI from your account", - PreRun: requireAuth, SilenceUsage: true, RunE: func(cmd *cobra.Command, args []string) error { - logout() - return nil + return logout() }, } diff --git a/cmd/logout_test.go b/cmd/logout_test.go new file mode 100644 index 0000000..20bd45e --- /dev/null +++ b/cmd/logout_test.go @@ -0,0 +1,87 @@ +package cmd + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/spf13/viper" +) + +func preserveLogoutConfig(t *testing.T) { + t.Helper() + + originalConfigFile := viper.ConfigFileUsed() + originalAPIURL := viper.GetString("api_url") + originalAccessToken := viper.GetString("access_token") + originalRefreshToken := viper.GetString("refresh_token") + originalLastRefresh := viper.GetInt64("last_refresh") + t.Cleanup(func() { + viper.Set("api_url", originalAPIURL) + viper.Set("access_token", originalAccessToken) + viper.Set("refresh_token", originalRefreshToken) + viper.Set("last_refresh", originalLastRefresh) + viper.SetConfigFile(originalConfigFile) + }) +} + +func TestLogoutClearsLocalCredentialsWhenServerLogoutFails(t *testing.T) { + preserveLogoutConfig(t) + + configPath := filepath.Join(t.TempDir(), "config.yaml") + if err := os.WriteFile(configPath, []byte("{}\n"), 0o600); err != nil { + t.Fatalf("create test config: %v", err) + } + viper.SetConfigFile(configPath) + if err := viper.ReadInConfig(); err != nil { + t.Fatalf("read test config: %v", err) + } + viper.Set("api_url", "://invalid-api-url") + viper.Set("access_token", "access-token") + viper.Set("refresh_token", "refresh-token") + viper.Set("last_refresh", int64(123)) + + if err := logout(); err != nil { + t.Fatalf("logout() error = %v", err) + } + + stored := viper.New() + stored.SetConfigFile(configPath) + if err := stored.ReadInConfig(); err != nil { + t.Fatalf("read persisted config: %v", err) + } + if got := stored.GetString("access_token"); got != "" { + t.Fatalf("persisted access token = %q, want empty", got) + } + if got := stored.GetString("refresh_token"); got != "" { + t.Fatalf("persisted refresh token = %q, want empty", got) + } + if got := stored.GetInt64("last_refresh"); got != 0 { + t.Fatalf("persisted last refresh = %d, want 0", got) + } +} + +func TestLogoutReturnsConfigWriteError(t *testing.T) { + preserveLogoutConfig(t) + + configPath := filepath.Join(t.TempDir(), "missing", "config.yaml") + viper.SetConfigFile(configPath) + viper.Set("access_token", "access-token") + viper.Set("refresh_token", "") + viper.Set("last_refresh", int64(123)) + + err := logout() + if err == nil { + t.Fatal("logout() unexpectedly succeeded") + } + if !strings.Contains(err.Error(), "failed to clear stored credentials") { + t.Fatalf("logout() error = %q, want config write error", err) + } +} + +func TestLogoutCommandDoesNotRequireAuthentication(t *testing.T) { + if logoutCmd.PreRun != nil { + t.Fatal("logout command unexpectedly requires authentication") + } +} diff --git a/cmd/root.go b/cmd/root.go index 91a541d..4710335 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -15,11 +15,16 @@ import ( "github.com/spf13/viper" ) -var cfgFile string +var ( + cfgFile string + fetchAccessToken = api.FetchAccessToken + fetchUpdateInfo = version.FetchUpdateInfo +) var rootCmd = &cobra.Command{ - Use: "bootdev", - Short: "Official Boot.dev CLI", + Use: "bootdev", + Short: "Official Boot.dev CLI", + PersistentPreRun: loadVersionInfo, Long: `The official CLI for Boot.dev. This program is meant as a companion app (not a replacement) for the website.`, } @@ -28,12 +33,20 @@ as a companion app (not a replacement) for the website.`, // This is called by main.main(). It only needs to happen once to the rootCmd. func Execute(currentVersion string) error { rootCmd.Version = currentVersion - info := version.FetchUpdateInfo(rootCmd.Version) + info := version.VersionInfo{CurrentVersion: currentVersion} defer info.PromptUpdateIfAvailable() ctx := version.WithContext(context.Background(), &info) return rootCmd.ExecuteContext(ctx) } +func loadVersionInfo(cmd *cobra.Command, args []string) { + info := version.FromContext(cmd.Context()) + if info == nil { + return + } + *info = fetchUpdateInfo(cmd.Root().Version) +} + func init() { cobra.OnInitialize(initConfig) rootCmd.PersistentFlags().StringVar(&cfgFile, "config", "", "config file (default $HOME/.bootdev.yaml or $XDG_CONFIG_HOME/bootdev/config.yaml)") @@ -51,6 +64,7 @@ func readViperConfig(paths []string) error { // initConfig reads in config file and ENV variables if set. func initConfig() { + viper.SetConfigPermissions(0o600) viper.SetDefault("frontend_url", "https://boot.dev") viper.SetDefault("api_url", "https://api.boot.dev") viper.SetDefault("access_token", "") @@ -100,10 +114,22 @@ func initConfig() { } } + cobra.CheckErr(secureConfigFile(viper.ConfigFileUsed())) + viper.SetEnvPrefix("bd") viper.AutomaticEnv() // read in environment variables that match } +func secureConfigFile(path string) error { + if path == "" { + return nil + } + if err := os.Chmod(path, 0o600); err != nil { + return fmt.Errorf("secure config file permissions: %w", err) + } + return nil +} + // Chain multiple commands together. func compose(commands ...func(cmd *cobra.Command, args []string)) func(cmd *cobra.Command, args []string) { return func(cmd *cobra.Command, args []string) { @@ -159,6 +185,25 @@ func promptToContinue(title string, message string, prompt string) bool { return response == "y" || response == "yes" } +func refreshCredentials() error { + creds, err := fetchAccessToken() + if err != nil { + return err + } + if creds == nil || creds.AccessToken == "" || creds.RefreshToken == "" { + return fmt.Errorf("invalid credentials received") + } + + viper.Set("access_token", creds.AccessToken) + viper.Set("refresh_token", creds.RefreshToken) + viper.Set("last_refresh", time.Now().Unix()) + + if err := viper.WriteConfig(); err != nil { + return fmt.Errorf("save refreshed credentials: %w", err) + } + return nil +} + // Call this function at the beginning of a command handler // if you need to make authenticated requests. This will // automatically refresh the tokens, if necessary, and prompt @@ -181,16 +226,5 @@ func requireAuth(cmd *cobra.Command, args []string) { return } - creds, err := api.FetchAccessToken() - promptLoginAndExitIf(err != nil) - if creds.AccessToken == "" || creds.RefreshToken == "" { - promptLoginAndExitIf(err != nil) - } - - viper.Set("access_token", creds.AccessToken) - viper.Set("refresh_token", creds.RefreshToken) - viper.Set("last_refresh", time.Now().Unix()) - - err = viper.WriteConfig() - promptLoginAndExitIf(err != nil) + promptLoginAndExitIf(refreshCredentials() != nil) } diff --git a/cmd/root_test.go b/cmd/root_test.go new file mode 100644 index 0000000..c38fb59 --- /dev/null +++ b/cmd/root_test.go @@ -0,0 +1,159 @@ +package cmd + +import ( + "context" + "io" + "os" + "path/filepath" + "runtime" + "strings" + "testing" + + api "github.com/bootdotdev/bootdev/client" + "github.com/bootdotdev/bootdev/version" + "github.com/spf13/cobra" + "github.com/spf13/viper" +) + +func TestSecureConfigFileRestrictsExistingFilePermissions(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("Windows does not support Unix file permission semantics") + } + + path := filepath.Join(t.TempDir(), "config.yaml") + if err := os.WriteFile(path, []byte("access_token: secret\n"), 0o644); err != nil { + t.Fatalf("create config file: %v", err) + } + if err := os.Chmod(path, 0o644); err != nil { + t.Fatalf("set initial config permissions: %v", err) + } + + if err := secureConfigFile(path); err != nil { + t.Fatalf("secureConfigFile() error = %v", err) + } + + info, err := os.Stat(path) + if err != nil { + t.Fatalf("stat config file: %v", err) + } + if got := info.Mode().Perm(); got != 0o600 { + t.Fatalf("config permissions = %o, want 600", got) + } +} + +func TestSecureConfigFileAllowsEmptyPath(t *testing.T) { + if err := secureConfigFile(""); err != nil { + t.Fatalf("secureConfigFile() error = %v", err) + } +} + +func TestExecuteSkipsVersionLookupForHelp(t *testing.T) { + originalFetch := fetchUpdateInfo + originalOut := rootCmd.OutOrStdout() + originalErr := rootCmd.ErrOrStderr() + t.Cleanup(func() { + fetchUpdateInfo = originalFetch + rootCmd.SetArgs(nil) + rootCmd.SetOut(originalOut) + rootCmd.SetErr(originalErr) + }) + + called := false + fetchUpdateInfo = func(currentVersion string) version.VersionInfo { + called = true + return version.VersionInfo{CurrentVersion: currentVersion} + } + rootCmd.SetArgs([]string{"--help"}) + rootCmd.SetOut(io.Discard) + rootCmd.SetErr(io.Discard) + + if err := Execute("v1.2.3"); err != nil { + t.Fatalf("Execute() error = %v", err) + } + if called { + t.Fatal("help unexpectedly triggered a version lookup") + } +} + +func TestLoadVersionInfoPopulatesCommandContext(t *testing.T) { + originalFetch := fetchUpdateInfo + t.Cleanup(func() { + fetchUpdateInfo = originalFetch + }) + + fetchUpdateInfo = func(currentVersion string) version.VersionInfo { + return version.VersionInfo{ + CurrentVersion: currentVersion, + LatestVersion: "v1.2.4", + IsOutdated: true, + } + } + + info := version.VersionInfo{} + cmd := &cobra.Command{Version: "v1.2.3"} + cmd.SetContext(version.WithContext(context.Background(), &info)) + + loadVersionInfo(cmd, nil) + + if info.CurrentVersion != "v1.2.3" || info.LatestVersion != "v1.2.4" || !info.IsOutdated { + t.Fatalf("version context was not populated: %#v", info) + } +} + +func TestRefreshCredentialsPersistsRotatedCredentials(t *testing.T) { + const ( + oldAccessToken = "old-access-token" + oldRefreshToken = "old-refresh-token" + newAccessToken = "new-access-token" + newRefreshToken = "new-refresh-token" + ) + + originalFetchAccessToken := fetchAccessToken + originalConfigFile := viper.ConfigFileUsed() + originalAccessToken := viper.GetString("access_token") + originalRefreshToken := viper.GetString("refresh_token") + originalLastRefresh := viper.GetInt64("last_refresh") + t.Cleanup(func() { + fetchAccessToken = originalFetchAccessToken + viper.Set("access_token", originalAccessToken) + viper.Set("refresh_token", originalRefreshToken) + viper.Set("last_refresh", originalLastRefresh) + viper.SetConfigFile(originalConfigFile) + }) + fetchAccessToken = func() (*api.LoginResponse, error) { + return &api.LoginResponse{ + AccessToken: newAccessToken, + RefreshToken: newRefreshToken, + }, nil + } + + configPath := filepath.Join(t.TempDir(), "config.yaml") + if err := os.WriteFile(configPath, []byte("{}\n"), 0o600); err != nil { + t.Fatalf("create test config: %v", err) + } + viper.SetConfigFile(configPath) + if err := viper.ReadInConfig(); err != nil { + t.Fatalf("read test config: %v", err) + } + viper.Set("access_token", oldAccessToken) + viper.Set("refresh_token", oldRefreshToken) + + if err := refreshCredentials(); err != nil { + t.Fatalf("refreshCredentials() error = %v", err) + } + + if got := viper.GetString("access_token"); got != newAccessToken { + t.Fatalf("access token = %q, want %q", got, newAccessToken) + } + if got := viper.GetString("refresh_token"); got != newRefreshToken { + t.Fatalf("refresh token = %q, want %q", got, newRefreshToken) + } + + config, err := os.ReadFile(configPath) + if err != nil { + t.Fatalf("read persisted config: %v", err) + } + if !strings.Contains(string(config), newAccessToken) || !strings.Contains(string(config), newRefreshToken) { + t.Fatalf("persisted config does not contain rotated credentials:\n%s", config) + } +} diff --git a/cmd/status.go b/cmd/status.go index 64a9a17..6dc5781 100644 --- a/cmd/status.go +++ b/cmd/status.go @@ -28,9 +28,8 @@ func checkAuthStatus() { return } - // Verify token is still valid by attempting to refresh - _, err := api.FetchAccessToken() - if err != nil { + // Verify the token is still valid and persist any rotated credentials. + if err := refreshCredentials(); err != nil { fmt.Println("Authentication expired") fmt.Println("Run 'bootdev login' to re-authenticate") return diff --git a/render/view.go b/render/view.go index b340e73..b6fa82b 100644 --- a/render/view.go +++ b/render/view.go @@ -220,6 +220,11 @@ func renderStepResult(step stepModel) string { str.WriteString(" > Command stdout:\n\n") str.WriteString(gray.Render(truncateVisualOutput(step.result.CLICommandResult.Stdout))) str.WriteByte('\n') + if step.result.CLICommandResult.Stderr != "" { + str.WriteString(" > Command stderr:\n\n") + str.WriteString(gray.Render(truncateVisualOutput(step.result.CLICommandResult.Stderr))) + str.WriteByte('\n') + } str.WriteString(renderJqOutputs(step.result.CLICommandResult.JqOutputs)) availableVariables, expectsVariables := availableVariablesForCLIResult(*step.result.CLICommandResult) if expectsVariables { diff --git a/render/view_test.go b/render/view_test.go index 1cc946f..9f8551c 100644 --- a/render/view_test.go +++ b/render/view_test.go @@ -105,6 +105,7 @@ func TestVerboseViewShowsSuccessfulDetails(t *testing.T) { tests: []testModel{{text: "Expect stdout to contain all of: hello", passed: &passed, finished: true}}, result: &api.CLIStepResult{CLICommandResult: &api.CLICommandResult{ Stdout: "hello", + Stderr: "diagnostic message", }}, }} @@ -115,6 +116,8 @@ func TestVerboseViewShowsSuccessfulDetails(t *testing.T) { "Expect stdout to contain all of: hello", "Command stdout:", "hello", + "Command stderr:", + "diagnostic message", } { if !strings.Contains(view, expected) { t.Errorf("view missing %q\n%s", expected, view) diff --git a/version/version.go b/version/version.go index ad4ed56..830f5f5 100644 --- a/version/version.go +++ b/version/version.go @@ -1,21 +1,19 @@ package version import ( + "context" "fmt" - "io" - "net/http" "os" "os/exec" - "slices" - "strings" + "time" "github.com/goccy/go-json" "golang.org/x/mod/semver" ) const ( - repoOwner = "bootdotdev" - repoName = "bootdev" + modulePath = "github.com/bootdotdev/bootdev" + updateCheckTimeout = 10 * time.Second ) type VersionInfo struct { @@ -30,7 +28,8 @@ func FetchUpdateInfo(currentVersion string) VersionInfo { latest, err := getLatestVersion() if err != nil { return VersionInfo{ - FailedToFetch: err, + CurrentVersion: currentVersion, + FailedToFetch: err, } } isUpdateRequired := isUpdateRequired(currentVersion, latest) @@ -68,45 +67,28 @@ func isUpdateRequired(current string, latest string) bool { } func getLatestVersion() (string, error) { - goproxyDefault := "https://proxy.golang.org" - goproxy := goproxyDefault - cmd := exec.Command("go", "env", "GOPROXY") + return getLatestVersionWithTimeout(updateCheckTimeout) +} + +func getLatestVersionWithTimeout(timeout time.Duration) (string, error) { + ctx, cancel := context.WithTimeout(context.Background(), timeout) + defer cancel() + + cmd := exec.CommandContext(ctx, "go", "list", "-m", "-json", modulePath+"@latest") output, err := cmd.Output() - if err == nil { - goproxy = strings.TrimSpace(string(output)) + if ctxErr := ctx.Err(); ctxErr != nil { + return "", fmt.Errorf("latest version check failed: %w", ctxErr) } - - proxies := strings.Split(goproxy, ",") - if !slices.Contains(proxies, goproxyDefault) { - proxies = append(proxies, goproxyDefault) + if err != nil { + return "", fmt.Errorf("failed to query latest version: %w", err) } - for _, proxy := range proxies { - proxy = strings.TrimSpace(proxy) - proxy = strings.TrimRight(proxy, "/") - if proxy == "direct" || proxy == "off" { - continue - } - - url := fmt.Sprintf("%s/github.com/%s/%s/@latest", proxy, repoOwner, repoName) - resp, err := http.Get(url) - if err != nil { - continue - } - defer resp.Body.Close() - - body, err := io.ReadAll(resp.Body) - if err != nil { - continue - } - - var version struct{ Version string } - if err = json.Unmarshal(body, &version); err != nil { - continue - } - - return version.Version, nil + var latest struct{ Version string } + if err := json.Unmarshal(output, &latest); err != nil { + return "", fmt.Errorf("failed to parse latest version: %w", err) } - - return "", fmt.Errorf("failed to fetch latest version") + if latest.Version == "" { + return "", fmt.Errorf("latest version response did not include a version") + } + return latest.Version, nil } diff --git a/version/version_test.go b/version/version_test.go new file mode 100644 index 0000000..316cf2a --- /dev/null +++ b/version/version_test.go @@ -0,0 +1,64 @@ +package version + +import ( + "context" + "errors" + "os" + "path/filepath" + "runtime" + "testing" + "time" +) + +func TestGetLatestVersionRespectsGoProxyOff(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("test uses a POSIX executable script") + } + + dir := t.TempDir() + fakeGo := filepath.Join(dir, "go") + script := `#!/bin/sh +if [ "$GOPROXY" != "off" ]; then + echo "unexpected GOPROXY: $GOPROXY" >&2 + exit 1 +fi +printf '{"Version":"v1.2.3"}' +` + if err := os.WriteFile(fakeGo, []byte(script), 0o755); err != nil { + t.Fatalf("create fake go command: %v", err) + } + t.Setenv("PATH", dir) + t.Setenv("GOPROXY", "off") + t.Setenv("GOPRIVATE", "") + t.Setenv("GONOPROXY", "none") + + latest, err := getLatestVersionWithTimeout(time.Second) + if err != nil { + t.Fatalf("get latest version: %v", err) + } + if latest != "v1.2.3" { + t.Fatalf("latest version = %q, want v1.2.3", latest) + } +} + +func TestGetLatestVersionHasOverallTimeout(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("test uses a POSIX executable script") + } + + dir := t.TempDir() + fakeGo := filepath.Join(dir, "go") + if err := os.WriteFile(fakeGo, []byte("#!/bin/sh\nexec /bin/sleep 5\n"), 0o755); err != nil { + t.Fatalf("create fake go command: %v", err) + } + t.Setenv("PATH", dir) + + start := time.Now() + _, err := getLatestVersionWithTimeout(25 * time.Millisecond) + if !errors.Is(err, context.DeadlineExceeded) { + t.Fatalf("error = %v, want context deadline exceeded", err) + } + if elapsed := time.Since(start); elapsed > time.Second { + t.Fatalf("version lookup exceeded timeout allowance: %s", elapsed) + } +}