Skip to content

chore(ci): add fork-side SSO audit script + workflow#20

Closed
awais786 wants to merge 7 commits into
foss-mainfrom
chore/sso-audit-ci
Closed

chore(ci): add fork-side SSO audit script + workflow#20
awais786 wants to merge 7 commits into
foss-mainfrom
chore/sso-audit-ci

Conversation

@awais786
Copy link
Copy Markdown

Summary

Per-fork SSO contract audit in CI. Mirrors Pressingly/plane#32 and Pressingly/twenty#9.

Checks

Row File Invariant Severity
14 app/stores/AuthStore.ts SPA logout MUST NOT call /oauth2/sign_out informational
20 server/middlewares/authentication.ts MUST define normalizeProxyEmail OR throw AuthenticationError(...proxy...) for stale-session detection in the JWT cookie branch security-critical
21 same No polynomial-backtracking email-shape regex regression guard

Spec source: sso-rules-moneta/openspec/specs/proxy-auth-middleware/spec.md.

Current state

Local dry-run on foss-main: rows 20 + 21 both ❌. The fix for both lives in PR #19 — introduces normalizeProxyEmail AND replaces the polynomial regex with indexOf-based detection. When #19 merges, the audit goes green.

To unblock this PR, rebase onto fix/proxy-auth-stale-session-on-user-switch (or wait for #19).

🤖 Generated with Claude Code

Adds a per-fork deterministic audit for the cross-app SSO contract.
Mirrors Pressingly/plane#32 and Pressingly/twenty#9.

Rows covered:
  14 — SPA logout (app/stores/AuthStore.ts) MUST NOT call /oauth2/sign_out
  20 — server/middlewares/authentication.ts MUST define normalizeProxyEmail
       OR throw AuthenticationError on proxy-identity mismatch in the JWT
       cookie branch. SECURITY-CRITICAL.
  21 — No polynomial-backtracking email-shape regex in authentication.ts

Local dry-run on foss-main:
  row 14 ✅, row 20 ❌, row 21 ❌. Both correctly waiting for Pressingly/
  outline#19 (introduces normalizeProxyEmail + replaces the regex with
  indexOf). When #19 merges, this audit will go green on foss-main.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 16, 2026

Outline SSO Fork Audit

Cross-app contract: https://github.com/awais786/sso-rules-moneta/blob/main/openspec/specs/proxy-auth-middleware/spec.md
Row numbers match the 21-row table at https://github.com/awais786/sso-rules-moneta/blob/main/skills/app-rules/SKILL.md#5-report

Row Invariant Status Notes
14 logout shape: SPA logout does not call /oauth2/sign_out app/stores/AuthStore.ts 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)
20 session-identity reconciliation present (Rule 2 mismatch flush) server/middlewares/authentication.ts does NOT define normalizeProxyEmail and has no throw AuthenticationError(...proxy...) call site. The cross-app spec (proxy-auth-middleware Rule 2) requires the JWT cookie branch of validateAuthentication to compare normalized X-Auth-Request-Email against the JWT user's email and throw AuthenticationError on mismatch, gated on env.AUTH_TYPE === 'SSO' and transport === 'cookie'. The existing outer auth() catch block then clears the accessToken + lastSignedIn cookies via err.headers. Without this check, the stale-session-on-user-switch leak returns. Reference: #19.
21 email-shape detection uses indexOf, not polynomial regex Polynomial-backtracking email-shape regex detected in server/middlewares/authentication.ts: 316: const isValidEmail = /^[^\s@]+@[^\s@]+.[^\s@]+$/.test(emailClaim);. Rewrite to indexOf-based check per openspec proxy-auth-middleware §'email-shape detection SHALL avoid polynomial-backtracking regex'. Reference: normalizeProxyEmail in this file.

2 violations. Security-critical (row 20): 1.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds a fork-specific CI audit to verify that this Outline fork continues to satisfy key invariants from the cross-app SSO “proxy-auth-middleware” contract, and publishes the results to the GitHub job summary (and as a sticky PR comment on same-repo PRs).

Changes:

  • Adds a scripts/sso-audit.sh bash script that checks contract rows 14/20/21 against the repository’s source tree.
  • Adds a GitHub Actions workflow to run the audit on relevant PR changes, on foss-main pushes, and on a weekly schedule.
  • Posts audit output to the job summary and (when safe) as a sticky PR comment.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File Description
