Add comprehensive documentation for alerts, database schema, deployment, detection rules, and user management #3
Conversation
2abet
commented
Nov 12, 2025
2abet
commented
- Introduced ALERTS.md detailing alert types, notification channels, alert rules, management, tuning, integration patterns, troubleshooting, and API reference.
- Added DATABASE.md outlining the database schema, core tables, security tables, indexes, data relationships, retention policies, security features, backup strategies, monitoring, maintenance, and troubleshooting.
- Created DEPLOYMENT.md covering production architecture, Docker setup, Kubernetes deployment, security hardening, monitoring, backup strategies, performance optimization, maintenance procedures, troubleshooting, and a security checklist.
- Expanded DETECTION_RULES.md with detailed rule types, configuration, management, testing, risk scoring, best practices, troubleshooting, and API reference.
- Updated USER_MANAGEMENT.md to include user roles, management procedures, security best practices, API access, troubleshooting, and audit compliance.
…nt, detection rules, and user management - Introduced ALERTS.md detailing alert types, notification channels, alert rules, management, tuning, integration patterns, troubleshooting, and API reference. - Added DATABASE.md outlining the database schema, core tables, security tables, indexes, data relationships, retention policies, security features, backup strategies, monitoring, maintenance, and troubleshooting. - Created DEPLOYMENT.md covering production architecture, Docker setup, Kubernetes deployment, security hardening, monitoring, backup strategies, performance optimization, maintenance procedures, troubleshooting, and a security checklist. - Expanded DETECTION_RULES.md with detailed rule types, configuration, management, testing, risk scoring, best practices, troubleshooting, and API reference. - Updated USER_MANAGEMENT.md to include user roles, management procedures, security best practices, API access, troubleshooting, and audit compliance.
WalkthroughFive new comprehensive documentation files were added covering alerting, PostgreSQL schema, production deployment, detection rules, and RBAC-based user management. No code or public API declarations were modified. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (7)
docs/USER_MANAGEMENT.md (3)
72-72: Consider increasing minimum password requirement for production.A minimum of 6 characters is quite weak for a security-sensitive application. Industry best practices and most compliance frameworks (NIST, CIS, etc.) recommend 8+ characters minimum. The recommendation to use 12+ is good guidance, but consider enforcing this at the system level rather than just recommending it.
Also applies to: 74-75
93-102: Clarify that Bearer token in curl example is a placeholder.While the documentation correctly shows
YOUR_ACCESS_TOKENas a placeholder, the static analysis flagged this as a potential security token exposure. Consider adding an explicit note that this is a placeholder example to reduce any confusion, especially for users who may copy-paste the command directly.Suggested addition before the curl example:
# Note: Replace YOUR_ACCESS_TOKEN with your actual token obtained from login
117-120: Document password recovery limitation as a TODO or enhancement.The lack of self-service password recovery is noted as "not currently available," but this is a significant usability and security gap (users locked out require admin intervention). Consider opening a GitHub issue to track implementation of email-based or security-question-based recovery, or clearly mark this as a known limitation in the roadmap.
Would you like me to help create a GitHub issue to track this enhancement?
docs/ALERTS.md (1)
303-312: Add authentication requirement note to test notification endpoint.The test notification endpoint at line 305 accepts webhook URLs and credentials inline. Consider documenting that this endpoint requires admin authentication to prevent unauthorized users from testing connections to arbitrary endpoints.
docs/DATABASE.md (1)
41-42: Clarify encryption status in schema documentation.The schema documentation states that
promptandresponsefields are "encrypted in production" (lines 41-42), but the current table schema shows unencryptedTEXTcolumns. While the future encryption feature is documented separately (lines 285-291), this could confuse implementers.Consider:
- Adding a note in the Core Tables section indicating these are planned for encryption
- Or creating separate columns now with encryption-ready naming (e.g.,
prompt_encrypted,response_encrypted) to signal future directionAlso applies to: 285-291
docs/DEPLOYMENT.md (2)
117-142: Add security note about .env.prod file handling.The
.env.prodfile (lines 117-142) contains sensitive values likeDB_PASSWORD,JWT_SECRET_KEY, andENCRYPTION_KEY. While this is standard for docker-compose development, production deployments should use secure secrets management (e.g., Kubernetes Secrets, AWS Secrets Manager, HashiCorp Vault).Add a security note recommending against committing
.env.prodto version control and using proper secrets management for production:Add this note after line 118:
⚠️ **Security Warning**: Do NOT commit `.env.prod` to version control. For production, use Kubernetes Secrets, environment variable injection from secure vaults (AWS Secrets Manager, HashiCorp Vault), or container orchestration platform secrets management.
544-553: Expand security checklist with additional production controls.The security checklist is a good starting point but could be more comprehensive. Consider adding items such as:
- Rate limiting configured
- CORS properly restricted
- Database connection pooling configured
- SQL injection prevention verified
- CSRF protection enabled (if applicable)
- Security headers configured (CSP, X-Frame-Options, etc.)
- Regular security scanning/vulnerability assessments scheduled
- Incident response plan documented
- Access control audits scheduled
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
docs/ALERTS.md(1 hunks)docs/DATABASE.md(1 hunks)docs/DEPLOYMENT.md(1 hunks)docs/DETECTION_RULES.md(1 hunks)docs/USER_MANAGEMENT.md(1 hunks)
🧰 Additional context used
🪛 Gitleaks (8.29.0)
docs/USER_MANAGEMENT.md
[high] 95-101: Discovered a potential authorization token provided in a curl command header, which could compromise the curl accessed resource.
(curl-auth-header)
🪛 LanguageTool
docs/USER_MANAGEMENT.md
[style] ~80-~80: Consider using a different verb to strengthen your wording.
Context: ...e - Regularly review user permissions - Remove access for departed team members - Moni...
(REMOVE_REVOKE)
🔇 Additional comments (3)
docs/ALERTS.md (1)
151-156: Verify Slack mention syntax in webhook payloads.Line 155 includes
"mention_users": ["@security-team"], but the Slack Incoming Webhooks API may not support direct mentions in this format. The payload format should be verified against the current Slack API documentation, as mentions typically require user IDs or specific message formatting.Can you confirm that this mention syntax works with the actual FlagWise implementation, or if it requires different formatting (e.g.,
<@USER_ID>or mrkdwn format)?docs/DETECTION_RULES.md (1)
82-82: Fix potential formatting issue in email regex table entry.Line 82 shows:
`\b[A-Za-z0-9._%+-]+@[A-Za-z0-9.-]+\.[A-Z\|a-z]{2,}\b`The backtick escaping and the
\|character (escaped pipe) appear odd. Verify this regex is correct and properly escaped. The pattern should match standard email addresses, but the escaped pipe character is unusual—typically you'd use|unescaped in regex character classes, or use|a-zif trying to match uppercase or lowercase.Can you confirm the correct regex pattern for email matching? A more standard pattern would be:
\b[A-Za-z0-9._%+-]+@[A-Za-z0-9.-]+\.[A-Za-z]{2,}\bdocs/DATABASE.md (1)
246-258: Verify data retention periods meet compliance requirements.The cleanup function removes requests after 6 months and resolved alerts after 3 months. Verify these periods comply with your organizational retention policies and any applicable regulations (GDPR, SOC2, etc.). If auditing or compliance requires longer retention, these values may need adjustment.
Can you confirm these retention periods align with your data retention and compliance requirements?
- Update alert user mention format in ALERTS.md - Clarify encryption status for prompt and response fields in DATABASE.md - Add security warnings and best practices in DEPLOYMENT.md - Correct regex formatting in DETECTION_RULES.md - Revise password requirements and recovery limitations in USER_MANAGEMENT.md
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (12)
docs/USER_MANAGEMENT.md (3)
119-123: Flagged enhancement need: password recovery limitation should have a timeline.The documentation appropriately identifies that email-based or security-question password recovery is missing, but doesn't specify whether this is planned for a specific release or timeline. Consider adding target version/quarter information to set user expectations.
72-76: Consider clarifying password complexity requirements for production.While minimum 8 characters is specified, production environments typically benefit from explicit complexity rules (e.g., mix of upper/lowercase, digits, special characters). Current guidance is permissive.
84-88: Clarify implications of "multiple concurrent sessions" policy.Allowing multiple concurrent sessions for the same user can pose security risks (e.g., token theft, unauthorized access from multiple locations). Consider documenting monitoring strategies or constraints to mitigate this if it's intentional.
docs/ALERTS.md (2)
199-207: Alert fatigue prevention lacks concrete implementation guidance.While best practices are listed, the section would benefit from specific configuration examples (e.g., suppression rule patterns, escalation policy templates, or alert grouping threshold recommendations).
141-142: Missing alert retention and lifecycle documentation.The docs cover creating, acknowledging, and resolving alerts, but don't specify: How long are alerts retained? What triggers automatic cleanup? How do archival and deletion work? Add clarity on alert lifecycle and retention policies.
docs/DETECTION_RULES.md (3)
49-57: Custom scoring rule syntax needs clarification and examples.The "custom_scoring" rule type documentation uses pseudo-code pattern (
tokens_total > 2000) without explaining the actual implementation language, operators, or how to write custom scoring logic. Add concrete examples and documentation on the supported syntax.
70-73: Combination logic AND/OR behavior needs clarification with examples.The section states AND and OR logic exists but doesn't explain how these work in practice—do they combine patterns within a rule or across multiple rules? Add examples showing both behaviors.
79-87: Default rules table: patterns are difficult to read/copy due to markdown escaping.The markdown escaping of regex patterns (e.g.,
\b[A-Za-z0-9._%+-]+@[A-Za-z0-9.-]+\.[A-Za-z]{2,}\b) makes the table harder to visually parse and copy. Consider moving patterns to a separate reference or use inline code blocks for each pattern.docs/DATABASE.md (1)
267-267: Missing dependency documentation: pg_cron extension requirement not noted.Line 267 uses
cron.schedule()which requires thepg_cronPostgreSQL extension. This dependency should be documented in a prerequisites or setup section, not buried in the cleanup example.docs/DEPLOYMENT.md (3)
309-312: Database user privileges insufficient—missing CREATEDB/CREATEROLE grants.The database user creation (line 309) grants only basic table permissions. Consider whether the production user needs additional privileges such as
CREATEDBorCREATEROLEfor maintenance tasks, or document why these are intentionally restricted.
546-562: Security checklist items lack implementation guidance and references.The security checklist is comprehensive but items are high-level bullets without links to configuration examples or specific guidance. Add inline references or links to corresponding sections/documentation for items like "rate limiting configured," "SQL injection prevention verified," and "security headers configured."
558-558: Missing connection pooling configuration documentation.The security checklist (line 558) mentions "Database connection pooling configured" but no pooling setup is documented (e.g., PgBouncer, pgpool). Either add configuration examples for a recommended pooling solution or link to external documentation.
Do you want me to add a connection pooling configuration section (e.g., PgBouncer setup) to the Deployment Guide, or should this be in a separate document?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
docs/ALERTS.md(1 hunks)docs/DATABASE.md(1 hunks)docs/DEPLOYMENT.md(1 hunks)docs/DETECTION_RULES.md(1 hunks)docs/USER_MANAGEMENT.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/USER_MANAGEMENT.md
[style] ~81-~81: Consider using a different verb to strengthen your wording.
Context: ...e - Regularly review user permissions - Remove access for departed team members - Moni...
(REMOVE_REVOKE)
🪛 markdownlint-cli2 (0.18.1)
docs/DEPLOYMENT.md
8-8: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
docs/DATABASE.md
223-223: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (1)
docs/DATABASE.md (1)
315-341: Verify monitoring query column names for accuracy.The monitoring query at line 338 references
idx_tup_readandidx_tup_fetchinpg_stat_user_indexes. Verify these column names are correct for the PostgreSQL version documented (15+), as column names may vary across versions.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
docs/USER_MANAGEMENT.md (3)
79-82: Consider strengthening verb choice in access control guidance.Line 81 uses "Regularly review user permissions" while the next bullet uses "Remove access." For consistency and stronger intent, consider "Revoke access for departed team members" to emphasize the action rather than review framing.
Example improvement:
- Regularly review user permissions - Remove access for departed team members + Regularly audit user permissions + Revoke access for departed team members(Note: The stronger verb pairs better convey the security action needed.)
24-28: Acknowledge the security trade-off of hardcoded default credentials in documentation.Documenting the default admin credentials (admin/admin123) is necessary for operational guidance, and the
⚠️ warning to change immediately is appropriate. This aligns well with the "Security Best Practices" section.Consider cross-linking or emphasizing that production deployments should use secrets management (e.g., environment variables, Kubernetes Secrets, or vault solutions) rather than relying on defaults.
Is there a corresponding section in
DEPLOYMENT.mdthat covers secrets management at scale? If not, consider adding a brief note here linking to deployment secrets best practices.
84-89: Session management: clarify monitoring guidance for concurrent sessions.The note "Multiple concurrent sessions allowed (monitor for suspicious activity)" is appropriately cautious but somewhat vague. Consider providing concrete indicators of suspicious behavior (e.g., multiple concurrent sessions from different geographic regions, rapid session creation/termination).
docs/DEPLOYMENT.md (2)
117-118: Security warning on.env.prodfile is clear; recommend integration with CI/CD secrets pipeline.The warning against committing
.env.prodto version control is appropriately prominent. Consider adding a brief callout on how to inject secrets during container orchestration (e.g., using Docker secrets, Kubernetes Secret objects, or a dedicated secrets vault). This reinforces the note and provides actionable guidance for operators following this guide.Example enhancement:
⚠️ **Security Warning**: Do NOT commit `.env.prod` to version control. For production, use Kubernetes Secrets, environment variable injection from secure vaults (AWS Secrets Manager, HashiCorp Vault), or container orchestration platform secrets management. + +**Example**: In Kubernetes, reference secrets in your deployment: +```yaml +envFrom: + - secretRef: + name: flagwise-secrets +```
546-563: Security checklist is comprehensive; consider adding incident response and post-mortem procedures.The security checklist covers deployment, encryption, access control, and scanning well. Consider adding reminders for:
- Post-incident review procedures (e.g., log analysis, root-cause analysis templates)
- Security alert thresholds (e.g., failed login attempts, privilege escalation attempts)
- Automated response actions (e.g., automatic account lockout after N failed attempts)
These reinforce the operational security posture beyond static checks.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
docs/ALERTS.md(1 hunks)docs/DATABASE.md(1 hunks)docs/DEPLOYMENT.md(1 hunks)docs/USER_MANAGEMENT.md(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- docs/ALERTS.md
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/DATABASE.md
🧰 Additional context used
🪛 LanguageTool
docs/USER_MANAGEMENT.md
[style] ~81-~81: Consider using a different verb to strengthen your wording.
Context: ...e - Regularly review user permissions - Remove access for departed team members - Moni...
(REMOVE_REVOKE)
🪛 markdownlint-cli2 (0.18.1)
docs/DEPLOYMENT.md
8-8: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (1)
docs/DEPLOYMENT.md (1)
499-504: SQL maintenance tasks correctly use SELECT wrapper for pg_stat_statements_reset().The maintenance procedure at line 503 correctly invokes
SELECT pg_stat_statements_reset();with proper SQL syntax. ✓ This resolves the prior syntax error concern.
1 participant
