Skip to content

[test-improver] Improve tests for server session path-validation#8060

Merged
lpcox merged 1 commit into
mainfrom
test-improver/session-util-path-validation-45d3e0ef42a929d0
Jun 25, 2026
Merged

[test-improver] Improve tests for server session path-validation#8060
lpcox merged 1 commit into
mainfrom
test-improver/session-util-path-validation-45d3e0ef42a929d0

Conversation

@github-actions

Copy link
Copy Markdown
Contributor

File Analyzed

  • Test File: internal/server/session_util_test.go
  • Package: internal/server
  • Lines of Code: 49 → 89 (+40 lines)

Improvements Made

1. Increased Coverage — Direct tests for isSinglePathSegmentSessionID

isSinglePathSegmentSessionID is a security-critical function that prevents path-traversal attacks on session-ID-based filesystem paths. It was previously only exercised indirectly through HTTP handler integration tests, leaving it at 66.7% coverage.

  • ✅ Added TestIsSinglePathSegmentSessionID with 15 table-driven sub-tests
  • ✅ Covers the first guard: empty string, single-dot, double-dot
  • ✅ Covers the second guard: absolute paths (/etc/passwd, /)
  • ✅ Covers the third guard: forward-slash traversal, relative traversal (../etc), current-dir prefix (./session), backslash traversal
  • ✅ Covers the happy path: simple IDs, UUID format, API key format, hex tokens, numeric strings

Coverage for isSinglePathSegmentSessionID: 66.7% → 88.9%

The remaining 11.1% is a single unreachable branch on Linux — the filepath.Base guard is defence-in-depth code that only activates on Windows-style paths, which are fully blocked by earlier checks on Linux.

2. Better Testing Patterns

  • ✅ Table-driven test using t.Run sub-tests for clear per-case failure messages
  • ✅ Descriptive sub-test names explain both input and expected outcome
  • ✅ Grouped cases by the guard they exercise (dot-special, absolute-path, path-separator, valid)
  • ✅ Consistent testify assert.Equal assertions

Test Execution

All tests pass:

=== RUN   TestIsSinglePathSegmentSessionID
--- PASS: TestIsSinglePathSegmentSessionID (0.00s)
    --- PASS: .../empty_string
    --- PASS: .../single_dot
    --- PASS: .../double_dot
    --- PASS: .../absolute_path
    --- PASS: .../root_slash
    --- PASS: .../forward_slash_traversal
    --- PASS: .../relative_traversal
    --- PASS: .../current-dir_prefix
    --- PASS: .../backslash_traversal
    --- PASS: .../simple_session_ID
    --- PASS: .../UUID_format
    --- PASS: .../API_key_format
    --- PASS: .../hex_token
    --- PASS: .../single_character
    --- PASS: .../numeric_string
ok  github.com/github/gh-aw-mcpg/internal/server  5.3s

Overall package coverage: 93.6% → 93.8%

Why These Changes?

isSinglePathSegmentSessionID guards session IDs used to construct filesystem paths under the payload directory. Without direct tests, regressions in this logic (which prevents path-traversal attacks) would only be caught by indirect HTTP handler tests that may not exercise all the security-relevant branches. Direct table-driven tests make the intent explicit and ensure each guard is independently verified.


Generated by Test Improver Workflow
Focuses on better patterns, increased coverage, and more stable tests

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • index.crates.io

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "index.crates.io"

See Network Configuration for more information.

Generated by Test Improver · 913.4 AIC · ⊞ 29.5K ·

…t.go

Add a table-driven test covering the path-traversal prevention logic in
isSinglePathSegmentSessionID, which was previously only exercised indirectly
through HTTP handler integration tests.

Newly covered cases:
- empty string, single dot, double dot (first guard)
- absolute paths like /etc/passwd and / (second guard)
- forward-slash and backslash traversal (third guard)
- valid identifiers: simple, UUID, API key, hex token, numeric

Coverage for isSinglePathSegmentSessionID: 66.7% → 88.9%
(the remaining 11.1% is an unreachable branch on Linux — the
filepath.Base guard is defence-in-depth for Windows-only inputs)

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

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 direct, table-driven unit coverage for isSinglePathSegmentSessionID, the session-ID path validation helper used by the server to prevent path traversal when constructing session-scoped filesystem paths.

Changes:

  • Introduces TestIsSinglePathSegmentSessionID with table-driven subtests covering dot-special, absolute-path, and path-separator rejection cases plus common “valid” IDs.
  • Improves security regression coverage by explicitly testing inputs like ../etc, ./session, /, and path\traversal.
Show a summary per file
File Description
internal/server/session_util_test.go Adds focused table-driven tests for session ID single-segment/path-traversal validation.

Copilot's findings

Tip

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

  • Files reviewed: 1/1 changed files
  • Comments generated: 0

@lpcox lpcox merged commit f71582a into main Jun 25, 2026
24 checks passed
@lpcox lpcox deleted the test-improver/session-util-path-validation-45d3e0ef42a929d0 branch June 25, 2026 16:11
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