Skip to content

Implement milestone 14 P4 lifecycle and test hygiene#99

Merged
bjcorder merged 43 commits into
mainfrom
p3-oauth-flow-client-storage
May 24, 2026
Merged

Implement milestone 14 P4 lifecycle and test hygiene#99
bjcorder merged 43 commits into
mainfrom
p3-oauth-flow-client-storage

Conversation

@bjcorder

@bjcorder bjcorder commented May 24, 2026

Copy link
Copy Markdown
Owner

Summary

Implements GitHub milestone 14 / v0.2 P4 lifecycle and test-hygiene issues #87-#95.

  • Adds lifecycle review findings for JWT logout denylist gaps, refresh-family logout revocation gaps, sliding expiry without rotation, and password-change without global revocation.
  • Adds clean-baseline false-positive fixtures across framework families for the P1-P4 check inventory.
  • Adds hand-rolled JSON report snapshot tests and Rust integration coverage for the CLI exit-code matrix.
  • Documents the consolidated category-audit decision: no new finding category, schema bump, or SARIF rule change required.
  • Completes final v0.2 docs updates across CHANGELOG, README, roadmap, and coverage matrix.
  • Addresses final review findings by tightening P4.4 global-revocation suppression and adding explain redaction regression coverage.

Issues / commits

Tracker: #62

Validation

  • cargo fmt --check
  • cargo check --workspace
  • cargo test -q
  • cargo clippy --workspace --all-targets -- -D warnings
  • cargo test --workspace --all-targets

Review results

Fresh-context completion, correctness, security, and /sec-review passes were run.

  • Completion review: no confirmed milestone blockers.
  • Correctness review: no blockers; optional OAuth/provider/fixture enforcement improvements deferred.
  • Security review: one P4.4 blocker fixed in 9a2fabd.
  • /sec-review: explain redaction path already sanitized reports before rendering; regression coverage added in 9a2fabd.

Deferred follow-ups

  • Broaden OAuth CommonJS/import-only suppression.
  • Reconcile/document NextAuth/Auth.js provider coverage limits.
  • Consider enforcing every fixture expected.json finding generically.
  • Consider P3-specific JSON snapshots.
  • Consider bounded file-size reads for sessionscope explain.

View with Codesmith Autofix with Codesmith
Need help on this PR? Tag @codesmith with what you need. Autofix is disabled.

bjcorder and others added 30 commits May 21, 2026 22:21
… sensitive information'

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
@bjcorder

Copy link
Copy Markdown
Owner Author

Review/remediation update:

Summary:

  • Resolved PR Implement milestone 14 P4 lifecycle and test hygiene #99 merge conflicts with latest origin/main while preserving P3 base changes and P4 lifecycle/test-hygiene changes.
  • Fixed review blockers for JWT logout denylist suppression, OAuth state/nonce issuance-vs-validation detection, wildcard loopback redirect URIs, and OAuth/client secret redaction.
  • Added focused regression coverage for the remediated cases, plus a small client-storage template-literal cookie key coverage improvement flagged by review.

Security/regression review:

  • Fresh-context follow-up reviewers found no remaining blockers after the second follow-up pass.
  • Central redaction now covers quoted camelCase clientSecret values, and OAuth detector-local redaction covers short quoted clientSecret values.

Merge conflicts:

  • Resolved in 50631e6.

Validation:

  • cargo fmt --check — passed
  • cargo check --workspace — passed
  • cargo test -q — passed
  • cargo clippy --workspace --all-targets -- -D warnings — passed
  • cargo test --workspace --all-targets — passed

Commits pushed:

Remaining notes:

  • None

@bjcorder

Copy link
Copy Markdown
Owner Author

CI follow-up update:

Summary:

  • Fixed the failing Windows cargo test --workspace --all-targets check.
  • Root cause was JSON snapshot comparison normalizing Windows path separators but not CRLF line endings from checkout, causing snapshot text mismatch on Windows only.
  • Updated snapshot normalization to normalize CRLF to LF and added regression coverage.

Validation:

  • cargo test -q -p sessionscope-testing snapshots — passed
  • cargo test -q -p sessionscope-testing --test json_snapshots — passed
  • cargo fmt --check — passed
  • cargo check --workspace — passed
  • cargo test -q — passed
  • cargo clippy --workspace --all-targets -- -D warnings — passed
  • cargo test --workspace --all-targets — passed

Commits pushed:

Remaining notes:

  • Waiting for GitHub CI to rerun on the pushed commit.

@bjcorder

Copy link
Copy Markdown
Owner Author

CI follow-up update 2:

The Windows snapshot failure persisted because the previous fix only normalized snapshot text. The actual scan output also needed deterministic source text before detector offsets/excerpts are produced.

Summary:

  • Normalized source file line endings to LF during source loading in sessionscope-core.
  • Kept snapshot normalization tolerant of actual and escaped CRLF sequences.
  • Added regression coverage for CRLF source reads and snapshot normalization.
  • Reproduced the Windows-only condition locally by converting the failing Express fixture to CRLF; json_snapshots now passes under that simulation.

Validation:

  • cargo test -q -p sessionscope-core source::tests::normalizes_windows_line_endings_for_deterministic_scans — passed
  • cargo test -q -p sessionscope-testing snapshots — passed
  • CRLF fixture simulation + cargo test -q -p sessionscope-testing --test json_snapshots — passed
  • cargo fmt --check — passed
  • cargo check --workspace — passed
  • cargo test -q — passed
  • cargo clippy --workspace --all-targets -- -D warnings — passed
  • cargo test --workspace --all-targets — passed

Commit pushed:

  • 39680e2 — Fix Windows snapshot CI line endings

Waiting for GitHub CI to rerun on the new commit.

@bjcorder
bjcorder merged commit 731914d into main May 24, 2026
10 checks passed
@bjcorder
bjcorder deleted the p3-oauth-flow-client-storage branch May 24, 2026 15:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment