Enforce RBAC, run scheduled jobs, and deliver notifications - #17
Merged
Conversation
The backend shipped a full permission model, a cron field on every job and six
notification channel types, none of which did anything. This wires them up and
fixes the data-layer bugs found alongside them.
Security and access control
- Add app/routes/deps.py with require_permission/require_admin and apply it
across every router. The REST API was reachable anonymously, including the
endpoint that stores a shell command and the one that executes it.
- Resolve SECRET_KEY from the environment, else generate and persist a random
key. The previous fallback was a published constant, so anyone could forge an
admin token against a default deployment.
- Redact secrets in notification channel responses and preserve them when a
client sends the redaction placeholder back on update.
- Move change-password credentials from query parameters into a request body.
- Gate self-registration behind ALLOW_SELF_REGISTRATION (default off) and stop
an admin from demoting, deactivating or deleting the last administrator.
- Escape HTML in the plain-text note render/preview path.
- Escape LIKE metacharacters so a search for "%" no longer matches everything.
- Keep monitor ping keys out of list responses; expose them on their own route.
Data layer
- Apply PRAGMA foreign_keys on every connection. It was set only during init,
so every ON DELETE CASCADE was dead and deleting a folder root orphaned all
of its scripts. Adds a one-shot repair for databases already affected.
- Add an additive migration step; CREATE TABLE IF NOT EXISTS never delivered a
new column to an existing database.
- Enable WAL and a busy timeout, and index the columns the hot queries filter on.
- Serialize every timestamp as UTC-aware so the UI stops shifting them.
- Aggregate tags with a unit separator; a tag containing a comma became two.
Scheduling, monitoring and notifications
- Add app/services/cron.py (validated 5-field parser with timezone support) and
app/services/scheduler.py, which fires due jobs, evaluates monitors on a timer
and reaps executions stranded by a restart.
- Add app/services/notifier.py with working Slack, Discord, webhook, PagerDuty,
SMTP and Twilio delivery; "test channel" now sends and reports the outcome.
- Validate cron expressions and timezones on write and compute next_run_at.
- Kill the whole process group on job timeout; the drain used to hang forever.
- Raise incidents and alert channels on job failure, and resolve them on recovery.
Scanning, search and attachments
- Run directory walks off the event loop and guard against symlink loops.
- Populate the folders table so the folder tree and folder notes work at all.
- Maintain the FTS index on scan, watch and note writes, and honour the
search_content/search_notes flags that were computed and then ignored.
- Rebuild watch mode around one worker per root instead of a thread per event,
and apply the root's include/exclude patterns.
- Stream attachment uploads with an early size abort, sanitise filenames, and
remove files from disk when their parent is deleted.
- Turn off difflib's autojunk heuristic, which zeroed similarity scores for any
file over ~2 KB, and make the sweep load each file once with a bounded cap.
- Add PUT /api/folder-roots/{id} so content indexing and watch mode can be
enabled without deleting and recreating the root.
Deployment
- Proxy /api to the backend in nginx-frontend.conf. Without it the composed
frontend answered every API call with index.html.
- Stop shipping a default SECRET_KEY in docker-compose.yml.
Tests: 110 passing (was 57). Adds coverage for cron parsing, RBAC enforcement,
cascade deletes, migrations and secret redaction.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JybLja9BWK1Q6yfejNYmwr
The React app had no sign-in screen and never sent a bearer token, so every authenticated endpoint was unreachable from the browser and the Team page could only ever show "Not authenticated". Several whole backend feature areas had no UI at all. This wires the client to the API and rebuilds the interface on a shared design system. Authentication - Add a sign-in screen, an auth context, and axios interceptors that attach the token and drop back to sign-in on 401 instead of leaving a dead error page. - Show who is signed in, offer sign-out, and add a Settings page for changing your own password. - Hide navigation entries and actions the account lacks permission for, and block direct URLs to pages it cannot reach, mirroring the server's rules. Features that had no interface - Duplicates page: exact (hash) duplicates and near-duplicate similarity groups. - Full-text content search and saved searches on the Search page, plus the eight filters SearchRequest supported but the form never exposed. - Script detail: note editing and deletion with Markdown rendering, attachments, custom fields, similar scripts, and a change history that shows old to new values and who made the change. - Folder roots: an edit dialog, so content indexing and watch mode can finally be turned on, plus scan history and per-root script counts. - Bulk tagging and status changes, and metadata export, from the Scripts page. - Notification channel pickers on monitors and jobs, so alerts reach someone. - A typed notification channel form (the old one was a JSON textarea seeded with a template the backend rejected), a live cron preview, and index maintenance and watch-mode controls in Settings. Interface - Replace two conflicting palettes and 100+ inline style objects with CSS design tokens, and add a dark theme that follows the OS with a manual override. - Replace 43 blocking alert() calls with a toast layer, and window.confirm with a dialog that states the impact of a destructive action. - Add an accessible Modal (dialog role, focus trap, Escape, restored focus), associate every label with its control, make table headers sortable with aria-sort, add a skip link, and fix text colours that failed AA contrast. - Collapse the sidebar into a drawer below 900px; it used to push ~540px of navigation above every page on a phone. - Add an error boundary, a 404 route, per-page document titles, skeleton loaders, empty states, and retryable error banners. Bugs - Surface the server's error detail; users saw "Request failed with status code 400" and, for validation errors, "[object Object]". - Parse offset-less timestamps as UTC. Every time in the UI was shifted by the viewer's timezone offset. - Clear the scan-status polling interval on unmount and time it out; it leaked one interval per scan and polled forever. - Discard stale list responses so a slow request cannot overwrite newer results, and clamp the page number when a result set shrinks. - Guard every submit against double-clicks, and send only changed status fields so saving no longer writes four no-op audit entries. - Re-arm the mounted ref on mount: under StrictMode it was only ever cleared, which left the app stuck on "Checking your session". Backend and deployment - Keep monitor ping keys out of listings; they are the credential a ping needs. - Serve demo mode a real administrator account and show its one-time password, instead of finishing setup with no way to sign in. Docs: README, docs/API.md and both .env.example files now describe the scheduler, real notification delivery, RBAC enforcement and the new settings. Verified end to end in a browser: sign-in, all ten pages, script detail, tag creation, modal Escape, impact-stating confirmations, full-text search, live cron validation, dark mode, the 404 route, the mobile drawer, sign-out, and a viewer account seeing a correctly reduced interface. 110 backend tests pass and the frontend builds clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JybLja9BWK1Q6yfejNYmwr
CI could never pass: the frontend job pinned Node 18, which Vite 7 refuses to install on (it requires ^20.19 || >=22.12). Split the workflow into a frontend and a backend job, move to Node 22, run pytest from backend/ so pytest.ini applies, and install from the new requirements-dev.txt. Overlap prevention was a check-then-act race: two concurrent triggers could each see no running execution and each insert one. A partial unique index on job_executions(job_id) WHERE status='running' makes the database refuse the second, and both the manual trigger and the scheduler translate that into the 409 (or a skip) it means. Bulk tagging now rejects unknown tag ids with a 404 instead of letting the foreign-key failure be counted as "skipped", and it looks each tag name up once rather than per script. Adds tests/test_scheduling.py (17 tests) covering cron and timezone validation, next_run_at computation, the cron preview, overlap prevention at both the API and database level, output capture, incident creation on failure, timeout killing the process group, reaping executions stranded by a restart, background monitor evaluation including never-pinged monitors, ping recovery resolving incidents, and ping keys staying out of listings. 127 backend tests pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JybLja9BWK1Q6yfejNYmwr
Eleven issues found reviewing the diff, all fixed:
Backend
- Retain references to scheduled job tasks. asyncio holds only a weak reference,
so a running job could be garbage-collected mid-execution, stranding its row
at 'running' and blocking the job until the next startup reap.
- Key the overlap guard on a new job_executions.overlap_key column instead of
job_id. The unconditional unique index made prevent_overlap = false
unimplementable: such jobs were still refused a concurrent run.
- Commit monitor evaluation before dispatching alerts. Sending inside the open
write transaction held SQLite's single write lock for up to the notifier's
10s HTTP timeout, so unrelated writers failed with "database is locked" -
reachable from a plain GET /api/monitors.
- Stop a watcher off the event loop, and drain the queue before signalling the
worker. Blocking joins and an unbounded put could deadlock the whole API when
a worker thread had already died with a full queue.
- Allow user creation when REQUIRE_AUTH=false. There is no user object in that
mode, so the Team page's "New user" action could never succeed.
- Create an administrator when setup completes in demo mode through
/api/setup/complete. Only /api/setup/demo had been fixed, so that path still
marked setup complete with no account to sign in with.
- Skip files younger than an hour when pruning orphaned attachments. An upload
writes its file before inserting the row that names it, so a concurrent prune
could delete an in-flight upload.
- Move GET /monitors/{id}/ping-url behind write access. Read access is held by
the viewer role, which undid removing ping keys from the monitor response.
Frontend
- Do not require a command when a job is linked to a script, and carry
script_id through the edit form; such a job could not be saved at all.
- Take the open-incident count from the stats endpoint. It was derived from a
list fetched with limit 5, so the dashboard tile plateaued at 5.
- Read onClose through a ref in Modal. Every caller passes an inline arrow, so
any parent re-render (a toast, for instance) re-ran the focus-trap effect and
pulled focus out of the field being typed in.
Adds tests for the corrected overlap semantics in both directions. 129 backend
tests pass; the browser walkthrough passes unchanged, and a job with
prevent_overlap disabled now accepts two concurrent triggers.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JybLja9BWK1Q6yfejNYmwr
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
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
2 participants
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.
The backend shipped a full permission model, a cron field on every job and six
notification channel types, none of which did anything. This wires them up and
fixes the data-layer bugs found alongside them.
Security and access control
across every router. The REST API was reachable anonymously, including the
endpoint that stores a shell command and the one that executes it.
key. The previous fallback was a published constant, so anyone could forge an
admin token against a default deployment.
client sends the redaction placeholder back on update.
an admin from demoting, deactivating or deleting the last administrator.
Data layer
so every ON DELETE CASCADE was dead and deleting a folder root orphaned all
of its scripts. Adds a one-shot repair for databases already affected.
new column to an existing database.
Scheduling, monitoring and notifications
app/services/scheduler.py, which fires due jobs, evaluates monitors on a timer
and reaps executions stranded by a restart.
SMTP and Twilio delivery; "test channel" now sends and reports the outcome.
Scanning, search and attachments
search_content/search_notes flags that were computed and then ignored.
and apply the root's include/exclude patterns.
remove files from disk when their parent is deleted.
file over ~2 KB, and make the sweep load each file once with a bounded cap.
enabled without deleting and recreating the root.
Deployment
frontend answered every API call with index.html.
Tests: 110 passing (was 57). Adds coverage for cron parsing, RBAC enforcement,
cascade deletes, migrations and secret redaction.
Co-Authored-By: Claude Opus 5 noreply@anthropic.com
Claude-Session: https://claude.ai/code/session_01JybLja9BWK1Q6yfejNYmwr