Skip to content

chore(security): enable security-guidance plugin with repo rules#69

Merged
joefairburn merged 3 commits into
mainfrom
tirana
Jul 23, 2026
Merged

chore(security): enable security-guidance plugin with repo rules#69
joefairburn merged 3 commits into
mainfrom
tirana

Conversation

@joefairburn

@joefairburn joefairburn commented Jul 23, 2026

Copy link
Copy Markdown
Owner

What & why

Enables the security-guidance@claude-plugins-official plugin in checked-in .claude/settings.json so it loads in cloud sessions and for everyone who clones the repo, and adds .claude/claude-security-guidance.md describing this repo's threat model for the plugin's model-backed reviews. The guidance is grounded in the actual code — tenant isolation via branded scope ids in src/lib/scope.ts, the Bearer/session/OAuth auth surfaces in middleware/02.api-auth.ts, signed-token artifact downloads, and secret/logging/injection conventions — so end-of-turn and commit reviews flag cross-tenant leaks and auth bypasses in Wrightful's terms rather than generically.

How verified

Config/docs only, no runtime code; the pre-commit hook (vp fmt) ran and reformatted the files on commit.

  • pnpm check passes (format + lint + type-check)
  • Relevant tests pass (pnpm test / pnpm --filter … test / pnpm test:e2e)
  • Added/updated a worklog in docs/worklog/ for non-trivial changes
  • Schema change? Regenerated the migration (pnpm --filter @wrightful/dashboard db:generate)
  • User-facing reporter change? Added a changeset (pnpm changeset)

Notes for reviewers

The guidance file is advisory context for the security-guidance plugin's reviewer model — additive only, it surfaces findings for Claude to fix and does not block writes. Lookup also supports ~/.claude/... (per-machine) and .claude/claude-security-guidance.local.md (gitignored personal overrides) if reviewers want to extend it.

Summary by CodeRabbit

  • Documentation
    • Added a new security guidance document for the Wrightful dashboard, covering required multi-tenant isolation scoping, authentication/authorization expectations, rate-limiting invariants, secure artifact download authorization, and best practices for secrets handling, logging, parameterized SQL, and safe rendering/redirect behavior.
  • Chores
    • Enabled an official security guidance plugin to improve supported development workflows.

Enable the security-guidance@claude-plugins-official plugin in checked-in
settings and add .claude/claude-security-guidance.md describing this repo's
tenancy, auth, artifact, and secret-handling threat model for the plugin's
model-backed reviews.
@coderabbitai

coderabbitai Bot commented Jul 23, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 69d41af2-5fbb-415b-8462-6392cc248ffe

📥 Commits

Reviewing files that changed from the base of the PR and between 3515aa9 and 1bd636d.

📒 Files selected for processing (1)
  • .claude/claude-security-guidance.md
🚧 Files skipped from review as they are similar to previous changes (1)
  • .claude/claude-security-guidance.md

📝 Walkthrough

Walkthrough

Added dashboard security guidance for tenant isolation, authentication, artifact downloads, secrets, logging, injection safety, and reviewer findings, then enabled the corresponding Claude plugin.

Changes

Security guidance

Layer / File(s) Summary
Tenant isolation and API security guidance
.claude/claude-security-guidance.md
Defines reviewer rules for tenant scoping, authentication and API surfaces, artifact download authorization, secrets, logging, SQL injection, and server-rendered HTML safety.
Security guidance plugin configuration
.claude/settings.json
Enables the security-guidance@claude-plugins-official Claude plugin.

Estimated code review effort: 1 (Trivial) | ~5 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects enabling the security-guidance plugin and adding repository-specific security rules.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch tirana

Comment @coderabbitai help to get the list of available commands.

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jul 23, 2026

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Updated (UTC)
✅ Deployment successful!
View logs
wrightful 1bd636d Jul 23 2026, 11:23 AM

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: cb17fdf77c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread .claude/claude-security-guidance.md Outdated
Comment on lines +10 to +12
- Every run-scoped query MUST filter by a branded `projectId` (and by `teamId`
on the `runs` table). Flag any query over `runs`, `testResults`,
`testResultAttempts`, `testTags`, `testAnnotations`, `artifacts`, `monitors`,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Exempt unique-ID lookups from two-column run scoping

This tells the reviewer plugin that every runs query lacking both tenant columns is a leak, but the same guidance explicitly blesses runByIdWhere. That helper deliberately scopes a globally unique run ID by projectId alone (apps/dashboard/src/lib/scope.ts:221), and is used for safe run-detail and ingest reads; consequently, reviews following this file will repeatedly flag the established predicate as a security defect. State the single-run-ID exception (or require runScopeWhere only for list/aggregate queries) so the guidance remains internally consistent.

Useful? React with 👍 / 👎.

@wrightful

wrightful Bot commented Jul 23, 2026

Copy link
Copy Markdown

✅ Wrightful — 51 passed

Passed Failed Flaky Skipped Duration
51 0 0 1 1m 9s

View run report → · Compare to base →

Commit: 1bd636d · Base: 3515aa9

Clarify that projectId-alone scoping is correct for globally-unique ULID
lookups (runByIdWhere, childByIdWhere); only list/aggregate runs queries AND
teamId. Prevents the reviewer plugin from flagging the blessed predicates.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3515aa92fd

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +10 to +13
- Every run-scoped query MUST filter by a branded `projectId`. Flag any query
over `runs`, `testResults`, `testResultAttempts`, `testTags`,
`testAnnotations`, `artifacts`, `monitors`, `monitorExecutions`, or
`quarantinedTests` that lacks a project filter.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Exempt trusted fleet-wide jobs from the tenant-query rule

This applies to any query over these tables, but scheduled maintenance deliberately enumerates rows across projects before reapplying per-row scope: for example, sweepStaleRuns selects stale rows from runs without a project predicate, and the monitor scheduler similarly selects due monitors globally. As written, the plugin will flag those required cron queries as cross-tenant leaks whenever they are changed, creating false-positive security reviews; carve out trusted maintenance/cron scans that do not expose tenant data and perform scoped mutations.

Useful? React with 👍 / 👎.

Comment thread .claude/claude-security-guidance.md Outdated
Comment on lines +43 to +44
artifact download is a signed token. Flag new `/api/*` routes that read tenant
data without an explicit auth check.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Recognize middleware-protected API routes as authenticated

A handler need not contain its own auth check when the route is protected by the global middleware: /api/v1/runs, for example, relies on 02.api-auth.ts to validate Bearer credentials and then calls getApiKey(c). This instruction will cause correctly wired new ingest/query handlers to be reported merely because the check is centralized; require reviewers to verify the route matcher and middleware ordering rather than demanding a handler-local check.

AGENTS.md reference: AGENTS.md:L106-L108

Useful? React with 👍 / 👎.

Address two P2 false-positive risks: (1) fleet-wide maintenance scans
(staleRunFilter sweep, dueMonitorsWhere scheduler) legitimately SELECT across
projects then scope per-row, and (2) Bearer ingest/query handlers rely on
02.api-auth.ts centrally, so they carry no handler-local auth check. Tell the
reviewer to verify the cron/queue context and the middleware route matcher
rather than flag these as leaks.
@joefairburn
joefairburn merged commit 47e6351 into main Jul 23, 2026
11 checks passed
@joefairburn
joefairburn deleted the tirana branch July 23, 2026 11:34
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.

1 participant