do a round of refactoring & cleanup - #200
Merged
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR performs a refactor/cleanup pass across the CLI to improve reliability and safety: adding timeouts and output/body caps for external calls, tightening the local login HTTP handler, and persisting rotated credentials more consistently.
Changes:
- Add timeouts and size limits around network requests,
go-based version checks, and executed CLI commands (including capturing stderr separately). - Harden authentication/config handling (secure config file permissions, persist rotated tokens, make logout best-effort but still clear local credentials).
- Improve local verification/rendering (render stderr, validate captured HTTP response variables, safer JSON interpolation).
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| version/version.go | Switch latest-version lookup to go list -m -json with a hard timeout. |
| version/version_test.go | Adds tests for GOPROXY/timeout behavior of version lookup. |
| render/view.go | Display CLI command stderr in verbose step rendering. |
| render/view_test.go | Extends verbose view test expectations to include stderr output. |
| cmd/status.go | Use shared credential refresh helper when validating auth status. |
| cmd/root.go | Defer version lookup to a persistent pre-run, lock down config permissions, and centralize credential refresh/persistence. |
| cmd/root_test.go | Adds coverage for config file permission hardening, version lookup timing, and credential persistence. |
| cmd/logout.go | Make logout return errors on local write failures; best-effort server logout; remove auth prerequisite. |
| cmd/logout_test.go | Adds coverage for logout clearing local creds even when server logout fails, and for write errors. |
| cmd/login.go | Harden local login HTTP server: stricter CORS, body size limits, handler muxing, and clean shutdown. |
| cmd/login_test.go | Adds unit tests for login handler origin checks and code-size limits. |
| client/lessons.go | Track stderr locally (omit from JSON) and serialize HTTP fetch errors via FetchErr. |
| client/lessons_test.go | Adds JSON serialization tests for stderr omission and FetchErr behavior. |
| client/auth.go | Introduce a shared API HTTP client with timeout; add Logout() API helper. |
| client/auth_test.go | Adds timeout behavior test for FetchAccessToken and client timeout config assertion. |
| checks/runner.go | Use an HTTP client with an explicit timeout for lesson checks. |
| checks/local.go | Validate that captured HTTP response variables/headers match the response content. |
| checks/local_test.go | Adds coverage for rejecting stale/mismatched HTTP capture variables and allowing empty captures. |
| checks/http.go | Safer JSON interpolation before marshalling; cap response-body reads; reuse truncation for parsing. |
| checks/http_test.go | Adds tests for nested JSON interpolation safety, response-body cap, and timeout/client config. |
| checks/cli.go | Add execution timeout/output caps, separate stdout/stderr capture, and cancellation on excessive output. |
| checks/cli_test.go | Adds tests for CLI timeouts, output caps, bounded buffer behavior, and stderr isolation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.