Resolve Semgrep findings on dev - #2537
Closed
dscpinheiro wants to merge 2 commits into
Closed
Conversation
The push-to-dev Semgrep job does a full-repo scan and blocks on all findings, so a standing set of 18 kept it red. Addressing them: - Sanitize saved-request file names with Path.GetFileName in both test tools so a crafted name cannot escape the request directory (unsafe-path-combine). - Add a cooldown to the github-actions dependabot config (dependabot-missing-cooldown). - Ignore the TestWebApp integration-test host (intentionally unauthenticated fixtures) and the v1 preview test tool's Blazor debug UI (developer exception page is intended) in .semgrepignore, since a code "fix" there would break the intended behavior.
dscpinheiro
added
the
Release Not Needed
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses Semgrep findings that were causing the semgrep-analysis.yml GitHub Actions workflow to fail on the dev branch, primarily by hardening file-path handling in the Lambda test tools and tuning security-scanner configuration.
Changes:
- Prevent saved-request path traversal by constraining loaded request paths to the basename via
Path.GetFileName(...)(v1 and v2 Lambda test tools). - Add a Dependabot “cooldown” window for GitHub Actions updates to reduce churn from freshly-published versions.
- Extend
.semgrepignoreto exclude specific intentionally-unauthenticated/local-debug-only fixtures from scanning.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| Tools/LambdaTestTool/src/Amazon.Lambda.TestTool/SampleRequests/SampleRequestManager.cs | Sanitizes saved-request filename before combining into a filesystem path. |
| Tools/LambdaTestTool-v2/src/Amazon.Lambda.TestTool/Services/LambdaRequestManager.cs | Applies the same saved-request filename sanitization for the v2 tool. |
| .semgrepignore | Ignores specific integration-test host and preview Blazor debug UI paths to avoid intentional findings. |
| .github/dependabot.yml | Adds a cooldown delay for GitHub Actions dependency update PR creation. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Use ordinal StartsWith/IndexOf for the saved-request routing token in both test tools (it is a fixed internal token on a file-path security boundary). Add a unit test covering that a traversal-style saved-request name cannot read outside the saved-request directory.
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.
Issue #, if available: N/A
Description of changes: Fixes / ignores the findings that are causing the
semgrep-analysis.ymlaction to fail.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.