Skip to content

Security fixes#347

Merged
tbckr merged 7 commits into
tbckr:mainfrom
zorak1103:security_fixes
Feb 18, 2026
Merged

Security fixes#347
tbckr merged 7 commits into
tbckr:mainfrom
zorak1103:security_fixes

Conversation

@zorak1103

Copy link
Copy Markdown
Collaborator

Please see commit messages

Add ErrEmptyResponse sentinel error and bounds checking for resp.Choices[0]
in both streaming and non-streaming API response handlers. Non-streaming path
returns an error for empty choices array, while streaming path uses continue
to skip empty chunks (which may occur as heartbeats).

Prevents index out of bounds panics when OpenAI API returns responses with no
choices.
Add SanitizeCommand function to remove:
- ANSI escape sequences (CSI and OSC)
- Unicode bidirectional override characters
- Zero-width characters (U+200B-D, U+FEFF)
- Non-printable control characters (preserving newlines and tabs)

Apply sanitization in ExecuteCommandWithConfirmation before displaying the
command to the user and executing it. This ensures what-you-see-is-what-
executes (WYSIWYG) and prevents display-based command injection attacks
where malicious Unicode or ANSI sequences could hide dangerous commands.
Add r.exit(1) call in the panic recovery handler. Previously, panics were
logged but the program would continue executing and exit with code 0,
masking critical failures. Now ensures that any panic results in a non-zero
exit code, providing proper error signaling to the calling process.
Implement Unwrap() method on exitError to enable proper error chain traversal
with errors.Is() and errors.As(). This allows callers to inspect wrapped
errors and maintains Go 1.13+ error handling best practices.
Add scanner.Err() check after the scanning loop in GetSession. Previously,
scanner errors (such as lines exceeding bufio.MaxScanTokenSize) were silently
ignored, potentially returning incomplete or corrupt session data. Now properly
propagates scanner errors to the caller.

Follows the same pattern used in pkg/fs/fs.go for consistent error handling.
@zorak1103 zorak1103 requested a review from tbckr as a code owner February 16, 2026 17:15
@codecov

codecov Bot commented Feb 16, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 92.30769% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.63%. Comparing base (abd2da9) to head (fe5f4e4).
⚠️ Report is 266 commits behind head on main.

Files with missing lines Patch % Lines
pkg/api/api.go 50.00% 1 Missing and 1 partial ⚠️
pkg/cli/root.go 50.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #347      +/-   ##
==========================================
+ Coverage   75.80%   78.63%   +2.82%     
==========================================
  Files          20       20              
  Lines        1298     1072     -226     
==========================================
- Hits          984      843     -141     
+ Misses        226      145      -81     
+ Partials       88       84       -4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

- Remove trailing empty lines in api_test.go and shell_test.go
- Align comment spacing in shell.go for consistency
@tbckr tbckr merged commit d012e92 into tbckr:main Feb 18, 2026
7 checks passed
@tbckr

tbckr commented Feb 18, 2026

Copy link
Copy Markdown
Owner

Thanks for finding and fixing these security issues!

tbckr pushed a commit that referenced this pull request Feb 18, 2026
* fix: guard against empty API response choices to prevent panic

Add ErrEmptyResponse sentinel error and bounds checking for
resp.Choices[0]
in both streaming and non-streaming API response handlers. Non-streaming
path
returns an error for empty choices array, while streaming path uses
continue
to skip empty chunks (which may occur as heartbeats).

Prevents index out of bounds panics when OpenAI API returns responses
with no
choices.

* fix: sanitize shell commands to prevent display manipulation

Add SanitizeCommand function to remove:
- ANSI escape sequences (CSI and OSC)
- Unicode bidirectional override characters
- Zero-width characters (U+200B-D, U+FEFF)
- Non-printable control characters (preserving newlines and tabs)

Apply sanitization in ExecuteCommandWithConfirmation before displaying
the
command to the user and executing it. This ensures what-you-see-is-what-
executes (WYSIWYG) and prevents display-based command injection attacks
where malicious Unicode or ANSI sequences could hide dangerous commands.

* fix: exit with non-zero code on panic recovery

Add r.exit(1) call in the panic recovery handler. Previously, panics
were
logged but the program would continue executing and exit with code 0,
masking critical failures. Now ensures that any panic results in a
non-zero
exit code, providing proper error signaling to the calling process.

* fix: add Unwrap method to exitError for error chain traversal

Implement Unwrap() method on exitError to enable proper error chain
traversal
with errors.Is() and errors.As(). This allows callers to inspect wrapped
errors and maintains Go 1.13+ error handling best practices.

* fix: check scanner error after reading chat session file

Add scanner.Err() check after the scanning loop in GetSession.
Previously,
scanner errors (such as lines exceeding bufio.MaxScanTokenSize) were
silently
ignored, potentially returning incomplete or corrupt session data. Now
properly
propagates scanner errors to the caller.

Follows the same pattern used in pkg/fs/fs.go for consistent error
handling.

* style: remove trailing newlines and fix comment alignment

- Remove trailing empty lines in api_test.go and shell_test.go
- Align comment spacing in shell.go for consistency
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