scripts/sso-audit.sh Implements the contract checks and outputs a markdown report with pass/fail status per row.
.github/workflows/sso-audit.yml Runs the audit in CI, publishes output, optionally comments on PRs, and fails the job on security-critical violations.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread scripts/sso-audit.sh Outdated
Comment on lines +109 to +114
has_normalize_helper=$(grep -cE "\bnormalizeProxyEmail\b" "$AUTH_MIDDLEWARE" || true)

# Fallback marker: "proxy identity" literal in a throw — used in the
# AuthenticationError message when the mismatch fires.
local has_throw_proxy
has_throw_proxy=$(grep -cE "throw\s+AuthenticationError\(.*[Pp]roxy" "$AUTH_MIDDLEWARE" || true)
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Addressed in commit c7ce838 — switched to grep -cF (fixed-string match) for portable detection. Avoids the \b word-boundary which is a GNU extension.

Comment thread scripts/sso-audit.sh
Comment on lines +138 to +146
local hits
hits=$(grep -nE '\[\^[\\\\]s@\]\+@\[\^[\\\\]s@\]\+\\\.\[\^[\\\\]s@\]\+' "$AUTH_MIDDLEWARE" 2>/dev/null || true)

if [[ -n "$hits" ]]; then
record 2 "❌" "Polynomial-backtracking email-shape regex detected in $AUTH_MIDDLEWARE: $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 $AUTH_MIDDLEWARE; using indexOf-based detection"
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Verified locally against the current branch state — the audit regex does match line 316 of server/middlewares/authentication.ts:

$ grep -nE '\[\^[\\\\]s@\]\+@\[\^[\\\\]s@\]\+\\\.\[\^[\\\\]s@\]\+' server/middlewares/authentication.ts
316:    const isValidEmail = /^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(emailClaim);

And bash scripts/sso-audit.sh correctly fires Row 21 ❌ on this finding (output truncated):

| 21 | email-shape detection uses indexOf, not polynomial regex | ❌ | Polynomial-backtracking email-shape regex detected in server/middlewares/authentication.ts: 316:    const isValidEmail = /^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(emailClaim); ... |

So:

  1. The [\\\\]s escaping in the audit's bash-regex correctly matches the literal \s inside the source's JS regex character class [^\s@].
  2. The audit is doing its job — flagging an actual ReDoS surface that remains on the fwd: token code path (lines 311–319). The PR that introduced normalizeProxyEmail replaced the regex on the proxy-header code path but not on this one.

This finding is real and should not be removed from the audit. Closing this thread; the underlying ReDoS gap on the fwd: path is a separate follow-up issue worth opening.

Comment thread .github/workflows/sso-audit.yml Outdated
Comment on lines +24 to +27
permissions:
contents: read
pull-requests: write

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@copilot apply changes based on this feedback

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Implemented in 6b26a1c. I scoped workflow permissions to least privilege: pull-requests: write was removed from workflow-level permissions and moved to a separate PR-only sso-fork-audit-comment job, while the main audit job now runs with read-only permissions and passes output via artifact.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@awais786
Copy link
Copy Markdown
Author

@copilot apply changes based on the comments in this thread

@awais786
Copy link
Copy Markdown
Author

@copilot apply changes based on the comments in this thread

Copilot AI and others added 3 commits May 16, 2026 11:43
Copy link
Copy Markdown

Copilot AI commented May 16, 2026

@copilot apply changes based on the comments in this thread

Fixed in ff9aeff. Row 20 now uses grep -F for fixed-string matching and [[:space:]] for POSIX-compliant whitespace detection. Row 21 uses a simplified pattern '\[\^\\s@\]\+@' to detect the problematic JavaScript regex literal in source code, with head -1 to prevent table formatting issues from multiple matches.

@copilot apply changes based on the comments in this thread

Fixed in ff9aeff. Row 20 now uses grep -F for fixed-string matching and [[:space:]] for POSIX-compliant whitespace detection. Row 21 uses a simplified pattern '\[\^\\s@\]\+@' to detect the problematic JavaScript regex literal in source code, with head -1 to prevent table formatting issues from multiple matches.

@awais786
Copy link
Copy Markdown
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.

@awais786 awais786 closed this May 18, 2026
@awais786 awais786 deleted the chore/sso-audit-ci branch May 18, 2026 08:26
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.

3 participants