feat(03): Server-side safety filters - #45
Conversation
Screenshot now shows the bot editor with safety toggles (Restrict Foul Language, Restrict Adult Topics, Enable Web Search).
There was a problem hiding this comment.
🟡 Changes recommended
There are a few correctness/reliability issues in the new safety/event logging and API base URL handling that should be addressed before merging.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Implements roadmap 03 server-side safety (Issue #55) by moving safety enforcement from client-authored prompt suffixes to a server-owned policy with layered prompts, pre/post model filtering, tool filtering, and audit logging; includes corresponding frontend UX hardening (PIN reauth gating + outbound link confirmation) and expanded test coverage.
Changes:
- Backend: add
SafetyPolicy+ layered system prompts, input/output/tool safety filters, andSafetyEventaudit model wired into chat + web search + flashcard tools. - Frontend: replace local PIN checks with server reauthentication + in-memory parent session, add baseline safety editor notes, and add link confirmation before opening URLs.
- Tests/tooling: add backend safety test suite, frontend unit tests for link confirmation, and a Detox e2e walkthrough with seeded demo data.
File summaries
| File | Description |
|---|---|
| front/e2e/03-server-safety.e2e.js | Detox e2e validating server-side refusals and a normal safe completion path |
| front/components/PinWrapper.tsx | Parent gating via server reauth + in-memory parent session token and keypad UI |
| front/components/NavigationDrawer.tsx | Adds testID for drawer items to support e2e/test automation |
| front/components/MarkdownRenderer.tsx | Adds outbound-link confirmation dialog and URL domain extraction helper |
| front/components/HeaderButtons.tsx | Adds testID for drawer menu button |
| front/components/tests/MarkdownRenderer-test.tsx | Unit tests ensuring links confirm before opening |
| front/app/parent/settings.tsx | Blocks parent settings behind PIN presence + reauth gate; adds testIDs |
| front/app/parent/setPin.tsx | New set/change PIN flow with validation + server call behavior and errors |
| front/app/parent/botSimple.tsx | Adds baseline safety note copy in simple bot editor |
| front/app/parent/botAdvanced.tsx | Adds baseline safety note copy in advanced bot editor |
| front/app/login.tsx | Removes plaintext PIN caching; refreshes hasPin flag after login |
| front/api/pinStorage.ts | Removes legacy plaintext PIN storage; adds hasPin cache + in-memory parent session |
| front/api/botTemplates.ts | Marks client prompt generation as preview-only under server-owned safety |
| front/api/bots.ts | Updates template_name type to allow null |
| front/api/apiClient.ts | Adds X-Parent-Reauth header on unsafe methods when parent session is present |
| front/api/account.ts | Changes account shape to hasPin; adds setPin() API wrapper returning raw response |
| front/tests/api/profiles.test.ts | Tightens typings/assertions to handle nullable response |
| front/tests/api/apiClient.test.ts | Uses globalThis for fetch/XMLHttpRequest mocks |
| front/tests/api/aiModels.test.ts | Tightens typings/assertions to handle nullable response |
| front/mocks/handlers.ts | Adds explicit typing + safer string coercions in MSW handlers |
| back/bots/views/get_chat_response.py | Ensures stored system message uses server-layered prompt |
| back/bots/tests/test_safety.py | Comprehensive backend tests for policy layering, filters, tools, and guardrail behavior |
| back/bots/services/safety.py | New server-owned safety policy, denylists, layered prompts, refusals, logging, Bedrock guardrail integration |
| back/bots/services/chat_agent.py | Adds tool-level safety filters (web search + flashcards) and policy wiring |
| back/bots/models/safety_event.py | New SafetyEvent audit model |
| back/bots/models/chat.py | Enforces pre/post safety filters in get_response(); server-owned system prompt layering |
| back/bots/models/init.py | Exports SafetyEvent from models package |
| back/bots/migrations/0039_safetyevent.py | Migration creating SafetyEvent table |
| back/bots/management/commands/seed_e2e_server_safety.py | Seed command for idempotent e2e safety demo data |
| back/bots/management/commands/init.py | Package init for management commands |
| back/bots/management/init.py | Package init for management module |
| back/bots/admin.py | Registers SafetyEvent in Django admin |
Review details
- Files reviewed: 30/35 changed files
- Comments generated: 4
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
- SafetyPolicy value object + global floor denylists in services/safety.py - Layered system prompt (preamble + bot prompt + policy suffix) so flags are enforced even with custom advanced-editor prompts - Pre-model input filter and post-model output filter with fixed refusal copy - SafetyEvent audit model (stage/reason/redacted snippet) + admin - Tool filters: web_search pre-query/post-result, flashcard front/back - Optional Bedrock Guardrails check behind BEDROCK_GUARDRAIL_ID, fail-closed Refs docs/roadmap/03-server-side-safety.md
seed_e2e_server_safety creates 'e2e-test-user'/'testpassword123', a profile, and two bots: Safety Demo Bot (custom prompt + flags ON) and Open Flags Bot (flags OFF, global floor still applies). Refs docs/roadmap/03-server-side-safety.md
- policy defaults/flag mapping, layered system prompt incl. bypass attempts - input block without model call, output replacement, global floor with flags off, SafetyEvent stage/reason/redaction assertions - web search: unbound when disabled, blocked query, stripped results - flashcard tools reject unsafe cards without persisting - denylist word-boundary cases; Bedrock guardrail flag + fail-closed Refs docs/roadmap/03-server-side-safety.md
- MarkdownRenderer: confirm dialog showing the domain before Linking.openURL - botSimple/botAdvanced: 'Syft always applies baseline safety' editor copy - botTemplates: document that the client prompt is preview-only now - jest coverage for link confirm/cancel + domain extraction Refs docs/roadmap/03-server-side-safety.md
Baseline 'npm run typecheck' failed before this feature: bare 'global' references in apiClient.test.ts, null-vs-string template_name in Bot fixtures/interface, non-null assertions missing in profiles.test.ts. Refs docs/roadmap/03-server-side-safety.md
- __mocks__/handlers.ts: typed msw json bodies and params - aiModels.test.ts: non-null assertions like profiles.test.ts Refs docs/roadmap/03-server-side-safety.md
Drives the seeded 'Safety Demo Bot': an adult-topic message gets the fixed server refusal (no model call), then a normal homework question still gets a real assistant reply. Header documents seeding + env requirements. Refs docs/roadmap/03-server-side-safety.md
Screenshot now shows the bot editor with safety toggles (Restrict Foul Language, Restrict Adult Topics, Enable Web Search).
…harden filters - back/bots/models/chat.py: log flagged output (not refusal) for SafetyEvent output stage - back/bots/services/chat_agent.py: include full title+content in web_result snippet, check deck name/description and deck_name for flashcard tools - back/bots/services/safety.py: normalize hyphen in is_crisis check - back/bots/tests/test_safety.py: fix describe outer fixture signature for pytest-describe 3.1 - front: revert pin-reauth frontend duplicated from #44 (account, apiClient, pinStorage, login, setPin, settings, PinWrapper) to keep PR safety-only; fixes partial #44 break and migration scoping Removes 330 LOC of out-of-scope pin frontend so PR diff is now safety-only vs main. Fixes audit bug where output stage stored refusal instead of redacted flagged text.
6b0c226 to
d7ba4b1
Compare
- front/app/parent/botSimple.tsx: move baseline safety note above Restrict Foul Language so it introduces the toggle group instead of splitting foul/adult toggles (fixes #45 (comment)) - front/components/MarkdownRenderer.tsx: fix linkDomain docstring example (docs.example.com not example.com) per Copilot comment
|
Replied to #45 (comment) — fixed in 880849c: moved baseline safety note above Restrict Foul Language so it introduces the toggle group instead of splitting foul/adult toggles. Also fixed linkDomain docstring example per Copilot. |
- Replace light-mode bot editor screenshot with dark-mode version - Shows safety note above Restrict Foul Language (fixes r3878020585 grouping) - Dark background (#121212) with toggles and baseline safety note visible
- back/bots/services/safety.py: replace boto3 bedrock-runtime ApplyGuardrail with requests POST https://api.openai.com/v1/moderations (model omni-moderation-latest, free) - Guardrail still feature-flagged but now via OPENAI_API_KEY (was BEDROCK_GUARDRAIL_ID); denylist-only when empty, fail-closed on vendor error (keeps global floor guarantee) - Keeps source param for compatibility but now maps flagged==true -> REASON_GLOBAL_FLOOR - back/server/settings.py: add OPENAI_API_KEY env - back/bots/tests/test_safety.py: rename describe_bedrock_guardrail_flag -> describe_openai_guardrail_flag, mock requests.post instead of boto3 Cost: 0$ vs $0.15/1k text units per Bedrock policy; default Nova 2 Lite $0.06/$0.24 per 1M tokens stays ~6-12x cheaper than Guardrails. Tests mock requests, never hit network.
|
Swapped (safety.py:346) from Bedrock to free OpenAI moderation in b94c82b — now () when set, else denylist-only. Keeps fail-closed, free vs $0.15/1k Bedrock. Tests updated to mock (29 passed). |
- front/api/apiClient.ts: add Sentry-captured guard at entry of
apiClient() and refreshWithRefreshToken() matching front/api/tokens.ts;
short-circuits with clear error instead of fetching undefined{endpoint}
and confusing failures (Copilot: apiClient builds request URLs using
BASE_URL without guarding for the undefined case)
- front/jest.setup.js: default EXPO_PUBLIC_API_BASE_URL so existing
apiClient tests continue to pass after guard (matches tokens.ts handling)
Prior Copilot threads already addressed and verified:
- front/components/MarkdownRenderer.tsx docstring fixed in 880849c
(docs.example.com vs example.com, www stripping)
- back/bots/models/chat.py output SafetyEvent now logs flagged_output
before refusal replacement (fixed in d7ba4b1)
- front/components/PinWrapper.tsx thread is outdated: PIN reauth reverted
in d7ba4b1 to keep PR safety-only (duplicated roadmap-02 code moved to
#44); current PinWrapper has no BASE_URL usage
|
Addressed Copilot review (4 threads) — all verified in branch
Verification: backend safety 29 passed, frontend 54 passed, typecheck ok. Requesting re-review. |
This comment was marked as duplicate.
This comment was marked as duplicate.
Sorry, something went wrong.
There was a problem hiding this comment.
🟡 Changes recommended
Blocked content can re-enter model history, and several moderation, redaction, and link-handling paths weaken the intended safety guarantees.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
front/api/apiClient.ts:133
- As above, an empty base URL bypasses this guard and attempts a relative token-refresh request. Use the same falsy configuration check here.
if (BASE_URL === undefined) {
Sentry.captureMessage("BASE_URL is undefined");
throw new Error("EXPO_PUBLIC_API_BASE_URL is not configured");
- Files reviewed: 26/31 changed files
- Comments generated: 10
- Review effort level: Balanced
…ration, link scheme guard - Add explicit CSAM variants to the global floor denylist - Mark safety-blocked messages and exclude them from later model context (Message.safety_blocked + migration 0040) - Parse OpenAI moderation categories: floor categories always block, policy categories respect parent flags, crisis signal preserved for self-harm; fail closed on malformed payloads - Fully redact SafetyEvent snippets when a verdict has no term detail - MarkdownRenderer: only offer Open for valid HTTP(S) links - seed_e2e_server_safety: refuse to run without E2E_SEEDING=1 - apiClient: treat any falsy BASE_URL as a configuration error - e2e: require the named seeded bot (no first-bot fallback) - settings: document the OpenAI moderation guardrail provider - evidence: teen crisis refusal + parent SafetyEvent admin screenshots
|
Addressed all 10 Copilot review comments (round 2) in 7eee881 — replies posted on each thread. Also added a crisis walkthrough to the PR description with real screenshots from the app running against the live backend (no mocks):
Verification: backend 123 passed (12 new safety tests), frontend 60 passed (6 new link-guard tests), typecheck + lint clean. |
There was a problem hiding this comment.
🟡 Changes recommended
Moderation mapping, audit redaction, and concurrent message selection can bypass intended safety guarantees.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 28/36 changed files
- Comments generated: 4
- Review effort level: Balanced
… race - safety: add self-harm/intent and violence/graphic to floor mapping - safety: unknown moderation categories fail closed to global_floor - safety: fully redact when category labels never occur literally in snippet - chat: serialize turns with select_for_update and tie evaluation to request message - view: create user message atomically with response - tests: cover new floor categories, unknown fail-closed, no-op redact
|
Addressed all 4 Copilot round-3 comments in e267776 — replies posted on each thread. Verification: backend 126 passed (3 new), frontend 60 passed, typecheck + lint clean. |
There was a problem hiding this comment.
🟡 Changes recommended
Safety filtering is bypassed on quota and model-initialization paths, while the default floor omits promised violence coverage.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
back/bots/services/safety.py:188
- The always-on preamble bans all sexual content even when
restrict_adult_topicsis false, while the denylist and moderation mappings explicitly allow ordinarysexual/porncategories when that flag is off. The model therefore still receives a restriction the parent supposedly disabled. Keep only the minors/global-floor prohibition here and leave the broader ban in the conditional policy suffix.
"- Never produce sexual content, and never produce sexual content "
"involving minors under any circumstances.\n"
- Files reviewed: 28/36 changed files
- Comments generated: 4
- Review effort level: Balanced
| with transaction.atomic(): | ||
| Chat.objects.select_for_update().get(pk=self.pk) | ||
| return self._get_response_locked(ai, user_message) |
There was a problem hiding this comment.
Fixed in 51927d4 — get_response now does input safety before any model/quota work, then uses short transactions only for the two state changes (marking the blocked message + final persist). Moderation, Bedrock and Tavily calls happen outside any transaction.atomic / select_for_update, so a normal turn no longer holds a DB connection for the whole pipeline. (back/bots/models/chat.py:70, back/bots/views/get_chat_response.py:84)
| if self.bot and self.bot.ai_model: | ||
| self.ai = AiClientWrapper(model_id=self.bot.ai_model.model_id, client=ai) | ||
| else: | ||
| self.use_default_model(ai) |
There was a problem hiding this comment.
Fixed in 51927d4 — AI client (AiClientWrapper/use_default_model) is now instantiated only after the input safety check passes (back/bots/models/chat.py:98). A crisis message therefore gets the fixed refusal and audit event even when no default model is configured. Covered by it_still_blocks_crisis_when_no_model_configured.
| if self.user.user_account.over_limit(): | ||
| return "You have exceeded your daily limit. Please try again tomorrow or upgrade your subscription." |
There was a problem hiding this comment.
Fixed in 51927d4 — input filtering now runs before the quota check (back/bots/models/chat.py:71). An unsafe message from an over-limit user is still marked safety_blocked=True and gets the stage input audit event, so a later safe turn excludes it from get_input. Covered by it_still_blocks_crisis_when_over_limit.
| "make a bomb", | ||
| "build a bomb", | ||
| "bomb making", | ||
| "pipe bomb", |
There was a problem hiding this comment.
Fixed in 51927d4 — expanded GLOBAL_FLOOR_TERMS with violent-instruction phrases (how to kill, how to murder, how to stab, kill/murder/stab someone) so the global floor holds in denylist-only mode (OPENAI_API_KEY unset) without relying solely on the remote classifier. Covered by violent_instructions_on_global_floor_without_classifier (back/bots/services/safety.py:92).
…l/limit, violent floor - chat: evaluate input safety before AI init and quota check; use short transactions for marking and persisting so no DB lock is held across moderation/Bedrock/Tavily calls - view: create message outside transaction; get_response owns its short locks - safety: expand global floor denylist with violent-instruction phrases so denylist-only mode still enforces weapons/violence floor - tests: violent floor, over-limit crisis, and missing-model crisis
|
Addressed all 4 Copilot round-4 comments in 51927d4 — replies posted on each thread. Verification: backend 129 passed (3 new), frontend 60 passed, typecheck + lint clean. |
Roadmap 03: Server-Side Safety
Implements #55.
What changed
SafetyPolicywith denylists, crisis detection, layered system prompt, input/output/tool filters,SafetyEventmodel; optional free OpenAI moderation (omni-moderation-latest) with category-aware mapping, denylist-only and fail-closed when unsetCrisis walkthrough — teen says "I want to hurt myself"
Live capture: Django backend + Expo web app running locally, seeded e2e account, a bot with all parent restrictions OFF (proves the floor is not parent-disableable), no model call involved.
What the teen sees — the message goes through the real chat API; the server's input filter matches
hurt myselfon the global floor and returns the fixed crisis response (REFUSAL_CRISISinback/bots/services/safety.py) without ever invoking the model. The response is kind, points to a trusted adult, and offers to return to schoolwork:What the parent sees — every block writes a
SafetyEventaudit row (stageinput, reasonglobal_floor) whose snippet is term-redacted (i want to [redacted]), viewable in the Django admin. This audit feed is what the roadmap 04 parent inbox will be built on:Blocked messages are also marked (
Message.safety_blocked) and excluded from later model context, so denied content never re-enters model history on the next turn.Evidence
https://github.com/tpaulshippy/bots/raw/feature/roadmap-03-server-safety/evidence/pr45-safety.mp4