Skip to content

fix: Track C audit remediation — security hardening#34

Merged
henrylove0 merged 1 commit into
mainfrom
tom-audit-cf
Jun 9, 2026
Merged

fix: Track C audit remediation — security hardening#34
henrylove0 merged 1 commit into
mainfrom
tom-audit-cf

Conversation

@henrylove0

Copy link
Copy Markdown
Contributor

Summary

  • P1-C1: Documented CRM schema contract in docs/SCHEMA-CONTRACT.md. Twenty uses per-workspace schemas (workspace_<base36_uuid>) with singular table names (person, company). No crm.people or crm.companies tables exist — external systems (gateway, ERP) must use the correct names.
  • P1-C2: Hardened admin-token middleware — SHA-256 hash at startup, crypto.timingSafeEqual for comparison, sliding-window rate limiter (10 attempts/min/IP), audit logging on every accept/reject.
  • P1-C3: Stopped leaking GoTrue error details to clients — returns generic "Authentication failed", logs full details server-side. Also hardened admin-token endpoint in gotrue-auth.controller.ts with timing-safe comparison.
  • P2-C1: Removed default Redis password (exe-crm-redis-default) — now requires explicit REDIS_PASSWORD via ${REDIS_PASSWORD:?Set REDIS_PASSWORD} in all compose references.
  • P2-C2: Added GPG encryption for backup SQL dumps using EXE_BACKUP_KEY env var (AES-256 symmetric). Warns if key not set but does not block backups.

Test plan

  • Verify admin-token middleware rejects invalid tokens with timing-safe comparison
  • Verify rate limiter blocks after 10 failed attempts from same IP within 60s
  • Verify GoTrue login returns generic error message on auth failure (no error_description leak)
  • Verify docker compose config fails without REDIS_PASSWORD set
  • Verify backup container encrypts dumps when EXE_BACKUP_KEY is set
  • Verify backup container still works (unencrypted) when EXE_BACKUP_KEY is unset

🤖 Generated with Claude Code

P1-C1: Document CRM schema contract — Twenty uses workspace-scoped schemas
(workspace_<base36_uuid>) with singular table names (person, company), not
people/companies. No compatibility views needed.

P1-C2: Harden admin-token middleware — hash token at startup (SHA-256),
use crypto.timingSafeEqual for comparison, add sliding-window rate limiter
(10 attempts/min/IP), audit log on every accept/reject.

P1-C3: Stop leaking GoTrue error details — return generic "Authentication
failed" to clients, log full error server-side. Also harden admin-token
endpoint in gotrue-auth controller with timing-safe comparison.

P2-C1: Remove default Redis password — require explicit REDIS_PASSWORD
via ${REDIS_PASSWORD:?Set REDIS_PASSWORD} in all compose references.

P2-C2: Encrypt backup SQL dumps — add optional GPG symmetric encryption
using EXE_BACKUP_KEY env var (AES-256). Warns if key not set.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 704c0f9971

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

return;
}

this.rateLimiter.record(clientIp);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Limit only failed admin-token attempts

When a client uses the configured admin Bearer token for GraphQL/REST/MCP calls, every successful request is recorded before the token comparison, so the 11th valid request from the same IP within 60 seconds is treated as rate-limited and falls through to the normal auth middleware without req.adminTokenAuthenticated. Since the admin token is not a JWT/API key, legitimate integrations making more than 10 admin-token API calls per minute will start receiving auth failures; record attempts only after a failed comparison or exempt accepted tokens.

Useful? React with 👍 / 👎.

Comment thread docs/SCHEMA-CONTRACT.md
| `companyId` | `uuid` | FK to `company.id` |
| `createdAt` | `timestamptz` | |
| `deletedAt` | `timestamptz` | Soft-delete |

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Correct the documented company domain column

The schema contract tells gateway/ERP callers to query workspace_<id>.company."domainNameUrl", but the standard company metadata and duplicate criteria use the LINKS subfield column domainNamePrimaryLinkUrl (with related domainNamePrimaryLinkLabel/domainNameSecondaryLinks), not domainNameUrl. External systems following this new contract will select a non-existent column and fail when reading company domains.

Useful? React with 👍 / 👎.

@henrylove0 henrylove0 closed this Jun 9, 2026
@henrylove0 henrylove0 deleted the tom-audit-cf branch June 9, 2026 22:36
@henrylove0 henrylove0 restored the tom-audit-cf branch June 9, 2026 23:22
@henrylove0 henrylove0 reopened this Jun 9, 2026
@henrylove0 henrylove0 merged commit fd2382a into main Jun 9, 2026
2 of 3 checks passed
@henrylove0 henrylove0 deleted the tom-audit-cf branch June 9, 2026 23:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant