Skip to content

[#538] Honor delegated hacker read and edit permissions - #539

Merged
DGoel1602 merged 3 commits into
mainfrom
codex/blade-hacker-permissions
Sep 6, 2026
Merged

[#538] Honor delegated hacker read and edit permissions#539
DGoel1602 merged 3 commits into
mainfrom
codex/blade-hacker-permissions

Conversation

@DGoel1602

@DGoel1602 DGoel1602 commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Why

Organizers granted READ_HACKERS and EDIT_HACKERS were still redirected away from Hackers, and direct API calls returned FORBIDDEN, because both Blade and the hacker router required officer status. The configurable permissions did not match their advertised behavior.

What

Closes: #538

  • Honor READ_HACKERS for roster navigation, direct and legacy links, search/filter/counts, application details, and the detail panel's event attendance. EDIT_HACKERS includes read access and gates profile, status, bulk, points, and deletion operations.
  • Hide write and selection controls from readers. Retain officer-only hackathon configuration and blacklist management; redact blacklist information in non-officer responses and prevent blacklist filtering or deletion from bypassing that restriction.
  • Add API permission-matrix and disposable-database regressions, Blade access tests, and desktop/mobile browser coverage. Update the existing hacker-management feature bundle.

Scope is Blade and the hacker-related API boundaries. Permission bits, schemas, dependencies, email-delivery mechanics, and check-in authorization are unchanged. This PR is based directly on main and does not include navigation PR #537.

Test Plan

  • pnpm verify:precommit passed: React analysis, formatting, lint (existing warnings), and root typecheck across API consumers.
  • API: 194 tests passed across hacker guards/access, hackathon access, event access, and role permissions. The new permission matrix failed 31 cases before the fix.
  • Blade: 34 targeted tests passed.
  • Playwright: all 7 hacker-management tests passed against a disposable local database, including 1440px/320px readers, legacy links, filters/search, details/attendance, editors, denied access, and existing officer flows.
  • pnpm build was attempted and failed on missing local production environment variables: Guild requires JUDGING_ACCESS_SECRET and NEXT_PUBLIC_BLADE_URL; 2026 requires KHIX_HACKER_PORTAL_CLIENT_ID and KHIX_HACKER_PORTAL_ORIGIN. Turbo then canceled Blade/Club builds. A separate pnpm --filter=@forge/blade build also failed collecting /judge/end because JUDGING_ACCESS_SECRET and NEXT_PUBLIC_BLADE_URL are missing. No environment validation was bypassed.

Checklist

  • Database: No schema changes, OR I ran pnpm db:generate and committed the generated files in packages/db/drizzle/
  • Environment Variables: No environment variables changed, OR I have contacted the Development Lead to modify them on Coolify BEFORE merging.

Co-authored-by: Codex <codex@openai.com>
@DGoel1602 DGoel1602 added Bug Something isn't working Major Big change - 2+ reviewers required Blade Change modifies code in Blade app API Change modifies code in the global API/tRPC package labels Sep 6, 2026
@DGoel1602 DGoel1602 self-assigned this Sep 6, 2026
@coderabbitai

coderabbitai Bot commented Sep 6, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Hacker management now supports READ_HACKERS and EDIT_HACKERS access. The API separates read and edit procedures, redacts blacklist data for non-officers, restricts blacklist filters, and protects blacklisted applications. Blade gates navigation, pages, selection, filters, and mutation controls by capability. Tests cover permission combinations, revocation, API safeguards, read-only views, editing, and redirects.

Merge Risk: 🔵 Low · up to 06554

Blacklist protections work in the current implementation, but regression coverage and the feature contract should be corrected so restricted controls and API response shapes remain protected in future changes.

🚥 Pre-merge checks | ✅ 7 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 30.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 20 functions across 15 files. (4 skipped:… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (7 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The changes satisfy issue #538 by enabling READ_HACKERS access, granting EDIT_HACKERS write access, hiding read-only controls, and preserving officer-only blacklist and configuration safeguards.
Out of Scope Changes check ✅ Passed The changes remain within the linked issue scope: Blade hacker-management access and hacker-related API boundaries. No unrelated schema, dependency, permission-bit, email, or check-in changes are incl…
No Hardcoded Secrets ✅ Passed No hardcoded secret values were introduced. The changed files contain only environment-variable names such as JUDGING_ACCESS_SECRET and test fixture data; scans found no API-key, password, token, be…
Validated Env Access ✅ Passed No new direct process.env usage was introduced. The only matching reference in a changed file was already present on main and was not added, removed, or modified. Added source-like lines contain n…
No Typescript Escape Hatches ✅ Passed No TypeScript escape hatch was introduced. The pull-request TypeScript diff contains no any type, @ts-ignore, @ts-expect-error, or non-null assertion. The only added any match is the word in t…
Title check ✅ Passed The title starts with the required issue number format, describes the delegated hacker permission change, and is 55 characters long.
Description check ✅ Passed The description directly explains the delegated hacker permissions, scope, safeguards, tests, and build limitation.
Full details: Docstring Coverage

Explanation

Docstring coverage is 30.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 20 functions across 15 files. (4 skipped: 4 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/blade-hacker-permissions

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

Co-authored-by: Codex <codex@openai.com>

@DVidal1205 DVidal1205 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

lgtm but plz remove images from within the repo before merging

Co-authored-by: Codex <codex@openai.com>

@coderabbitai coderabbitai 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.

Note

Quiet mode is enabled, so only the most important comments were posted inline. Other review comments are grouped below.

🟡 Other comments (2)
apps/blade/src/tests/e2e/hacker-management.spec.ts-279-281 (1)

279-281: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win

Assert both blacklist control names are absent

/Blacklist/ does not match Remove blacklist. Match both control names:

Proposed fix
-    await expect(dialog.getByRole("button", { name: /Blacklist/ })).toHaveCount(
-      0,
-    );
+    await expect(
+      dialog.getByRole("button", {
+        name: /^(Blacklist applicant|Remove blacklist)$/,
+      }),
+    ).toHaveCount(0);
.forge/features/hacker-management/srd.md-43-45 (1)

43-45: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Separate the API redaction from the UI label.

previewBulk and confirmBulk return reason: null for non-officer blacklist skips. bulk-confirm-dialog.tsx displays that value as “Requires officer review.” Update this sentence to describe both contracts; the SRD feeds implementation and test generation, so the current wording can cause an API-shape mismatch.


ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: QUIET

Plan: Team

Run ID: e9c1bc60-17e1-4191-be5e-a4b7efdb86b6

📥 Commits

Reviewing files that changed from the base of the PR and between bd97fcc and 065546a.

📒 Files selected for processing (19)
  • .forge/features/hacker-management/spec.md
  • .forge/features/hacker-management/srd.md
  • .forge/features/hacker-management/status.md
  • .forge/features/hacker-management/test-cases.md
  • apps/blade/src/app/_components/admin/hackathon/hackers/bulk-confirm-dialog.tsx
  • apps/blade/src/app/_components/admin/hackathon/hackers/hacker-detail-dialog.tsx
  • apps/blade/src/app/_components/admin/hackathon/hackers/hacker-filters.tsx
  • apps/blade/src/app/_components/admin/hackathon/hackers/hacker-roster.tsx
  • apps/blade/src/app/_components/admin/hackathon/hackers/hacker-table.tsx
  • apps/blade/src/app/admin/hackers/page.tsx
  • apps/blade/src/app/admin/layout.tsx
  • apps/blade/src/lib/admin-access.ts
  • apps/blade/src/tests/admin/admin-access.test.ts
  • apps/blade/src/tests/e2e/hacker-management.spec.ts
  • packages/api/src/routers/hackathon-event.ts
  • packages/api/src/routers/hacker.ts
  • packages/api/src/tests/hacker/access.test.ts
  • packages/api/src/tests/integration/hacker-guards.test.ts
  • packages/api/src/utils/hacker/access.ts

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.

@DGoel1602
DGoel1602 added this pull request to the merge queue Sep 6, 2026
Merged via the queue into main with commit 20878e0 Sep 6, 2026
13 checks passed
@DGoel1602
DGoel1602 deleted the codex/blade-hacker-permissions branch September 6, 2026 14:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

API Change modifies code in the global API/tRPC package Blade Change modifies code in Blade app Bug Something isn't working Major Big change - 2+ reviewers required

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Honor delegated hacker read and edit permissions in Blade

2 participants