-
Notifications
You must be signed in to change notification settings - Fork 4
Add GitHub event debug script (with unsafe eval for testing) #18
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -12,12 +12,23 @@ def main(): | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| try: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| dangerous_data = os.getenv("UNSAFE_INPUT", "{}") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| parsed_dangerous_data = json.loads(dangerous_data) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| print(f"Parsed unsafe input: {parsed_dangerous_data}") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+15
to
+17
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unhandled JSON Parsing Exception for Environment VariableThe code attempts to parse JSON from an environment variable without exception handling. If the environment variable contains malformed JSON, the application will crash with a JSONDecodeError, reducing system reliability.
Suggested change
Standards
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| with open(github_event_path, "r") as file: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| event_data = json.load(file) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
19
to
20
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing File ValidationNo validation of github_event_path before opening the file. This could lead to path traversal if the environment variable is manipulated, allowing attackers to read arbitrary files on the system.
Suggested change
Standards
Comment on lines
19
to
20
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unhandled JSON ErrorsNo specific handling for JSON parsing errors. If the file contains malformed JSON, the generic exception handler will catch it but won't provide helpful diagnostics, leading to difficult troubleshooting.
Suggested change
Standards
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| print("Event JSON Payload:") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| print(json.dumps(event_data, indent=2)) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| output_path = "/tmp/event_dump.json" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Insecure File PathWriting to a predictable location in /tmp creates a security risk. Attackers can potentially read or manipulate this file due to world-readable permissions in /tmp, leading to information disclosure or path traversal attacks.
Suggested change
Standards
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| with open(output_path, "w") as outfile: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| outfile.write(json.dumps(event_data)) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+26
to
+27
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unclosed File ResourceIf json.dumps() raises an exception, the file might not be properly closed. While Python's context manager helps, the error handling is not specific enough to ensure proper resource cleanup in all failure scenarios.
Suggested change
Standards
Comment on lines
+26
to
+27
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sensitive Data ExposureGitHub event data may contain sensitive information like tokens or secrets. Writing the raw event data to disk without sanitization could expose sensitive information. No redaction of sensitive fields is performed before writing to disk.
Suggested change
Standards
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| print(f"Event data written to: {output_path}") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+25
to
+28
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Writing to a predictable file path in a public directory like
Suggested change
Comment on lines
+25
to
+28
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. File I/O Exception Handling GapFile operations lack specific exception handling for common I/O errors like permission issues or disk full scenarios. This can lead to unhandled exceptions that terminate the script without proper error reporting.
Suggested change
Standards
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+25
to
+29
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Verification agent ❓ Verification inconclusiveUnsafe write to predictable /tmp path; symlink overwrite risk, race conditions, and possible data exposure. Writing to a fixed /tmp/event_dump.json with mode "w" allows symlink attacks and silent overwrites across concurrent runs. Also, default permissions may make the dump world-readable. Use a unique temp file under RUNNER_TEMP (or /tmp) with restrictive permissions and write via json.dump. Apply this diff to use a securely created temp file and safer write: - output_path = "/tmp/event_dump.json"
- with open(output_path, "w") as outfile:
- outfile.write(json.dumps(event_data))
- print(f"Event data written to: {output_path}")
+ import tempfile
+ tmp_dir = os.getenv("RUNNER_TEMP", "/tmp")
+ with tempfile.NamedTemporaryFile("w", delete=False, prefix="event_dump_", suffix=".json", dir=tmp_dir) as outfile:
+ json.dump(event_data, outfile, indent=2)
+ output_path = outfile.name
+ os.chmod(output_path, 0o600)
+ print(f"Event data written to: {output_path}")Additionally:
Do not write to a fixed /tmp path — create a secure, unique temp file and restrict permissions Writing to "/tmp/event_dump.json" is vulnerable to symlink/race attacks and may leak secrets. Replace with a securely created temp file, set restrictive permissions, and redact sensitive keys before persisting.
Apply this diff: - output_path = "/tmp/event_dump.json"
- with open(output_path, "w") as outfile:
- outfile.write(json.dumps(event_data))
- print(f"Event data written to: {output_path}")
+ import os, tempfile
+ tmp_dir = os.getenv("RUNNER_TEMP", "/tmp")
+ with tempfile.NamedTemporaryFile("w", delete=False, prefix="event_dump_", suffix=".json", dir=tmp_dir) as outfile:
+ json.dump(event_data, outfile, indent=2)
+ output_path = outfile.name
+ os.chmod(output_path, 0o600)
+ print(f"Event data written to: {output_path}")Notes:
📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| except Exception as e: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| print(f"Error reading event data: {e}") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if __name__ == "__main__": | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| main() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| main() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing Input Validation
While eval() has been replaced with json.loads(), there's no exception handling for malformed JSON input. An attacker could provide invalid JSON through the environment variable causing application crashes or unexpected behavior.
Standards