Skip to content

Conversation

@milhy545
Copy link

No description provided.

claude and others added 4 commits November 15, 2025 10:05
Add detailed documentation to help AI assistants understand and work
with the AgentAPI codebase. Includes:

- Project overview and architecture
- Detailed component descriptions (termexec, msgfmt, screentracker, httpapi)
- Development setup and workflows
- Code conventions and patterns
- Testing strategy and examples
- Common tasks and troubleshooting
- CI/CD and release process
- Quick reference guide

This documentation covers all 9 supported agents, build processes,
API endpoints, and provides practical examples for common development
tasks.
This commit resolves multiple HIGH and MEDIUM severity issues found during
security audit. All tests pass after changes.

**HIGH Severity Fixes:**

1. Fix nil pointer panic with --print-openapi flag
   - Added nil check before process.Wait() to prevent crash
   - File: cmd/server/server.go

2. Fix goroutine leak in snapshot loop
   - Added context cancellation with select statement
   - Changed time.Sleep to ticker for proper cleanup
   - File: lib/httpapi/server.go

3. Fix race condition in terminal screen reading
   - Always hold lock when reading terminal state
   - Prevents concurrent read/write on p.xp.State
   - File: lib/termexec/termexec.go

4. Add defensive checks for unsafe reflection
   - Added defer/recover to catch reflection failures
   - Improved error logging with recovery attempts
   - Warns about unsafe xpty dependency
   - File: lib/termexec/termexec.go

**MEDIUM Severity Fixes:**

5. Add message size validation
   - MaxMessageSize: 10MB for user messages
   - MaxRawMessageSize: 1KB for raw terminal input
   - Prevents DoS via oversized messages
   - File: lib/httpapi/server.go

6. Add HTTP server timeouts
   - ReadTimeout: 15s (prevent slow header attacks)
   - ReadHeaderTimeout: 5s (specifically for headers)
   - IdleTimeout: 60s (close idle connections)
   - WriteTimeout: 0 (disabled for SSE long-polling)
   - File: lib/httpapi/server.go

7. Replace panics with error handling
   - logctx.From() returns default logger instead of panic
   - conversation.statusInner() logs errors and returns safe fallback
   - convertStatus() logs unknown status instead of panic
   - Files: lib/logctx/logctx.go, lib/screentracker/conversation.go,
     lib/httpapi/events.go

All existing tests pass (CGO_ENABLED=0 go test ./...).
This commit addresses additional code quality improvements identified in
the security audit. All tests pass after changes.

**LOW Severity Improvements:**

1. Add SSE subscriber limits (prevents DoS)
   - MaxSSESubscribers constant set to 100 concurrent connections
   - Subscribe() now returns error when limit reached
   - Added logging when slow subscribers are disconnected
   - Added test for subscriber limit enforcement
   - Files: lib/httpapi/events.go, lib/httpapi/server.go,
     lib/httpapi/events_test.go

2. Extract magic numbers to named constants
   - Terminal screen stability constants:
     * ScreenStabilityCheckInterval = 16ms (~60 FPS)
     * ScreenStabilityRetries = 3
   - Message parsing heuristics constants:
     * MaxUserInputPrefixRunesToMatch = 6
     * MaxLinesToSearchForUserInput = 5
     * DefaultPrefixRunesToSearch = 25
     * MaxRuneLookahead = 5
   - Improved code maintainability and documentation
   - Files: lib/termexec/termexec.go, lib/msgfmt/msgfmt.go

3. Add CORS/Host wildcard security warnings
   - Logs prominent warning when using '*' for allowed origins
   - Logs warning when using '*' for allowed hosts
   - Warns users to only use wildcards in development
   - File: lib/httpapi/server.go

All existing tests pass (CGO_ENABLED=0 go test ./...).
New subscriber limit test added to events_test.go.
Copy link
Author

@milhy545 milhy545 left a comment

Choose a reason for hiding this comment

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

@gemini /review

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