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. |
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
| "child porn", | ||
| "child sexual", |
| latest_user_message = self.messages.filter(role='user').order_by('-id').first() | ||
| if latest_user_message is not None: | ||
| verdict = evaluate_text(latest_user_message.text, policy, source='INPUT') |
| results = data.get("results", []) | ||
| if results and results[0].get("flagged"): | ||
| return SafetyVerdict(True, REASON_GLOBAL_FLOOR) |
| redacted = normalize_text(snippet) | ||
| if verdict: | ||
| for term in verdict.matched_terms: | ||
| redacted = re.sub( | ||
| r"(?<!\w)" + re.escape(term) + r"(?!\w)", | ||
| "[redacted]", | ||
| redacted, | ||
| ) | ||
| return redacted[:limit] |
| const handleLinkPress = (url: string) => { | ||
| // Never open assistant links directly: confirm the destination first. | ||
| alert(`Open ${linkDomain(url)}?`, url, [ | ||
| { text: 'Cancel', style: 'cancel', onPress: () => {} }, | ||
| { | ||
| text: 'Open', | ||
| onPress: () => { | ||
| Linking.openURL(url).catch(() => null); | ||
| }, | ||
| }, | ||
| ]); | ||
| }; |
| user, created = User.objects.get_or_create(username=E2E_USERNAME) | ||
| if created or not user.check_password(E2E_PASSWORD): | ||
| user.set_password(E2E_PASSWORD) | ||
| user.save() |
| const safetyBot = | ||
| bots.results.find((b) => b.name === SAFETY_BOT_NAME) || bots.results[0]; |
| if (BASE_URL === undefined) { | ||
| Sentry.captureMessage("BASE_URL is undefined"); | ||
| throw new Error("EXPO_PUBLIC_API_BASE_URL is not configured"); | ||
| } |
|
|
||
| TAVILY_API_KEY = env('TAVILY_API_KEY', default='') | ||
|
|
||
| OPENAI_API_KEY = env('OPENAI_API_KEY', default='') |
| data = resp.json() | ||
| results = data.get("results", []) | ||
| if results and results[0].get("flagged"): | ||
| return SafetyVerdict(True, REASON_GLOBAL_FLOOR) | ||
| return None |
Roadmap 03: Server-Side Safety
Implements #55.
What changed
SafetyPolicywith denylists, crisis detection, layered system prompt, input/output/tool filters,SafetyEventmodelEvidence
https://github.com/tpaulshippy/bots/raw/feature/roadmap-03-server-safety/evidence/pr45-safety.mp4