chore(ci): add fork-side SSO audit script + workflow#32
Merged
Conversation
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) <noreply@anthropic.com>
Plane SSO Fork AuditCross-app contract: https://github.com/awais786/sso-rules-moneta/blob/main/openspec/specs/proxy-auth-middleware/spec.md
All fork-side invariants hold. |
There was a problem hiding this comment.
Pull request overview
Adds a deterministic fork-side SSO contract audit to this Plane fork, wiring it into GitHub Actions so regressions in key SSO invariants (especially the stale-session/user-switch class) are caught before they reach the bundle CI.
Changes:
- Added
scripts/sso-audit.shto audit a subset of cross-app SSO contract rows via grep-based checks and emit a markdown table. - Added
.github/workflows/sso-audit.ymlto run the audit on PR/push/schedule, publish to job summary, and post a sticky PR comment. - Updated
.gitignoreto allow trackingscripts/sso-audit.shwhile keeping the rest ofscripts/ignored.
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
scripts/sso-audit.sh |
Implements the fork-side SSO audit checks and emits a markdown report + exit code gating. |
.gitignore |
Un-ignores scripts/sso-audit.sh while keeping other scripts/ contents ignored. |
.github/workflows/sso-audit.yml |
Runs the audit in CI, publishes the report, and gates merges on security-critical failures. |
Comment on lines
+25
to
+29
| # 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). |
Comment on lines
+124
to
+131
| # 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) | ||
|
|
| 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)." |
Comment on lines
+57
to
+62
| - 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 |
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) <noreply@anthropic.com>
Comment on lines
+165
to
+169
| # Look for a call to <logout_name>(...) 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 |
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:]_])<alias>[[: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 4fb4805. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ified 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) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 3 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (1)
scripts/sso-audit.sh:212
- Row 21 uses raw
grep -nhits (including full line contents) in the Notes column. As with row 14, any|characters or newlines in the matched lines will corrupt the Markdown table output. Safer options: emit onlyfile:linelocations, or escape|and replace newlines with<br>before printing.
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
Comment on lines
+99
to
+101
| 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." |
Comment on lines
+39
to
+52
| 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 | ||
|
|
Comment on lines
+32
to
+35
| permissions: | ||
| contents: read | ||
| pull-requests: write | ||
|
|
There was a problem hiding this comment.
Implemented in 8a1b118. I split the workflow into two jobs for least privilege: the audit job now runs with contents: read only, and a separate PR-only same-repo comment job has pull-requests: write and posts the sticky comment from an uploaded/downloaded audit artifact. Security-critical audit gating behavior is preserved.
This was referenced May 16, 2026
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Author
|
Status check on open Copilot review threads (8 of 9 unreplied) after recent commits:
Net unresolved: 2 (markdown source-line break + Python setup). Both have |
Implemented in |
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
Per-fork SSO contract audit in CI. Runs
scripts/sso-audit.shon PRs, blocks merges on security-critical violations. Catches regressions before they reachfoss-server-bundle-devstack, whose CI emits?for fork-side rows.What it checks
/oauth2/sign_outproxy_auth.pyMUST import + calldjango.contrib.auth.logout(request)on identity mismatch^[^\s@]+@[^\s@]+\.[^\s@]+$regex in normalisationSpec source:
sso-rules-moneta/openspec/specs/proxy-auth-middleware/spec.md.Output
sso-fork-audit, updated on each push)Why Row 20 currently fails on this PR
This branch is against
foss-main, which doesn't have PR #29 merged. Soproxy_auth.pylacks thelogout(request)call. The audit is correctly blocking — that's the gate. Two ways to land:foss-main.fix/proxy-auth-stale-session-on-user-switchtemporarily.Both result in a green audit.
Notes
scripts/was gitignored upstream..gitignoreis changed toscripts/*with a!scripts/sso-audit.shnegation so this one file can be tracked.🤖 Generated with Claude Code