Skip to content

fix: address log limits, OAuth callbacks, job pagination, and CRLF icons - #3731

Open
MauricioPerera wants to merge 1 commit into
github:mainfrom
MauricioPerera:fix/mcp-audit-hardening
Open

MauricioPerera wants to merge 1 commit into
github:mainfrom
MauricioPerera:fix/mcp-audit-hardening

Conversation

@MauricioPerera

Copy link
Copy Markdown

Summary

Make Actions log downloads cancellable and bounded, keep unrelated OAuth callbacks from aborting login, retrieve failed jobs across every page, and normalize embedded icon data across Windows and Unix line endings.

Why

Log reads could continue after their MCP call was cancelled and retain excessive text. A run whose failed job appeared after the first page could incorrectly report no failures, and unrelated requests to the local OAuth callback could terminate a pending login. Windows CRLF checkouts also changed advertised icon data and broke tool-schema snapshot comparisons.

What changed

  1. Propagate the MCP context through log downloads and enforce a 30-second timeout, including response-body reads.
  2. Bound retained log text to 1 MiB per response, including combined failed-job logs, and 16 KiB per line plus a truncation marker. Preserve recent lines and valid UTF-8; report jobs skipped after the shared budget is exhausted.
  3. Validate OAuth state before handling provider errors. Reject unrelated callbacks with HTTP 400 while keeping the legitimate authorization flow available.
  4. Follow every jobs-list page, aggregate totals, and surface failures from later pages.
  5. Parse LF and CRLF icon data consistently so Windows checkouts produce the same icon URIs and tool schemas.

Add regression coverage for cancellation before and during downloads, deadlines, byte limits and ring-buffer rollover, UTF-8 truncation, shared multi-job budgets, pagination and later-page errors, unrelated OAuth callbacks followed by valid callbacks, and LF/CRLF icon inputs. Document the log limits in docs/server-configuration.md.

MCP impact

  • Tool schema or behavior changed: get_job_logs respects cancellation and bounded output, and failed-only reads cover all pages. Tool names and input schemas remain the same.
  • New tool added

Prompts tested (tool changes only)

Validation used automated tool-handler and HTTP tests. The covered scenarios include retrieving a single job's log content, collecting all failed-job logs across pages, cancelling a pending download, and collecting enough log text to exhaust the response budget. Interactive natural-language prompts were not run.

Security / limits

  • Auth / permissions considered: only callbacks with the expected OAuth state may complete or fail the pending login.
  • Data exposure, filtering, or token/size limits considered: downloads have a 30-second deadline; retained log text is capped at 1 MiB per response. JSON encoding and job metadata add to the serialized response size.

Tool renaming

  • I am not renaming tools as part of this PR.

Lint & tests

Validated locally on Windows with Go 1.27.3, directly against the repository files without overlays or snapshot updates:

  • go test ./...: 8,584 tests and subtests passed across 30 packages with tests.
  • go test -race ./...: all 30 tested packages passed; no races reported.
  • Regression tests rerun with -race after the final test response-cleanup adjustments.
  • go vet ./...: passed.
  • Repository-pinned golangci-lint v2.14.0 via go run github.com/golangci/golangci-lint/v2/cmd/[email protected] run: 0 issues.
  • go build ./cmd/github-mcp-server with an external output path: passed.
  • gofmt and git diff --check: passed.

The embedded icon input still contained 64 CRLF line endings during validation, exercising the Windows checkout behavior.

Docs

  • Updated: document Actions log download deadlines, byte and line limits, truncation, and individual retrieval of jobs omitted by the combined budget.

Bound and cancel Actions log downloads, cap retained log text, paginate failed jobs, reject unrelated OAuth callbacks without aborting login, and handle CRLF icon data. Add regression tests and document log limits.
@MauricioPerera
MauricioPerera requested a review from a team as a code owner November 7, 2026 02:48
Copilot AI auto review requested due to automatic review settings November 7, 2026 02:48
@MauricioPerera

Copy link
Copy Markdown
Author

Could a maintainer please approve the workflow runs for this fork PR using Approve workflows to run?

All seven Actions workflows are currently awaiting maintainer approval (action_required): Build and Test Go Project, golangci-lint, Documentation Check, License Check, CodeQL, Docker, and MCP Server Diff. They have not executed yet, so there are no CI results to diagnose.

Local validation passed: 8,584 tests and subtests across 30 packages, the full race-enabled suite, go vet, repository-pinned golangci-lint v2.14.0, and the server build. Once the workflows are approved, we can inspect the CI results and address any failures.

Thank you!

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The jobs pagination loop leaves PerPage unset, resulting in unnecessary API requests and rate-limit consumption for large matrix runs.

1 open issue
What changed in this PR

Improves the GitHub MCP Server’s log retrieval and OAuth reliability, while keeping embedded icon data consistent across platforms.

Changes:

  • Add cancellable, time-limited Actions log downloads, bounded output, and failed-job pagination.
  • Reject unrelated OAuth callbacks without terminating pending authorization.
  • Normalize LF/CRLF icon inputs, add regression coverage, and document log limits.
File Description
pkg/​octicons/​octicons.go Normalize line endings when parsing embedded icon data.
pkg/​octicons/​octicons_test.go Cover LF and CRLF inputs with optional trailing newlines.
pkg/​github/​actions.go Add download deadlines, shared log budgets, and job pagination.
pkg/​github/​actions_regression_test.go Cover cancellation, deadlines, pagination, and combined output limits.
pkg/​buffer/​buffer.go Bound retained log bytes and preserve UTF-8 during truncation.
pkg/​buffer/​buffer_test.go Cover byte limits, ring rollover, and UTF-8 truncation.
internal/​oauth/​callback.go Validate state before accepting callback results or provider errors.
internal/​oauth/​callback_test.go Verify unrelated callbacks leave legitimate authorization available.
docs/​server-configuration.md Document download deadlines, truncation, and shared log limits.

Review effort: Max (auto)

Comment thread pkg/github/actions.go

// Filter for failed jobs
// Gather failures from every page before reporting that a run has none.
opts := &github.ListWorkflowJobsOptions{Filter: "latest"}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants