Skip to content

security: fix secret leakage, path injection, and missing signature verification#4

Open
Keramikus-97 wants to merge 1 commit into
mainfrom
devin/1780761164-security-fixes
Open

security: fix secret leakage, path injection, and missing signature verification#4
Keramikus-97 wants to merge 1 commit into
mainfrom
devin/1780761164-security-fixes

Conversation

@Keramikus-97

Copy link
Copy Markdown
Owner

Summary

Fixes three critical security vulnerabilities found during a codebase audit:

  1. Secret leakage via repr()Config stores github_token and anthropic_api_key as plain dataclass fields. The default __repr__ exposes them in logs, tracebacks, and debuggers. Fixed by overriding __repr__ to mask both fields.

  2. URL path injection in GitHubClientowner/repo parameters are interpolated directly into API paths without validation. A crafted webhook payload with owner = "../admin" could redirect requests. Fixed by validating all path segments against ^[a-zA-Z0-9._-]+$ before use.

  3. Easy-to-misuse webhook parsing APIparse_raw() skips signature verification entirely, making it trivial to process forged payloads. Added parse_raw_verified() as a safe all-in-one entry point that enforces HMAC verification before parsing.

# Path validation (github_client.py)
_SAFE_PATH_SEGMENT = re.compile(r\"^[a-zA-Z0-9._-]+$\")
def _validate_path_segment(value, name):
    if not value or not _SAFE_PATH_SEGMENT.match(value):
        raise ValueError(...)

# Safe webhook path (webhook_handler.py)
def parse_raw_verified(event_header, body, signature, secret):
    if not verify_signature(body, signature, secret):
        return None
    return parse_raw(event_header, body)

Note: A fourth fix (GitHub Actions workflow hardening — pinning actions/checkout to SHA, adding author_association guard) is blocked by the OAuth proxy lacking workflow scope. A patch is provided separately for manual application.

Link to Devin session: https://app.devin.ai/sessions/430cdf458aec433caef3c2b2851e0432
Requested by: @Keramikus-97

- 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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant