security: fix secret leakage, path injection, and missing signature verification#4
Open
Keramikus-97 wants to merge 1 commit into
Open
security: fix secret leakage, path injection, and missing signature verification#4Keramikus-97 wants to merge 1 commit into
Keramikus-97 wants to merge 1 commit into
Conversation
- config.py: Override __repr__ to mask github_token and anthropic_api_key, preventing accidental secret exposure in logs and tracebacks - github_client.py: Validate owner/repo path segments against a safe charset regex to prevent URL path injection via crafted webhook payloads - webhook_handler.py: Add parse_raw_verified() that enforces signature verification before parsing, and document that parse_raw() alone is unsafe Co-Authored-By: dominicpape <dominicpape@gmx.net>
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.
Summary
Fixes three critical security vulnerabilities found during a codebase audit:
Secret leakage via
repr()—Configstoresgithub_tokenandanthropic_api_keyas plain dataclass fields. The default__repr__exposes them in logs, tracebacks, and debuggers. Fixed by overriding__repr__to mask both fields.URL path injection in
GitHubClient—owner/repoparameters are interpolated directly into API paths without validation. A crafted webhook payload withowner = "../admin"could redirect requests. Fixed by validating all path segments against^[a-zA-Z0-9._-]+$before use.Easy-to-misuse webhook parsing API —
parse_raw()skips signature verification entirely, making it trivial to process forged payloads. Addedparse_raw_verified()as a safe all-in-one entry point that enforces HMAC verification before parsing.Note: A fourth fix (GitHub Actions workflow hardening — pinning
actions/checkoutto SHA, addingauthor_associationguard) is blocked by the OAuth proxy lackingworkflowscope. A patch is provided separately for manual application.Link to Devin session: https://app.devin.ai/sessions/430cdf458aec433caef3c2b2851e0432
Requested by: @Keramikus-97