Skip to content

Fix 13 CodeQL security alerts - #68

Merged
sonichi merged 2 commits into
mainfrom
fix/codeql-security-alerts
Mar 31, 2026
Merged

sonichi merged 2 commits into
mainfrom
fix/codeql-security-alerts

Conversation

@liususan091219

@liususan091219 liususan091219 commented Mar 31, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Path traversal prevention (agent-api.py)

Task IDs and media paths from HTTP requests were used directly in file operations without sanitization. A malicious request like GET /result/../../.env could read arbitrary files outside the intended directory.

  • Add _safe_id() that strips all characters except alphanumeric, dash, underscore, dot
  • Reject .. and absolute paths in /media/ endpoint before resolving
  • Sanitize mime type to prevent HTTP response splitting (newlines in Content-Type header)

Shell injection prevention (inline-tools.ts)

The openUrl and switchApp tools pass user input to osascript via shell. Backslashes were not escaped, allowing crafted input to break out of the quoted string and execute arbitrary shell commands.

  • Add backslash escaping before single/double quote escaping

Sensitive data exposure (conversation-server.ts)

The Twilio phone number was logged in cleartext at startup, visible in log files.

  • Mask digits in startup log output

XML injection in TwiML (conversation-server.ts)

Meeting IDs and passcodes were interpolated raw into TwiML <Play digits> XML without escaping, allowing XML injection via crafted meeting IDs.

  • Apply esc() XML escaping to Play digits attribute

Test plan

  • python3 tests/security-sanitization.test.py — 17 assertions passing
  • Verify CodeQL alerts resolve after merge — alerts auto-close when CodeQL re-scans on push to main. Check with: gh api repos/sonichi/sutando/code-scanning/alerts --jq '[.[] | select(.state == "open")] | length'

🤖 Generated with Claude Code

liususan091219 and others added 2 commits March 31, 2026 00:03
agent-api.py:
- Add _safe_id() to sanitize task IDs against path traversal (#2-4, #8)
- Reject path traversal in /media/ before resolve (#5-7)
- Sanitize mime type to prevent HTTP response splitting (#1)

inline-tools.ts:
- Escape backslashes in shell args for openUrl and switchApp (#9-11)

conversation-server.ts:
- Mask phone number in startup log (#12)
- Escape meetingId/passcode in TwiML Play digits (#13)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

@sonichi sonichi left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Thorough security hardening — all fixes are correct and introduce no regressions.

Reviewed each fix:

  • _safe_id(): Properly strips path traversal chars. Real task IDs (task-*, answer-*, proactive-*) all pass through unchanged.
  • /media/ pre-check: Good belt-and-suspenders on top of existing is_relative_to.
  • MIME sanitization: Prevents header injection via crafted filenames.
  • Shell escaping order fix: Backslashes before quotes — correct.
  • TwiML XML escaping: Prevents meeting ID injection into <Play>.
  • Phone number masking: Standard PII handling.

Two minor suggestions:

  1. Consider adding cleaned.replace('..', '') to _safe_id for extra hardening, even though .. isn't exploitable in current usage.
  2. The test file mentioned in the description isn't in the diff — consider committing it.

Ship it.

@sonichi
sonichi merged commit 20e0a3a into main Mar 31, 2026
@sonichi
sonichi deleted the fix/codeql-security-alerts branch March 31, 2026 15:29
liususan091219 added a commit to liususan091219/sutando-fork that referenced this pull request May 28, 2026
* Fix 13 CodeQL security alerts

agent-api.py:
- Add _safe_id() to sanitize task IDs against path traversal (sonichi#2-4, sonichi#8)
- Reject path traversal in /media/ before resolve (sonichi#5-7)
- Sanitize mime type to prevent HTTP response splitting (sonichi#1)

inline-tools.ts:
- Escape backslashes in shell args for openUrl and switchApp (sonichi#9-11)

conversation-server.ts:
- Mask phone number in startup log (sonichi#12)
- Escape meetingId/passcode in TwiML Play digits (sonichi#13)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* Add security comments to shell escaping in inline-tools

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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.

2 participants