From 139daf4ae56d0560a3d5e104d816a989865fa2fb Mon Sep 17 00:00:00 2001 From: awais786 Date: Sat, 16 May 2026 14:27:31 +0500 Subject: [PATCH 01/10] chore(ci): add fork-side SSO audit script + workflow MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a per-fork deterministic audit that catches regressions of the cross-app SSO contract in Pressingly/plane BEFORE they reach foss-server-bundle-devstack. The bundle's CI runs an audit that emits `?` for fork-side rows because the forks aren't checked out there; this PR closes that gap from Plane's side. What it checks scripts/sso-audit.sh runs three deterministic checks against the files in this fork that satisfy fork-side rows of awais786/sso-rules-moneta:openspec/specs/proxy-auth-middleware/spec.md: Row 14 — logout shape: apps/web/core/store/user/index.ts MUST NOT call /oauth2/sign_out. Per logout-flow spec the per-app button is navigation-only; portal "Logout all" handles oauth2-proxy clearing. Row 20 — session-identity reconciliation (Rule 2 mismatch flush): apps/api/plane/authentication/middleware/proxy_auth.py MUST import django.contrib.auth.logout AND invoke logout(request). Without it, the stale-session-on-user- switch leak returns. SECURITY-CRITICAL — exit 1 on miss. Row 21 — polynomial-regex avoidance: proxy_auth.py + proxy_auth_utils.py MUST NOT introduce an unanchored [^\s@]+@[^\s@]+\.[^\s@]+ regex. Plane uses substring check today, so this is a regression guard. When upstream sso-rules-moneta adds new fork-side rows, vendor the updated check by editing this script. How it's wired .github/workflows/sso-audit.yml runs the script on: - pull_request paths: apps/api/plane/authentication/**, apps/web's auth-related modules, the script itself, the workflow itself - push to foss-main - weekly schedule (Mon 09:00 UTC) - manual workflow_dispatch Output is published in three places: - GitHub job summary (always) - Sticky PR comment via marocchino/sticky-pull-request-comment@v2 (PR runs only — one comment per PR, updated on each push) - Exit code 1 on security-critical violations → merge blocked Local dry-run On foss-main (pre-PR-29 state): row 20 fires ❌ because proxy_auth.py lacks the django_auth.logout import. Exit 1. Confirms the gate works. On fix/proxy-auth-stale-session-on-user-switch: all three rows ✅. Confirms the gate releases when the fix lands. Note: the audit script lives at scripts/sso-audit.sh which was otherwise gitignored under `scripts/`. The .gitignore is updated to a more specific `scripts/*` glob with a `!scripts/sso-audit.sh` negation so this single file can be tracked while leaving the rest of scripts/ ignored. Co-Authored-By: Claude Opus 4.7 (1M context) --- .github/workflows/sso-audit.yml | 68 ++++++++++ .gitignore | 4 +- scripts/sso-audit.sh | 218 ++++++++++++++++++++++++++++++++ 3 files changed, 289 insertions(+), 1 deletion(-) create mode 100644 .github/workflows/sso-audit.yml create mode 100755 scripts/sso-audit.sh diff --git a/.github/workflows/sso-audit.yml b/.github/workflows/sso-audit.yml new file mode 100644 index 00000000000..8956d37c0b6 --- /dev/null +++ b/.github/workflows/sso-audit.yml @@ -0,0 +1,68 @@ +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 the fork-side rows that foss-server-bundle-devstack's CI can't +# check (no fork code on disk in the bundle): today 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: + - 'apps/api/plane/authentication/**' + - 'apps/web/core/store/user/**' + - 'apps/web/core/services/auth.service.ts' + - 'apps/web/core/lib/wrappers/authentication-wrapper.tsx' + - 'scripts/sso-audit.sh' + - '.github/workflows/sso-audit.yml' + push: + branches: [foss-main] + schedule: + # 09:00 UTC every Monday — pre-week sanity check + - cron: '0 9 * * 1' + workflow_dispatch: + +permissions: + contents: read + pull-requests: write + +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: Post sticky PR comment + if: github.event_name == 'pull_request' && always() + uses: marocchino/sticky-pull-request-comment@v2 + with: + header: sso-fork-audit + path: audit-output.md + + - 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 diff --git a/.gitignore b/.gitignore index 541cc29f087..96245b105a9 100644 --- a/.gitignore +++ b/.gitignore @@ -109,7 +109,9 @@ build/ build/ .react-router/ temp/ -scripts/ +scripts/* +# Tracked exceptions inside scripts/ — vendored or fork-specific CI tooling. +!scripts/sso-audit.sh # SSO design specs (implemented, code is source of truth) docs/cognito_*.md diff --git a/scripts/sso-audit.sh b/scripts/sso-audit.sh new file mode 100755 index 00000000000..94b6cd50a32 --- /dev/null +++ b/scripts/sso-audit.sh @@ -0,0 +1,218 @@ +#!/usr/bin/env bash +# +# sso-audit.sh — Plane-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 Plane'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 (foss-server-bundle-devstack/scripts/ +# audit-sso.sh), the contract is now ~14 of 21 rows deterministic without +# any LLM invocation. The remaining rows (identity-managed UI gating, +# display-name UUID check) need either runtime SSO state or AST-level +# semantic analysis — those defer to the LLM-backed +# /sso-rules:audit-all-apps slash command. +# +# 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 MUST NOT call /oauth2/sign_out, MUST +# redirect to portal host (env-supplied or 4-label regex), MUST +# NOT POST /auth/sign-out/ (per logout-flow spec §"Per-app +# 'Logout' SHALL be navigation-only"; tolerated as drift today). +# Row 20 — session-identity reconciliation (Rule 2 mismatch flush): +# proxy_auth.py MUST call django.contrib.auth.logout(request) +# on identity mismatch. Without this, the stale-session-on-user- +# switch leak returns. SECURITY-CRITICAL. +# Row 21 — email-shape detection MUST NOT use polynomial-backtracking +# regex. Plane uses substring check today, so this row is +# informational — the audit 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 + +PROXY_AUTH="apps/api/plane/authentication/middleware/proxy_auth.py" +PROXY_AUTH_UTILS="apps/api/plane/authentication/middleware/proxy_auth_utils.py" +LOGOUT_SPA="apps/web/core/store/user/index.ts" + +# Output state — populated by each check_row_N function, printed at the end. +declare -a ROW_STATUS=() +declare -a ROW_TITLES=( + "logout shape: no /oauth2/sign_out, navigation-only redirect" + "session-identity reconciliation present (Rule 2 mismatch flush)" + "email-shape detection uses substring/indexOf, not polynomial regex" +) +declare -a ROW_NOTES=() + +# Row numbers per the cross-app SKILL.md §5 table. We only emit a subset +# here; bundle CI emits 1-21. +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 +# +# SPA logout MUST NOT contain `/oauth2/sign_out` literal — the bundle's +# logout model is navigation-only. Per logout-flow §"Per-app 'Logout' SHALL +# be navigation-only", a future ideal also drops the POST /auth/sign-out/ +# call, but that's tolerated drift today, so we don't fail on it. +# ============================================================================ +check_row_14() { + if [[ ! -f "$LOGOUT_SPA" ]]; then + record 0 "?" "$LOGOUT_SPA not found — skipping" + return + fi + + if grep -qE '/oauth2/sign_?out' "$LOGOUT_SPA"; then + local line + line=$(grep -nE '/oauth2/sign_?out' "$LOGOUT_SPA" | head -1) + record 0 "❌" "Logout calls \`/oauth2/sign_out\` at $LOGOUT_SPA:$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 "✅" "$LOGOUT_SPA does not invoke \`/oauth2/sign_out\` — navigation-only logout shape preserved" +} + +# ============================================================================ +# Row 20 (idx 1): session-identity reconciliation +# +# proxy_auth.py MUST call django.contrib.auth.logout(request) (imported as +# `logout` or aliased) on identity mismatch. The presence of the import + +# call is the deterministic signal; the bash audit can't verify the call +# site's *position* relative to the mismatch detection, only that it exists. +# That's a spec-conformance approximation; the test suite at +# apps/api/plane/authentication/tests/test_proxy_auth.py pins the exact +# behaviour. +# +# SECURITY-CRITICAL: without this call, the stale-session leak returns. +# ============================================================================ +check_row_20() { + if [[ ! -f "$PROXY_AUTH" ]]; then + record 1 "?" "$PROXY_AUTH not found — skipping" + return + fi + + # Detect either `from django.contrib.auth import logout` (any form) or + # an aliased import that brings `logout` into scope, plus a call site. + local has_import + has_import=$(grep -cE '^from django\.contrib\.auth import (.*\b)?logout(\b.*)?$|^from django\.contrib\.auth import logout as ' "$PROXY_AUTH" || true) + + local has_call + has_call=$(grep -cE '\blogout\(request\)|\bdjango_logout\(request\)' "$PROXY_AUTH" || true) + + if [[ "$has_import" -gt 0 && "$has_call" -gt 0 ]]; then + record 1 "✅" "django.contrib.auth.logout imported and invoked at least once in $PROXY_AUTH — Rule 2 mismatch flush in place" + return + fi + + if [[ "$has_import" -eq 0 ]]; then + record 1 "❌" "$PROXY_AUTH does NOT import django.contrib.auth.logout. The cross-app spec (proxy-auth-middleware Rule 2) requires the middleware to call logout(request) when the proxy header asserts a different identity than the existing Django session. Without it, the stale-session-on-user-switch leak returns. Fix: add \`from django.contrib.auth import logout\` (or aliased) AND invoke \`logout(request)\` immediately on mismatch, before any bail-out path." + return + fi + + record 1 "❌" "$PROXY_AUTH imports django.contrib.auth.logout but never calls it. Fix: invoke \`logout(request)\` on identity mismatch before falling through to the unauthenticated path." +} + +# ============================================================================ +# Row 21 (idx 2): polynomial-regex avoidance in email-shape detection +# +# Plane uses substring check (`"@" in email`) today, so this is a regression +# guard. The audit fires only if a Python regex matching the unanchored +# `[^\s@]+@[^\s@]+\.[^\s@]+` pattern is introduced. CodeQL's +# `py/polynomial-redos` (or `js/polynomial-redos`) flags this on adversarial +# input. +# ============================================================================ +check_row_21() { + local files=() + [[ -f "$PROXY_AUTH" ]] && files+=("$PROXY_AUTH") + [[ -f "$PROXY_AUTH_UTILS" ]] && files+=("$PROXY_AUTH_UTILS") + + if [[ ${#files[@]} -eq 0 ]]; then + record 2 "?" "No proxy_auth files found — skipping" + return + fi + + # Look for the canonical polynomial-backtracking shape: + # [^\s@]+@[^\s@]+\.[^\s@]+ + # in any of the inspected files. Allow trivial whitespace variations. + local hits + hits=$(grep -nE '\[\^[\\\\]s@\]\+@\[\^[\\\\]s@\]\+\\\.\[\^[\\\\]s@\]\+' "${files[@]}" 2>/dev/null || true) + + if [[ -n "$hits" ]]; then + record 2 "❌" "Polynomial-backtracking email-shape regex detected: $hits. Rewrite to indexOf/substring-based check per openspec proxy-auth-middleware §'email-shape detection SHALL avoid polynomial-backtracking regex'. Reference: Outline's normalizeProxyEmail in Pressingly/outline#19." + return + fi + + record 2 "✅" "No polynomial-backtracking email-shape regex in ${files[*]}; using substring check or other O(n) detection" +} + +# ============================================================================ +# Run checks +# ============================================================================ +check_row_14 +check_row_20 +check_row_21 + +# ============================================================================ +# Print table +# ============================================================================ +echo "## Plane SSO Fork Audit" +echo +echo "Cross-app contract: \`awais786/sso-rules-moneta:openspec/specs/proxy-auth-middleware/spec.md\`" +echo "Row numbers match \`skills/app-rules/SKILL.md\` §5 (the 21-row table)." +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 From 4fb4805b556b3466bf51818fc7c555b0e09f7d8a Mon Sep 17 00:00:00 2001 From: awais786 Date: Sat, 16 May 2026 14:46:49 +0500 Subject: [PATCH 02/10] chore(ci): address four Copilot review comments on the fork audit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All four comments from the PR #32 review are valid. Fixes: #1 (sso-audit.sh:29) — Row 14 description claimed three checks (no /oauth2/sign_out, portal-host redirect, no POST /auth/sign-out/) but the function only verifies the first. Narrows the comment to match what's actually implemented. The other two properties from logout-flow spec need runtime context or AST analysis; not in scope for this deterministic gate. #2 (sso-auth.sh:131) — Row 20 detection was brittle: - Single-line regex missed parenthesized multiline imports (`from django.contrib.auth import (\n login,\n logout,\n)`) - Call regex required exact `logout(request)` — missed whitespace (`logout( request )`), keyword form (`logout(request=request)`), and aliased call sites (`auth_logout(request)`) Could false-fail on legitimate fixes → block merges spuriously. New detection uses Python's `ast` module to parse the proxy_auth.py import block and extract the in-scope name for django.contrib.auth.logout (alias or plain). Then a whitespace-tolerant grep checks for a call to that name. Handles every variant in one pass without writing multi-line regex in bash. Verified against a synthetic fixture with aliased + parenthesized + whitespaced call. #3 (sso-audit.sh:191) — Output referenced `skills/app-rules/SKILL.md`, which lives in awais786/sso-rules-moneta, not in Pressingly/plane. Confused readers of the sticky PR comment. Replaced both repo- relative references with full GitHub URLs so the links work from the PR view. #4 (.github/workflows/sso-audit.yml:62) — `marocchino/sticky-pull- request-comment@v2` needs `pull-requests: write`. PRs from external forks get a read-only GITHUB_TOKEN regardless of declared permissions, and the action errors out, marking the whole workflow failed even when the audit passed. Two-layer fix: - Skip the comment step on fork PRs via `github.event.pull_request.head.repo.full_name == github.repository` - Add `continue-on-error: true` as a defensive belt so other comment-posting failures (rate limit, GitHub API blip) don't fail the audit gate either The audit gate itself (final exit-1 step) is unaffected — still blocks merge on security-critical violations regardless of comment posting. Local dry-run on foss-main still produces the expected red ❌ on row 20 with the new alias-aware detection. Switching to the fix branch detects the import (as `logout`) and the call site correctly. Co-Authored-By: Claude Opus 4.7 (1M context) --- .github/workflows/sso-audit.yml | 13 +++++- scripts/sso-audit.sh | 82 +++++++++++++++++++++++---------- 2 files changed, 69 insertions(+), 26 deletions(-) diff --git a/.github/workflows/sso-audit.yml b/.github/workflows/sso-audit.yml index 8956d37c0b6..fe6a9214741 100644 --- a/.github/workflows/sso-audit.yml +++ b/.github/workflows/sso-audit.yml @@ -55,7 +55,18 @@ jobs: run: cat audit-output.md >> "$GITHUB_STEP_SUMMARY" - name: Post sticky PR comment - if: github.event_name == 'pull_request' && always() + # Skip on fork PRs — GitHub provides a read-only token there even + # when the workflow declares `pull-requests: write`, and the + # marocchino action would fail and mark the whole workflow red + # even if the audit itself passed. Same-repo branch PRs always + # have write access. `continue-on-error` is a defensive belt — if + # the comment-posting fails for any other reason (rate limit, etc.) + # the audit gate itself is unaffected. + if: | + github.event_name == 'pull_request' && + github.event.pull_request.head.repo.full_name == github.repository && + always() + continue-on-error: true uses: marocchino/sticky-pull-request-comment@v2 with: header: sso-fork-audit diff --git a/scripts/sso-audit.sh b/scripts/sso-audit.sh index 94b6cd50a32..d36515b0b37 100755 --- a/scripts/sso-audit.sh +++ b/scripts/sso-audit.sh @@ -23,10 +23,12 @@ # identity-leak class of bug. # # Rows covered: -# Row 14 — logout shape: SPA logout MUST NOT call /oauth2/sign_out, MUST -# redirect to portal host (env-supplied or 4-label regex), MUST -# NOT POST /auth/sign-out/ (per logout-flow spec §"Per-app -# 'Logout' SHALL be navigation-only"; tolerated as drift today). +# Row 14 — logout shape: SPA logout file MUST NOT call /oauth2/sign_out. +# (The portal-host redirect target and the no-POST-/auth/sign-out/ +# properties from logout-flow spec are NOT checked here — they +# need either runtime context or AST-level analysis. The bash +# audit covers only the literal /oauth2/sign_out grep, which is +# the most common regression vector.) # Row 20 — session-identity reconciliation (Rule 2 mismatch flush): # proxy_auth.py MUST call django.contrib.auth.logout(request) # on identity mismatch. Without this, the stale-session-on-user- @@ -49,7 +51,7 @@ LOGOUT_SPA="apps/web/core/store/user/index.ts" # Output state — populated by each check_row_N function, printed at the end. declare -a ROW_STATUS=() declare -a ROW_TITLES=( - "logout shape: no /oauth2/sign_out, navigation-only redirect" + "logout shape: SPA logout does not call /oauth2/sign_out" "session-identity reconciliation present (Rule 2 mismatch flush)" "email-shape detection uses substring/indexOf, not polynomial regex" ) @@ -79,12 +81,13 @@ record() { } # ============================================================================ -# Row 14 (idx 0): logout shape +# Row 14 (idx 0): logout shape — narrow check # # SPA logout MUST NOT contain `/oauth2/sign_out` literal — the bundle's -# logout model is navigation-only. Per logout-flow §"Per-app 'Logout' SHALL -# be navigation-only", a future ideal also drops the POST /auth/sign-out/ -# call, but that's tolerated drift today, so we don't fail on it. +# logout model is navigation-only at the per-app layer. The portal handles +# oauth2-proxy clearing. This check enforces only that property; the other +# properties from logout-flow spec (portal-host redirect target, no POST +# /auth/sign-out/) are not verified here. # ============================================================================ check_row_14() { if [[ ! -f "$LOGOUT_SPA" ]]; then @@ -105,14 +108,26 @@ check_row_14() { # ============================================================================ # Row 20 (idx 1): session-identity reconciliation # -# proxy_auth.py MUST call django.contrib.auth.logout(request) (imported as -# `logout` or aliased) on identity mismatch. The presence of the import + +# proxy_auth.py MUST call django.contrib.auth.logout (imported as `logout` or +# under any local alias) on identity mismatch. The presence of the import + # call is the deterministic signal; the bash audit can't verify the call # site's *position* relative to the mismatch detection, only that it exists. # That's a spec-conformance approximation; the test suite at # apps/api/plane/authentication/tests/test_proxy_auth.py pins the exact # behaviour. # +# The detection is resilient to: +# - whitespace around the argument: `logout(request)`, `logout( request )`, +# `logout(\nrequest\n)` +# - keyword form: `logout(request=request)` +# - parenthesized multiline imports: +# from django.contrib.auth import ( +# login, +# logout, +# ) +# - aliased imports: `from django.contrib.auth import logout as django_logout` +# - mixed imports on one line: `from django.contrib.auth import login, logout` +# # SECURITY-CRITICAL: without this call, the stale-session leak returns. # ============================================================================ check_row_20() { @@ -121,25 +136,42 @@ check_row_20() { return fi - # Detect either `from django.contrib.auth import logout` (any form) or - # an aliased import that brings `logout` into scope, plus a call site. - local has_import - has_import=$(grep -cE '^from django\.contrib\.auth import (.*\b)?logout(\b.*)?$|^from django\.contrib\.auth import logout as ' "$PROXY_AUTH" || true) - - local has_call - has_call=$(grep -cE '\blogout\(request\)|\bdjango_logout\(request\)' "$PROXY_AUTH" || true) + # Resolve the in-scope name for django.contrib.auth.logout. + # If the file imports it under an alias, use that. Otherwise default to + # `logout`. Use python -c so we correctly handle multiline parenthesized + # imports without trying to write a multi-line regex in bash. + local logout_name + logout_name=$(python3 - "$PROXY_AUTH" <<'PY' 2>/dev/null || true +import ast, sys +src = open(sys.argv[1]).read() +try: + tree = ast.parse(src) +except SyntaxError: + sys.exit(0) +for node in ast.walk(tree): + if isinstance(node, ast.ImportFrom) and node.module == "django.contrib.auth": + for alias in node.names: + if alias.name == "logout": + print(alias.asname or "logout") + sys.exit(0) +PY +) - if [[ "$has_import" -gt 0 && "$has_call" -gt 0 ]]; then - record 1 "✅" "django.contrib.auth.logout imported and invoked at least once in $PROXY_AUTH — Rule 2 mismatch flush in place" + if [[ -z "$logout_name" ]]; then + record 1 "❌" "$PROXY_AUTH does NOT import django.contrib.auth.logout. The cross-app spec (proxy-auth-middleware Rule 2) requires the middleware to call logout(request) when the proxy header asserts a different identity than the existing Django session. Without it, the stale-session-on-user-switch leak returns. Fix: add \`from django.contrib.auth import logout\` AND invoke \`logout(request)\` immediately on mismatch, before any bail-out path." return fi - if [[ "$has_import" -eq 0 ]]; then - record 1 "❌" "$PROXY_AUTH does NOT import django.contrib.auth.logout. The cross-app spec (proxy-auth-middleware Rule 2) requires the middleware to call logout(request) when the proxy header asserts a different identity than the existing Django session. Without it, the stale-session-on-user-switch leak returns. Fix: add \`from django.contrib.auth import logout\` (or aliased) AND invoke \`logout(request)\` immediately on mismatch, before any bail-out path." + # Look for a call to (...) anywhere in the file. The argument + # can include whitespace, line breaks, or `request=request` keyword form; + # the audit only cares that the call exists, not its exact shape. + local call_pattern="\\b${logout_name}[[:space:]]*\\(" + if grep -qE "$call_pattern" "$PROXY_AUTH"; then + record 1 "✅" "django.contrib.auth.logout imported (as \`$logout_name\`) and invoked at least once in $PROXY_AUTH — Rule 2 mismatch flush in place" return fi - record 1 "❌" "$PROXY_AUTH imports django.contrib.auth.logout but never calls it. Fix: invoke \`logout(request)\` on identity mismatch before falling through to the unauthenticated path." + record 1 "❌" "$PROXY_AUTH imports django.contrib.auth.logout (as \`$logout_name\`) but never calls it. Fix: invoke \`$logout_name(request)\` on identity mismatch before falling through to the unauthenticated path." } # ============================================================================ @@ -187,8 +219,8 @@ check_row_21 # ============================================================================ echo "## Plane SSO Fork Audit" echo -echo "Cross-app contract: \`awais786/sso-rules-moneta:openspec/specs/proxy-auth-middleware/spec.md\`" -echo "Row numbers match \`skills/app-rules/SKILL.md\` §5 (the 21-row table)." +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 "|-----|-----------|--------|-------|" From ece9f698412b7d486b745cdbf5177bf3731eb490 Mon Sep 17 00:00:00 2001 From: awais786 Date: Sat, 16 May 2026 14:54:37 +0500 Subject: [PATCH 03/10] chore(ci): use portable word boundary in row 20 call detection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Copilot review on PR #32 flagged that the `\b` word-boundary in the row-20 grep is a GNU `grep -E` extension and undefined on BSD / POSIX- strict implementations. macOS / Alpine / busybox grep may interpret it as literal backspace or literal `b`, causing the audit to silently miss real `logout(...)` calls and false-fail rows that should pass. Replaced with POSIX character-class boundary: (^|[^[:alnum:]_])[[:space:]]*\( `(^|[^[:alnum:]_])` matches start-of-line OR any non-word character before the alias name — semantically identical to `\b` at the left edge, but using only ERE constructs that work in every grep implementation. The right edge doesn't need a boundary because `[[:space:]]*\(` already requires the next non-space character to be `(`, which disambiguates `logout(` from `logout_more(`. Verified against six fixtures: - `logout(request)` → match ✅ - `auth_logout( request )` → match ✅ - ` logout(request=request)` → match ✅ - `my_xauth_logout(request)` vs alias=auth_logout → skip ✅ - leading start-of-line → match ✅ - `something_logout(request)` vs alias=logout → skip ✅ The fifth Copilot comment is the last open one; the previous four are addressed by commit 4fb4805b55. Co-Authored-By: Claude Opus 4.7 (1M context) --- scripts/sso-audit.sh | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/scripts/sso-audit.sh b/scripts/sso-audit.sh index d36515b0b37..473f3137c33 100755 --- a/scripts/sso-audit.sh +++ b/scripts/sso-audit.sh @@ -165,7 +165,15 @@ PY # Look for a call to (...) anywhere in the file. The argument # can include whitespace, line breaks, or `request=request` keyword form; # the audit only cares that the call exists, not its exact shape. - local call_pattern="\\b${logout_name}[[:space:]]*\\(" + # + # Portable word boundary at the start: `(^|[^[:alnum:]_])` matches either + # start-of-line or any non-word character before the alias name. Using + # POSIX character classes rather than `\b` because `\b` is a GNU extension + # to `grep -E` and is undefined on BSD/POSIX strict implementations + # (`grep` on macOS, busybox, Alpine, etc. may interpret it as literal + # backspace or just `b`). The right side doesn't need a boundary because + # `[[:space:]]*\(` already requires the next non-space char to be `(`. + local call_pattern="(^|[^[:alnum:]_])${logout_name}[[:space:]]*\\(" if grep -qE "$call_pattern" "$PROXY_AUTH"; then record 1 "✅" "django.contrib.auth.logout imported (as \`$logout_name\`) and invoked at least once in $PROXY_AUTH — Rule 2 mismatch flush in place" return From ce67aa6025e996bf7436c1a569c5563ede7d2755 Mon Sep 17 00:00:00 2001 From: awais786 Date: Sat, 16 May 2026 14:59:01 +0500 Subject: [PATCH 04/10] =?UTF-8?q?chore(ci):=20tighten=20Row=2014=20success?= =?UTF-8?q?=20message=20=E2=80=94=20clarify=20what=20is/isn't=20verified?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous message "navigation-only logout shape preserved" overstated what the check actually verifies. The audit only confirms the SPA does NOT call /oauth2/sign_out. It does NOT verify: - whether the SPA POSTs /auth/sign-out/ (it does today — tolerated drift) - what host the SPA navigates to after logout - whether portal "Logout all" is wired correctly The new message states exactly what's checked and acknowledges the tolerated POST /auth/sign-out/ drift so readers don't misread the ✅ as "per-app Logout is fully spec-conformant." Co-Authored-By: Claude Opus 4.7 (1M context) --- scripts/sso-audit.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/sso-audit.sh b/scripts/sso-audit.sh index 473f3137c33..09fd4e782af 100755 --- a/scripts/sso-audit.sh +++ b/scripts/sso-audit.sh @@ -102,7 +102,7 @@ check_row_14() { return fi - record 0 "✅" "$LOGOUT_SPA does not invoke \`/oauth2/sign_out\` — navigation-only logout shape preserved" + record 0 "✅" "$LOGOUT_SPA 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. The SPA MAY still POST \`/auth/sign-out/\` — tolerated drift, see logout-flow spec)" } # ============================================================================ From 19d580a830f03881b1b776d1113ff38c4c274a5b Mon Sep 17 00:00:00 2001 From: Awais Qureshi Date: Sat, 16 May 2026 15:22:41 +0500 Subject: [PATCH 05/10] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- scripts/sso-audit.sh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/scripts/sso-audit.sh b/scripts/sso-audit.sh index 09fd4e782af..464d1dad759 100755 --- a/scripts/sso-audit.sh +++ b/scripts/sso-audit.sh @@ -17,7 +17,8 @@ # /sso-rules:audit-all-apps slash command. # # Exit codes: -# 0 — all rows ✅ or n/a / informational +# 0 — no SECURITY-CRITICAL rows failed. Non-security rows may still be +# reported as ❌; those findings are informational/non-gating. # 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. From 8a1b1180a397d49f03932672156a2f3c13bcaeab Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 16 May 2026 10:27:45 +0000 Subject: [PATCH 06/10] chore(ci): split sso audit comment into least-privilege job Agent-Logs-Url: https://github.com/Pressingly/plane/sessions/69fdbaa7-1abb-4dcd-9d1a-04cb7edd4fd8 --- .github/workflows/sso-audit.yml | 44 +++++++++++++++++++++------------ 1 file changed, 28 insertions(+), 16 deletions(-) diff --git a/.github/workflows/sso-audit.yml b/.github/workflows/sso-audit.yml index fe6a9214741..663942e84ef 100644 --- a/.github/workflows/sso-audit.yml +++ b/.github/workflows/sso-audit.yml @@ -31,7 +31,6 @@ on: permissions: contents: read - pull-requests: write jobs: sso-fork-audit: @@ -54,22 +53,11 @@ jobs: if: always() run: cat audit-output.md >> "$GITHUB_STEP_SUMMARY" - - name: Post sticky PR comment - # Skip on fork PRs — GitHub provides a read-only token there even - # when the workflow declares `pull-requests: write`, and the - # marocchino action would fail and mark the whole workflow red - # even if the audit itself passed. Same-repo branch PRs always - # have write access. `continue-on-error` is a defensive belt — if - # the comment-posting fails for any other reason (rate limit, etc.) - # the audit gate itself is unaffected. - if: | - github.event_name == 'pull_request' && - github.event.pull_request.head.repo.full_name == github.repository && - always() - continue-on-error: true - uses: marocchino/sticky-pull-request-comment@v2 + - name: Upload audit report + if: always() + uses: actions/upload-artifact@v4 with: - header: sso-fork-audit + name: sso-fork-audit-report path: audit-output.md - name: Fail on security-critical violations @@ -77,3 +65,27 @@ jobs: 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: + needs: sso-fork-audit + if: | + always() && + needs.sso-fork-audit.result != 'cancelled' && + github.event_name == 'pull_request' && + github.event.pull_request.head.repo.full_name == github.repository + runs-on: ubuntu-latest + permissions: + contents: read + pull-requests: write + steps: + - name: Download audit report + uses: actions/download-artifact@v4 + with: + name: sso-fork-audit-report + + - name: Post sticky PR comment + continue-on-error: true + uses: marocchino/sticky-pull-request-comment@v2 + with: + header: sso-fork-audit + path: audit-output.md From 8ae18831a39adf0622e10b791535d0e18df6563a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 16 May 2026 12:02:45 +0000 Subject: [PATCH 07/10] chore(ci): harden audit output rendering and setup python Agent-Logs-Url: https://github.com/Pressingly/plane/sessions/57ae3534-e3c7-4983-a658-6757fe345476 --- .github/workflows/sso-audit.yml | 5 +++++ scripts/sso-audit.sh | 10 +++++++++- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/.github/workflows/sso-audit.yml b/.github/workflows/sso-audit.yml index 663942e84ef..c1ea68127de 100644 --- a/.github/workflows/sso-audit.yml +++ b/.github/workflows/sso-audit.yml @@ -39,6 +39,11 @@ jobs: - name: Checkout uses: actions/checkout@v4 + - name: Setup Python + uses: actions/setup-python@v5 + with: + python-version: '3.12' + - name: Run fork audit id: audit run: | diff --git a/scripts/sso-audit.sh b/scripts/sso-audit.sh index 464d1dad759..f1afe4d7b41 100755 --- a/scripts/sso-audit.sh +++ b/scripts/sso-audit.sh @@ -81,6 +81,13 @@ record() { fi } +escape_markdown_cell() { + local cell=$1 + cell=${cell//|/\\|} + cell=${cell//$'\n'/'
'} + printf '%s' "$cell" +} + # ============================================================================ # Row 14 (idx 0): logout shape — narrow check # @@ -234,8 +241,9 @@ echo echo "| Row | Invariant | Status | Notes |" echo "|-----|-----------|--------|-------|" for i in "${!ROW_TITLES[@]}"; do + note="$(escape_markdown_cell "${ROW_NOTES[$i]:-}")" printf "| %d | %s | %s | %s |\n" \ - "${ROW_NUMBERS[$i]}" "${ROW_TITLES[$i]}" "${ROW_STATUS[$i]:-?}" "${ROW_NOTES[$i]:-}" + "${ROW_NUMBERS[$i]}" "${ROW_TITLES[$i]}" "${ROW_STATUS[$i]:-?}" "$note" done echo From f9cfd4aa991066efef8952cadd5c36140230fdc9 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 16 May 2026 12:03:24 +0000 Subject: [PATCH 08/10] chore(ci): use html-entity escaping for markdown table pipes Agent-Logs-Url: https://github.com/Pressingly/plane/sessions/57ae3534-e3c7-4983-a658-6757fe345476 --- scripts/sso-audit.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/sso-audit.sh b/scripts/sso-audit.sh index f1afe4d7b41..17b2bd0b8d3 100755 --- a/scripts/sso-audit.sh +++ b/scripts/sso-audit.sh @@ -83,7 +83,7 @@ record() { escape_markdown_cell() { local cell=$1 - cell=${cell//|/\\|} + cell=${cell//|/|} cell=${cell//$'\n'/'
'} printf '%s' "$cell" } From 7b36291820a898a431c39c29187c0aba4063100b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 16 May 2026 12:03:51 +0000 Subject: [PATCH 09/10] chore(ci): document markdown table cell escaping helper Agent-Logs-Url: https://github.com/Pressingly/plane/sessions/57ae3534-e3c7-4983-a658-6757fe345476 --- scripts/sso-audit.sh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/scripts/sso-audit.sh b/scripts/sso-audit.sh index 17b2bd0b8d3..2c58713b504 100755 --- a/scripts/sso-audit.sh +++ b/scripts/sso-audit.sh @@ -82,6 +82,8 @@ record() { } escape_markdown_cell() { + # Escape table-sensitive characters so notes render as a single markdown cell. + # Input: arbitrary note text. Output: `|` as HTML entity + newlines as
. local cell=$1 cell=${cell//|/|} cell=${cell//$'\n'/'
'} From ea7d9ef715909f67263fef784d0d390184e21e01 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 16 May 2026 12:04:19 +0000 Subject: [PATCH 10/10] chore(ci): quote markdown escape helper input Agent-Logs-Url: https://github.com/Pressingly/plane/sessions/57ae3534-e3c7-4983-a658-6757fe345476 --- scripts/sso-audit.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/sso-audit.sh b/scripts/sso-audit.sh index 2c58713b504..7914f08db66 100755 --- a/scripts/sso-audit.sh +++ b/scripts/sso-audit.sh @@ -84,7 +84,7 @@ record() { escape_markdown_cell() { # Escape table-sensitive characters so notes render as a single markdown cell. # Input: arbitrary note text. Output: `|` as HTML entity + newlines as
. - local cell=$1 + local cell="$1" cell=${cell//|/|} cell=${cell//$'\n'/'
'} printf '%s' "$cell"