diff --git a/.github/workflows/sso-audit.yml b/.github/workflows/sso-audit.yml new file mode 100644 index 0000000000000..f29c9a1dc2be3 --- /dev/null +++ b/.github/workflows/sso-audit.yml @@ -0,0 +1,86 @@ +name: SSO fork audit + +# Runs scripts/sso-audit.sh against this fork's auth code to verify +# satisfaction of the cross-app SSO contract at +# awais786/sso-rules-moneta:openspec/specs/proxy-auth-middleware/spec.md. +# +# Covers fork-side rows 14, 20, 21 from SKILL.md §5. Exits 1 on security- +# critical violations (row 20 — session-identity reconciliation) so the +# merge gate blocks the stale-session leak class of bug. +# +# When upstream sso-rules-moneta adds a new fork-side row, vendor the +# updated check into scripts/sso-audit.sh and re-run this workflow. + +on: + pull_request: + paths: + - 'packages/twenty-server/src/engine/guards/**' + - 'packages/twenty-server/src/engine/core-modules/auth/**' + - 'packages/twenty-front/src/modules/auth/**' + - 'scripts/sso-audit.sh' + - '.github/workflows/sso-audit.yml' + workflow_dispatch: + +permissions: + contents: read + +jobs: + sso-fork-audit: + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@v4 + + - name: Run fork audit + id: audit + run: | + set -o pipefail + if bash scripts/sso-audit.sh | tee audit-output.md; then + echo "audit_exit=0" >> "$GITHUB_OUTPUT" + else + echo "audit_exit=$?" >> "$GITHUB_OUTPUT" + fi + + - name: Publish to job summary + if: always() + run: cat audit-output.md >> "$GITHUB_STEP_SUMMARY" + + - name: Upload audit output + if: always() + uses: actions/upload-artifact@v4 + with: + name: sso-fork-audit-output + path: audit-output.md + if-no-files-found: warn + + - name: Fail on security-critical violations + if: steps.audit.outputs.audit_exit != '0' + run: | + echo "::error::Security-critical SSO contract violation in fork audit. See table for the failing row and fix." + exit 1 + + sso-fork-audit-comment: + # Run comment updates only on same-repo PRs. Fork PR tokens are read-only + # and cannot write PR comments. + if: | + always() && + github.event_name == 'pull_request' && + github.event.pull_request.head.repo.full_name == github.repository + needs: sso-fork-audit + runs-on: ubuntu-latest + permissions: + contents: read + pull-requests: write + steps: + - name: Download audit output + uses: actions/download-artifact@v4 + with: + name: sso-fork-audit-output + path: audit-results + + - name: Post sticky PR comment + continue-on-error: true + uses: marocchino/sticky-pull-request-comment@v2 + with: + header: sso-fork-audit + path: audit-results/audit-output.md diff --git a/scripts/sso-audit.sh b/scripts/sso-audit.sh new file mode 100755 index 0000000000000..4999207d7c5d3 --- /dev/null +++ b/scripts/sso-audit.sh @@ -0,0 +1,257 @@ +#!/usr/bin/env bash +# +# sso-audit.sh — Twenty-side fork audit against the cross-app SSO contract. +# +# ============================================================================ +# Covers the fork-side rows of awais786/sso-rules-moneta:openspec/specs/ +# proxy-auth-middleware/spec.md that a deterministic bash check can verify +# against Twenty's source tree. Catches regressions BEFORE they reach +# foss-server-bundle-devstack, where the same rows currently emit `?` (no +# fork code on disk in the bundle CI). +# +# Together with the bundle-side audit and the per-fork audits in Plane / +# Outline / Penpot / SurfSense, the cross-app contract is now ~14 of 21 +# rows deterministic without any LLM invocation. +# +# Exit codes: +# 0 — all rows ✅ or n/a / informational +# 1 — at least one SECURITY-CRITICAL row failed (today: row 20 session- +# identity reconciliation). These violations re-open the cross-user +# identity-leak class of bug. +# +# Rows covered: +# Row 14 — logout shape: SPA logout file (packages/twenty-front/src/ +# modules/auth/hooks/useAuth.ts) MUST NOT call /oauth2/sign_out. +# (The portal-host redirect target uses buildPortalUrl — verified +# elsewhere; this row only enforces no /oauth2/sign_out literal.) +# Row 20 — session-identity reconciliation (Rule 2 mismatch flush): +# JwtAuthGuard MUST call response.clearCookie('tokenPair', ...) +# on identity mismatch when AUTH_TYPE === 'SSO'. Without this, +# the stale-session-on-user-switch leak returns. SECURITY-CRITICAL. +# Row 21 — email-shape detection MUST NOT use polynomial-backtracking +# regex. Twenty uses indexOf-based detection in +# normalizeProxyEmail today; this row fires only if a regex is +# reintroduced. +# +# Source of truth: awais786/sso-rules-moneta:openspec/specs/proxy-auth- +# middleware/spec.md. When upstream adds a new fork-side row, vendor the +# updated check here. +# ============================================================================ + +set -euo pipefail + +JWT_GUARD="packages/twenty-server/src/engine/guards/jwt-auth.guard.ts" +USE_AUTH="packages/twenty-front/src/modules/auth/hooks/useAuth.ts" +# clearCookie('tokenPair', ...) +CLEAR_COOKIE_PATTERN='clearCookie\(\s*['"'"'"]tokenPair['"'"'"]' +# get('AUTH_TYPE') === 'SSO' +AUTH_TYPE_SSO_PATTERN='get\(\s*['"'"'"]AUTH_TYPE['"'"'"]\s*\)\s*===\s*['"'"'"]SSO['"'"'"]' +MISMATCH_HEADER_PATTERN="X-Auth-Request-Email" +MISMATCH_USER_PATTERN="data\\.user\\.email" +# 25 lines keeps the required signals in one logical guard block while +# tolerating normal formatting/wrapping drift. +PROXIMITY_WINDOW=25 + +declare -a ROW_STATUS=() +declare -a ROW_TITLES=( + "logout shape: SPA logout does not call /oauth2/sign_out" + "session-identity reconciliation present (Rule 2 mismatch flush)" + "email-shape detection uses indexOf, not polynomial regex" +) +declare -a ROW_NOTES=() +declare -a ROW_NUMBERS=(14 20 21) + +# Security-critical row indices (0-indexed into ROW_NUMBERS above). +SECURITY_CRITICAL=(1) # index 1 → row 20 +SECURITY_CRITICAL_FAILS=0 + +record() { + local idx=$1 status=$2 note=$3 + ROW_STATUS[$idx]="$status" + ROW_NOTES[$idx]="$note" + if [[ "$status" == "❌" ]]; then + for c in "${SECURITY_CRITICAL[@]}"; do + if [[ "$c" -eq "$idx" ]]; then + SECURITY_CRITICAL_FAILS=$((SECURITY_CRITICAL_FAILS + 1)) + return + fi + done + fi +} + +# ============================================================================ +# Row 14 (idx 0): logout shape — narrow check +# +# SPA logout MUST NOT contain `/oauth2/sign_out` literal. Twenty's logout +# uses `buildPortalUrl` to compute the redirect target; this audit only +# verifies the SPA doesn't try to clear the upstream proxy cookie itself. +# ============================================================================ +check_row_14() { + if [[ ! -f "$USE_AUTH" ]]; then + record 0 "?" "$USE_AUTH not found — skipping" + return + fi + + if grep -qE '/oauth2/sign_?out' "$USE_AUTH"; then + local line + line=$(grep -nE '/oauth2/sign_?out' "$USE_AUTH" | head -1) + record 0 "❌" "Logout calls \`/oauth2/sign_out\` at $USE_AUTH:$line — per logout-flow spec, per-app Logout is navigation-only. Drop the call; portal 'Logout all' handles oauth2-proxy clearing." + return + fi + + record 0 "✅" "$USE_AUTH does not invoke \`/oauth2/sign_out\` (this row verifies only that the SPA doesn't try to clear the upstream proxy cookie itself; that's the portal's job)" +} + +# ============================================================================ +# Row 20 (idx 1): session-identity reconciliation +# +# JwtAuthGuard MUST clear the tokenPair cookie on identity mismatch: +# response.clearCookie('tokenPair', { path: '/' }) +# +# AND it MUST gate the mismatch check on AUTH_TYPE === 'SSO' so non-SSO +# deployments are unaffected. +# +# Deterministic signal: +# - SSO gate: get('AUTH_TYPE') === 'SSO' +# - mismatch operands: X-Auth-Request-Email + data.user.email +# - token flush: clearCookie('tokenPair', ...) +# All must exist in the same nearby block (within ±PROXIMITY_WINDOW lines), so unrelated +# AUTH_TYPE/clearCookie references elsewhere in the file cannot false-pass. +# +# SECURITY-CRITICAL: without this, the stale-session leak returns. +# ============================================================================ +check_row_20() { + if [[ ! -f "$JWT_GUARD" ]]; then + record 1 "?" "$JWT_GUARD not found — skipping" + return + fi + + local clear_cookie_lines + clear_cookie_lines=$(grep -nE "$CLEAR_COOKIE_PATTERN" "$JWT_GUARD" || true) + + local auth_type_lines + auth_type_lines=$(grep -nE "$AUTH_TYPE_SSO_PATTERN" "$JWT_GUARD" || true) + + local mismatch_header_lines + mismatch_header_lines=$(grep -nE "$MISMATCH_HEADER_PATTERN" "$JWT_GUARD" || true) + + local mismatch_user_lines + mismatch_user_lines=$(grep -nE "$MISMATCH_USER_PATTERN" "$JWT_GUARD" || true) + + if [[ -n "$clear_cookie_lines" && -n "$auth_type_lines" && -n "$mismatch_header_lines" && -n "$mismatch_user_lines" ]]; then + local nearby_match + nearby_match=$( + awk \ + -v clearPattern="$CLEAR_COOKIE_PATTERN" \ + -v authPattern="$AUTH_TYPE_SSO_PATTERN" \ + -v headerPattern="$MISMATCH_HEADER_PATTERN" \ + -v userPattern="$MISMATCH_USER_PATTERN" \ + -v window="$PROXIMITY_WINDOW" ' + $0 ~ clearPattern { clear[NR]=1 } + $0 ~ authPattern { auth[NR]=1 } + $0 ~ headerPattern { header[NR]=1 } + $0 ~ userPattern { user[NR]=1 } + END { + for (line = 1; line <= NR; line++) { + hasClear = 0 + hasAuth = 0 + hasHeader = 0 + hasUser = 0 + for (i = line - window; i <= line + window; i++) { + if (clear[i]) hasClear = 1 + if (auth[i]) hasAuth = 1 + if (header[i]) hasHeader = 1 + if (user[i]) hasUser = 1 + } + if (hasClear && hasAuth && hasHeader && hasUser) { + print 1 + exit + } + } + print 0 + } + ' "$JWT_GUARD" + ) + + if [[ "$nearby_match" == "1" ]]; then + record 1 "✅" "$JWT_GUARD contains nearby SSO gate + identity-mismatch comparison + \`clearCookie('tokenPair'\` (within ±$PROXIMITY_WINDOW lines) — Rule 2 mismatch flush in place" + return + fi + fi + + if [[ -z "$clear_cookie_lines" ]]; then + record 1 "❌" "$JWT_GUARD does NOT call \`response.clearCookie('tokenPair', ...)\`. The cross-app spec (proxy-auth-middleware Rule 2) requires the guard to clear the tokenPair cookie when oauth2-proxy asserts a different identity than the JWT user. Without it, the stale-session-on-user-switch leak returns. Fix: add a comparison of \`X-Auth-Request-Email\` vs \`data.user.email\` after \`validateTokenByRequest\`, and on mismatch call \`response.clearCookie('tokenPair', { path: '/' })\` then return false. Gate the check on \`AUTH_TYPE === 'SSO'\`." + return + fi + + record 1 "❌" "$JWT_GUARD has \`clearCookie('tokenPair'\` but missing a nearby full Rule 2 block: \`get('AUTH_TYPE') === 'SSO'\` + \`X-Auth-Request-Email\` vs \`data.user.email\` comparison in the same conditional path. Keep these checks colocated to avoid false passes from unrelated references." +} + +# ============================================================================ +# Row 21 (idx 2): polynomial-regex avoidance in email-shape detection +# +# Twenty uses indexOf-based detection in normalizeProxyEmail (added in +# Pressingly/twenty#8). This is a regression guard — fires only if a +# polynomial-backtracking regex like /^[^\s@]+@[^\s@]+\.[^\s@]+$/ is +# reintroduced. CodeQL's `js/polynomial-redos` flags it on adversarial input. +# ============================================================================ +check_row_21() { + if [[ ! -f "$JWT_GUARD" ]]; then + record 2 "?" "$JWT_GUARD not found — skipping" + return + fi + + # Look for the canonical polynomial-backtracking shape used in JS regex + # literals: /^[^\s@]+@[^\s@]+\.[^\s@]+$/ and variations. + local hits + hits=$(grep -nE '\[\^[\\\\]s@\]\+@\[\^[\\\\]s@\]\+\\\.\[\^[\\\\]s@\]\+' "$JWT_GUARD" 2>/dev/null || true) + + if [[ -n "$hits" ]]; then + record 2 "❌" "Polynomial-backtracking email-shape regex detected in $JWT_GUARD: $hits. Rewrite to indexOf-based check per openspec proxy-auth-middleware §'email-shape detection SHALL avoid polynomial-backtracking regex'. Reference: \`normalizeProxyEmail\` in this file." + return + fi + + record 2 "✅" "No polynomial-backtracking email-shape regex in $JWT_GUARD; using indexOf-based detection" +} + +# ============================================================================ +# Run checks +# ============================================================================ +check_row_14 +check_row_20 +check_row_21 + +# ============================================================================ +# Print table +# ============================================================================ +echo "## Twenty SSO Fork Audit" +echo +echo "Cross-app contract: https://github.com/awais786/sso-rules-moneta/blob/main/openspec/specs/proxy-auth-middleware/spec.md" +echo "Row numbers match the 21-row table at https://github.com/awais786/sso-rules-moneta/blob/main/skills/app-rules/SKILL.md#5-report" +echo +echo "| Row | Invariant | Status | Notes |" +echo "|-----|-----------|--------|-------|" +for i in "${!ROW_TITLES[@]}"; do + printf "| %d | %s | %s | %s |\n" \ + "${ROW_NUMBERS[$i]}" "${ROW_TITLES[$i]}" "${ROW_STATUS[$i]:-?}" "${ROW_NOTES[$i]:-}" +done +echo + +# ============================================================================ +# Summary + exit code +# ============================================================================ +TOTAL_FAILS=0 +for s in "${ROW_STATUS[@]}"; do + [[ "$s" == "❌" ]] && TOTAL_FAILS=$((TOTAL_FAILS + 1)) +done + +if [[ "$TOTAL_FAILS" -eq 0 ]]; then + echo "**All fork-side invariants hold.**" + exit 0 +fi + +echo "**$TOTAL_FAILS violations.** Security-critical (row 20): $SECURITY_CRITICAL_FAILS." +if [[ "$SECURITY_CRITICAL_FAILS" -gt 0 ]]; then + exit 1 +fi +exit 0