Open
Conversation
Add a `readonly` parameter to Monitor and start_monitor() that prevents destructive operations (cancel, signal, console) in production environments. - Guard termui commands: cancel, signal, console - Guard webui DELETE /api/task endpoint with 403 response - Hide cancel button in webui via Alpine.js when readonly - Show read-only banner in webui dashboard - Defense-in-depth: cancel_monitored_task raises PermissionError - Add comprehensive tests for read-only mode
- Fix pre-existing bug: do_console console_enabled=False path now sets command_done event to prevent telnet session hang - Replace Alpine.js x-show with CSS-based approach for hiding cancel button in read-only mode (works reliably with HTMX/Mustache) - Add webui API tests for read-only cancel endpoint (403) and non-readonly cancel endpoint - Fix import ordering in tests
Member
Author
Automated Review (iteration 2) — Follow-upLocal Checks
CI Status
Previous Findings Status
New IssuesNone SummaryAll 3 previous findings have been addressed. No new issues introduced. All local and CI checks pass. |
Member
Author
Resolution CompleteThis PR is now ready for human review. Summary of ChangesAdded a
Review Iterations2 iterations — initial implementation + review fixes Test Coverage
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves #386
Summary
Add a
readonlyparameter toMonitorandstart_monitor()that, whenTrue, prevents destructive operations (task cancellation, signal sending, console access) in both the terminal UI and web UI. This protects production environments from accidental task cancellation.Implementation Plan
Approach
Thread a
readonlyboolean throughMonitor.__init__()andstart_monitor()that guards destructive commands in termui (cancel,signal,console) and the webui cancel endpoint (DELETE /api/task), while hiding cancel buttons in the frontend.Steps
monitor.py: Addreadonlyparameter toMonitor.__init__andstart_monitor; add guard incancel_monitored_tasktermui/commands.py: Add read-only guards todo_cancel,do_signal,do_console; update intro messagewebui/app.py: Guardcancel_taskendpoint with 403; passreadonlyto template; addreadonlytoWebUIContextwebui/templates/index.html: Hide cancel button via Alpine.js store; add read-only bannertests/test_monitor.py: Add tests for read-only behaviorFiles to Modify
aiomonitor/monitor.py— Addreadonlyparam, property, and defense-in-depth guardaiomonitor/termui/commands.py— Guarddo_cancel,do_signal,do_consoleaiomonitor/webui/app.py— Guard cancel endpoint, pass readonly to templateaiomonitor/webui/templates/index.html— Conditional cancel button, read-only bannertests/test_monitor.py— New tests for read-only modeEdge Cases
readonlydefaults toFalsecancel_monitored_taskraisesPermissionErroreven if UI guards are bypassedcommand_doneevent handling for commands that don't use@auto_command_doneTest Plan
Monitor(loop, readonly=True)constructorcancelcommand rejected in read-only modesignalcommand rejected in read-only modeconsolecommand rejected in read-only modecancel_monitored_taskraisesPermissionErrorin read-only modeps,where, etc.) still work in read-only mode