chore(ci): add fork-side SSO audit script + workflow#23
Closed
awais786 wants to merge 2 commits into
Closed
Conversation
Adds a per-fork deterministic audit. Final entry in the cross-app family — mirrors Pressingly/plane#32, Pressingly/twenty#9, Pressingly/outline#20, Pressingly/penpot#19. SurfSense is architecturally immune to the cross-user stale-session leak class (current_active_user resolves proxy_user before jwt_user). This audit pins that immunity by checking the regression-guard test file exists. Rows covered: 14 — SPA logout (surfsense_web/lib/auth-utils.ts) MUST NOT call /oauth2/sign_out 20 — surfsense_backend/tests/unit/test_current_active_user_proxy_ precedence.py MUST exist AND surfsense_backend/app/users.py MUST preserve the proxy_user > jwt_user precedence. SECURITY-CRITICAL. 21 — No polynomial-backtracking email-shape regex (SurfSense uses substring check today) Local dry-run on foss-main: row 14 ✅, row 20 ❌ (precedence test missing — #22 introduces it), row 21 ✅. When #22 merges, the audit goes green. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
SurfSense SSO Fork AuditCross-app contract: https://github.com/awais786/sso-rules-moneta/blob/main/openspec/specs/proxy-auth-middleware/spec.md
1 violations. Security-critical (row 20): 1. |
There was a problem hiding this comment.
Pull request overview
Adds a CI-only audit (no production code change) that verifies SurfSense's fork-side rows of the cross-app SSO contract from awais786/sso-rules-moneta. A bash script scripts/sso-audit.sh grep-checks three invariants — SPA logout shape, proxy_user > jwt_user precedence in current_active_user (architectural immunity), and absence of a polynomial-backtracking email-shape regex — and a new workflow runs it on PRs touching the relevant paths, on foss-main pushes, weekly, and on demand, publishing results to job summary and a sticky PR comment.
Changes:
- New
scripts/sso-audit.shimplementing checks for rows 14, 20, and 21. - New
.github/workflows/sso-audit.ymlthat executes the script, fails on security-critical violations (row 20), and posts a sticky PR comment for same-repo PRs. - Audit treats the row-20 precedence test file from PR #22 as the regression-guard pin.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 8 comments.
| File | Description |
|---|---|
| scripts/sso-audit.sh | Bash audit script that grep-checks the three fork-side invariants and prints a markdown summary table. |
| .github/workflows/sso-audit.yml | CI workflow that runs the audit on relevant triggers, surfaces results, and gates merges on security-critical failures. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| if grep -qE '/oauth2/sign_?out' "$AUTH_UTILS"; then | ||
| local line | ||
| line=$(grep -nE '/oauth2/sign_?out' "$AUTH_UTILS" | head -1) |
Comment on lines
+112
to
+128
| # The precedence pattern: in current_active_user, proxy_user is checked | ||
| # and returned BEFORE jwt_user. We grep for the canonical shape that | ||
| # makes the precedence visible. | ||
| local has_precedence | ||
| has_precedence=$(grep -cE "proxy_user.*is not None|return proxy_user" "$USERS_PY" || true) | ||
|
|
||
| local has_test | ||
| has_test=0 | ||
| [[ -f "$PRECEDENCE_TEST" ]] && has_test=1 | ||
|
|
||
| if [[ "$has_precedence" -gt 0 && "$has_test" -eq 1 ]]; then | ||
| record 1 "✅" "$USERS_PY preserves proxy_user > jwt_user precedence (architectural immunity); regression guard test at $PRECEDENCE_TEST pins the contract" | ||
| return | ||
| fi | ||
|
|
||
| if [[ "$has_precedence" -eq 0 ]]; then | ||
| record 1 "❌" "$USERS_PY does NOT show the proxy_user precedence pattern. SurfSense's immunity to the cross-user stale-session leak depends on \`current_active_user\` returning \`proxy_user\` before falling back to \`jwt_user\`. If this property is gone, the leak class is re-introduced. Reference: openspec proxy-auth-middleware §'Identity mismatch SHALL flush', surfsense-security.md §'architecturally immune', Pressingly/SurfSense#22." |
Comment on lines
+144
to
+152
| local hits | ||
| hits=$(grep -nE '\^\[\^[\\\\]s@\]\+@\[\^[\\\\]s@\]\+\\\.\[\^[\\\\]s@\]\+\$' "$MIDDLEWARE" 2>/dev/null || true) | ||
|
|
||
| if [[ -n "$hits" ]]; then | ||
| record 2 "❌" "Polynomial-backtracking email-shape regex detected in $MIDDLEWARE: $hits. SurfSense uses a substring check (\`'@' not in email\`); introducing the explicit ^...$ regex form re-enables polynomial backtracking on adversarial input. Rewrite to substring or indexOf-based check per openspec proxy-auth-middleware §'email-shape detection SHALL avoid polynomial-backtracking regex'." | ||
| return | ||
| fi | ||
|
|
||
| record 2 "✅" "No polynomial-backtracking email-shape regex in $MIDDLEWARE; using substring check" |
Comment on lines
+56
to
+65
| 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 |
Comment on lines
+13
to
+19
| paths: | ||
| - 'surfsense_backend/app/middleware/proxy_auth.py' | ||
| - 'surfsense_backend/app/users.py' | ||
| - 'surfsense_backend/tests/unit/test_current_active_user_proxy_precedence.py' | ||
| - 'surfsense_web/lib/auth-utils.ts' | ||
| - 'scripts/sso-audit.sh' | ||
| - '.github/workflows/sso-audit.yml' |
Comment on lines
+30
to
+36
| jobs: | ||
| sso-fork-audit: | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - name: Checkout | ||
| uses: actions/checkout@v4 | ||
|
|
Comment on lines
+26
to
+28
| permissions: | ||
| contents: read | ||
| pull-requests: write |
Comment on lines
+41
to
+45
| if bash scripts/sso-audit.sh | tee audit-output.md; then | ||
| echo "audit_exit=0" >> "$GITHUB_OUTPUT" | ||
| else | ||
| echo "audit_exit=$?" >> "$GITHUB_OUTPUT" | ||
| fi |
Scope `pull-requests: write` to the job, not the whole workflow — the audit step itself only needs `contents: read`. Bump actions/checkout to @v5 to match the convention used in Pressingly/outline. Same hardening Pressingly/plane and Pressingly/outline received during Copilot review; applied preemptively here for consistency. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Author
|
Closing: fork-side audit tooling not needed on this fork. The audit is being maintained in sso-rules-moneta instead, which is the single source of truth across all forks. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Final entry in the cross-app fork-audit family. Mirrors Pressingly/plane#32, Pressingly/twenty#9, Pressingly/outline#20, Pressingly/penpot#19.
SurfSense is architecturally immune to the cross-user stale-session leak class —
current_active_userresolvesproxy_user(per-request) before falling back tojwt_user. This audit pins that immunity via the regression-guard test introduced in PR #22.Checks
surfsense_web/lib/auth-utils.ts/oauth2/sign_outtests/unit/test_current_active_user_proxy_precedence.py+app/users.pycurrent_active_userMUST preserveproxy_user > jwt_userorderapp/middleware/proxy_auth.pySpec source:
sso-rules-moneta/openspec/specs/proxy-auth-middleware/spec.md.Current state
Local dry-run on
foss-main: row 20 ❌ — precedence test doesn't exist yet. PR #22 introduces it. When #22 merges, the audit goes green.To unblock this PR, rebase onto
fix/proxy-auth-stale-session-on-user-switch.🤖 Generated with Claude Code