Skip to content

fix: Prevent path traversal via webhook head commit SHA - #942

Merged
o-kopysov merged 2 commits into
mainfrom
fix/webhook-sha-path-traversal
Oct 15, 2026
Merged

o-kopysov merged 2 commits into
mainfrom
fix/webhook-sha-path-traversal

Conversation

@t-naumenko

@t-naumenko t-naumenko commented Oct 13, 2026 •

Copy link
Copy Markdown
Collaborator

Pull Request

Description

The pull_request.head.sha value from GitHub webhook payloads was stored without validation and appended to file system paths with string concatenation (~/LPVS/Projects/<repo>/<id>-<sha>). LPVSFileUtil.saveGithubDiffs then recursively deleted that path, recreated it and wrote the pull request files into it. A sender with a valid webhook signature could set the SHA to a traversal string such as x/../../../../etc/cron.d to delete and create directories and write files anywhere the JVM user can write. The official Docker image ran the JVM as root, so this extended to the whole container file system. The repository name, taken from the last segment of repository.html_url, could similarly be ...

Changes:

  • Webhook payload validation (LPVSPayloadUtil): the head commit SHA must be 40 (SHA-1) or 64 (SHA-256) hexadecimal characters; the repository owner and name must contain only A-Z a-z 0-9 . _ - and must not be . or ... Invalid webhooks are rejected with 400 Bad Request and are not saved, queued or reported to GitHub (GitHubController).
  • Path containment (LPVSFileUtil): new resolvePathInside(base, relative) resolves and normalizes paths and rejects any path outside the base directory. It is applied to the project directory, scan results directory and JSON file, and to PR file names written by saveFileToDisk. This also protects queue entries already stored in the database and any future input source.
  • Removed HtmlUtils.htmlEscape from the scan results file name: it never protected against path traversal (/ and . are not escaped).
  • Docker image runs as non-root user: added the unprivileged user lpvs (uid 10001) and USER lpvs. Python and SCANOSS stay installed as root and are read-only for the application. The working directory /home/lpvs/LPVS is pre-created and owned by lpvs, so Docker volumes mounted there are writable by the application.

Single scan (/scan) and CLI modes take the SHA from the GitHub API and are covered by the path containment check.

Fixes GHSA-66xf-749c-4648

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Upgrade notes:

  • Docker: the application now runs as user lpvs (uid 10001); its working directory moved from /root/LPVS to /home/lpvs/LPVS (and the thin launcher dependency cache from /root/.m2 to /home/lpvs/.m2). This directory holds only temporary files of the running scan, and the provided docker-compose files do not mount it, so no migration is needed. Docker volumes mounted at /home/lpvs/LPVS work out of the box; host directories (bind mounts) must be writable by uid 10001.
  • Webhooks with an invalid head commit SHA or repository URL now return 400 Bad Request. Payloads sent by GitHub are not affected.

Testing

  • LPVSPayloadUtilTest: valid SHA-1, SHA-256 and uppercase SHAs are accepted; traversal, wrong-length and non-hex SHAs and invalid repository URLs (.., ., spaces, missing owner) are rejected.
  • LPVSFileUtilTest: resolvePathInside accepts paths inside the base directory and rejects .., absolute paths, the base directory itself and sibling directories with the same name prefix; directory paths with a traversal SHA or .. repository name are rejected; saveFileToDisk rejects ../ file names; end-to-end saveGithubDiffs test with a traversal SHA verifies that a directory outside the sandbox is not deleted and no file is written into it.
  • GitHubControllerTest: a correctly signed webhook with a traversal SHA returns 400 with no repository, GitHub or queue interactions.
  • Verified the tests detect the vulnerability: disabling the input validation fails the 3 validation tests; disabling the containment check fails the 5 containment tests.
  • Full test suite: mvn verify — 332 tests, 0 failures. All changed lines in LPVSPayloadUtil, LPVSFileUtil and GitHubController are covered (JaCoCo).
  • Java format: google-java-format 1.20.0 with the workflow flags reports no changes.
  • Docker: built the image and verified in a container that the process runs as lpvs (uid 10001), can write to ~/LPVS, and cannot write to /etc/cron.d, /root, Python site-packages, /usr/local/bin or lpvs.jar. A CLI single scan (singlescan profile, network disabled) started successfully as lpvs and created its project and results directories without permission errors.

Test Configuration:

  • Java: v17 (Eclipse Temurin, maven:3.9-eclipse-temurin-17)
  • LPVS Release: v2.2.5 (main)

Checklist:

  • My code follows the style guidelines of this project
  • My code meets the required code coverage for lines (90% and above)
  • My code meets the required code coverage for branches (80% and above)
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

Signed-off-by: Tetiana Naumenko <t.naumenko@samsung.com>
@t-naumenko t-naumenko added this to the v2.2.6 milestone Oct 13, 2026
@t-naumenko t-naumenko self-assigned this Oct 13, 2026
@t-naumenko t-naumenko added bug Something isn't working fix labels Oct 13, 2026
@t-naumenko
t-naumenko marked this pull request as ready for review October 13, 2026 10:46
@codecov-commenter

codecov-commenter commented Oct 13, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.11%. Comparing base (afffee3) to head (afb27a9).

Additional details and impacted files
@@             Coverage Diff              @@
##               main     #942      +/-   ##
============================================
+ Coverage     94.02%   94.11%   +0.08%     
- Complexity      443      456      +13     
============================================
  Files            30       30              
  Lines          1841     1869      +28     
  Branches        236      243       +7     
============================================
+ Hits           1731     1759      +28     
  Misses           48       48              
  Partials         62       62              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

o-kopysov
o-kopysov previously approved these changes Oct 13, 2026

@o-kopysov o-kopysov left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

…mage

Signed-off-by: Tetiana Naumenko <t.naumenko@samsung.com>

@o-kopysov o-kopysov left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

@tdrozdovsky tdrozdovsky left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

@o-kopysov
o-kopysov merged commit af1ab90 into main Oct 15, 2026
16 checks passed
@o-kopysov
o-kopysov deleted the fix/webhook-sha-path-traversal branch October 15, 2026 06:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants