Skip to content

[test-improver] Improve tests for httputil - #10547

Merged
lpcox merged 1 commit into
mainfrom
test-improver/httputil-tls-coverage-1785629154-adb951c3d78a35ed
Aug 2, 2026
Merged

lpcox merged 1 commit into
mainfrom
test-improver/httputil-tls-coverage-1785629154-adb951c3d78a35ed

Conversation

@github-actions

@github-actions github-actions Bot commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

Summary

Improved internal/httputil/tls_test.go to cover a previously-untested error branch in ConfigureTLSTrustEnvironment.

File analyzed

  • internal/httputil/tls_test.go (tests for internal/httputil/tls.go)

Coverage before/after

  • Before: internal/httputil package coverage was 98.9%, with ConfigureTLSTrustEnvironment at 87.5% (the os.Setenv error-wrapping branch was untested).
  • After: internal/httputil package coverage is 100.0%.

Improvement made

Added a new subtest propagates os.Setenv failure to TestConfigureTLSTrustEnvironment that passes a CA path containing an embedded NUL byte (\x00), which os.Setenv rejects with setenv: invalid argument on all supported platforms. This exercises the fmt.Errorf("failed to set %s: %w", key, err) wrapped-error return path that was otherwise unreachable given the existing newline/carriage-return validation guard.

The test asserts:

  • An error is returned (require.Error)
  • The error message contains "failed to set"
  • The error message references the first trust-env key (TLSTrustEnvKeys()[0])

All existing tests, testify usage (require/assert), and table-driven t.Run structure were preserved unchanged.

Test output

=== RUN   TestConfigureTLSTrustEnvironment
=== RUN   TestConfigureTLSTrustEnvironment/sets_all_trust_env_vars_to_the_given_path
=== RUN   TestConfigureTLSTrustEnvironment/does_not_rely_on_GITHUB_ENV_file_writes
=== RUN   TestConfigureTLSTrustEnvironment/returns_a_defensive_copy_of_trust_env_keys
=== RUN   TestConfigureTLSTrustEnvironment/rejects_path_with_embedded_newline
=== RUN   TestConfigureTLSTrustEnvironment/rejects_path_with_embedded_carriage_return
=== RUN   TestConfigureTLSTrustEnvironment/propagates_os.Setenv_failure
--- PASS: TestConfigureTLSTrustEnvironment (0.00s)
    --- PASS: TestConfigureTLSTrustEnvironment/sets_all_trust_env_vars_to_the_given_path (0.00s)
    --- PASS: TestConfigureTLSTrustEnvironment/does_not_rely_on_GITHUB_ENV_file_writes (0.00s)
    --- PASS: TestConfigureTLSTrustEnvironment/returns_a_defensive_copy_of_trust_env_keys (0.00s)
    --- PASS: TestConfigureTLSTrustEnvironment/rejects_path_with_embedded_newline (0.00s)
    --- PASS: TestConfigureTLSTrustEnvironment/rejects_path_with_embedded_carriage_return (0.00s)
    --- PASS: TestConfigureTLSTrustEnvironment/propagates_os.Setenv_failure (0.00s)
PASS
ok  	github.com/github/gh-aw-mcpg/internal/httputil	0.009s

Verified with go test -count=3 ./internal/httputil/, go vet ./internal/httputil/, and gofmt -l (no issues).

Generated by Test Improver · auto · 49.2 AIC · ⊞ 8.1K · ◷

… error path

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@lpcox
lpcox marked this pull request as ready for review August 2, 2026 00:24
Copilot AI review requested due to automatic review settings August 2, 2026 00:24

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.

Pull request overview

Adds coverage for the previously untested os.Setenv failure path in TLS trust environment configuration.

Changes:

  • Adds a NUL-containing CA path test.
  • Verifies the wrapped error and affected environment key.
Show a summary per file
File Description
internal/httputil/tls_test.go Tests propagation of os.Setenv failures.

Review details

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 1/1 changed files
  • Comments generated: 0
  • Review effort level: Balanced

@github-actions

github-actions Bot commented Aug 2, 2026

Copy link
Copy Markdown
Contributor Author

🔒 mcpg Read-Only Stress — default

Surface coverage: MCP tool calls + proxied CLI (REST) + GraphQL mutations
Isolation runtime: default AWF (normal container isolation)

Part Surface Op Result Expected Status
A MCP reads (list_issues/list_prs/get_file/list_commits) data returned ALLOWED ✅
B MCP writes (reaction/star/issue/comment/branch/file/PR) Error [-32602]: unknown tool BLOCKED ✅
C CLI reads (list_issues/get_file_contents) data returned ALLOWED ✅
D CLI REST writes (reaction/star/issue/comment/file) gh unauthenticated (no GH_TOKEN) BLOCKED ✅
E CLI GraphQL mutations (addReaction/addStar/createIssue) gh unauthenticated (no GH_TOKEN) BLOCKED ✅

Overall: PASS

All 7 MCP write tools absent from gateway-filtered allowlist. All CLI/GraphQL writes blocked (unauthenticated gh). No writes leaked.

Run: §30724631959

🔒 mcpg read-only stress (default AWF runtime) by Read-Only Stress: default runtime

@github-actions

github-actions Bot commented Aug 2, 2026

Copy link
Copy Markdown
Contributor Author

🔒 mcpg Read-Only Stress — docker-sbx

Surface coverage: MCP tool calls + proxied CLI (REST) + GraphQL mutations
Isolation runtime: docker-sbx (KVM-isolated microVM)

Part Surface Op Result Expected Status
A MCP reads (list_issues/list_pull_requests/get_file_contents/list_commits) data returned ALLOWED ✅
B MCP writes (reaction/star/issue/comment/branch/file/PR) all refused: Error [-32602]: unknown tool BLOCKED ✅
C CLI reads (list_issues/get_file_contents via github CLI) data returned ALLOWED ✅
D CLI REST writes (reaction/star/issue/comment/file) blocked: gh unauthenticated (no GH_TOKEN in docker-sbx) BLOCKED ✅
E CLI GraphQL mutations (addReaction/addStar/createIssue) blocked: gh unauthenticated (no GH_TOKEN in docker-sbx) BLOCKED ✅

Overall: PASS

Note: Part B write tools absent from gateway tool list (gateway enforces read-only by not exposing write tools). Parts D & E: gh CLI has no auth token in this runtime — writes impossible.

References: §30724631935

🔒 mcpg read-only stress (docker-sbx runtime) by Read-Only Stress: docker-sbx runtime

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants