feat(pentest): credits wallet, admin grants, audit logging, UX polish#2691
Merged
feat(pentest): credits wallet, admin grants, audit logging, UX polish#2691
Conversation
…ooks
Frontend
- Split-view shell at /security/penetration-tests with run list + detail pane
- New /penetration-tests/new route with inline create form (replaces modal)
- 8 detail variants: empty, overview, running, completed, clean, failed, finding, create
- Live findings table + agent activity log (polling-based)
- Severity tokens (oklch) scoped to .pt-tokens, light/dark mode aware
Backend
- Swapped homegrown MacedClient → @maced/api-client v0.9.1
- New endpoints: GET /:id/issues, GET /:id/events
- Webhook signature verification via SDK's MacedClient.webhooks.constructEvent
- Raw-body preservation in main.ts for HMAC verification
- Attribution metadata (compOrganizationId/compEnvironment/compApiVersion)
sent on every create — disaster-recovery + audit parity
- Removed silent GitHub OAuth token forwarding to Maced (privacy)
Removed
- pentest-billing.{controller,service}.ts + Stripe webhook route (deferred to v2)
- Homegrown maced-client.ts + Zod schemas (replaced by SDK types)
- Phantom webhook handshake (Maced uses HMAC, not per-run tokens)
- listGithubRepos endpoint + getGithubTokenForOrg + CredentialVaultService
dep — no more silent OAuth-token forwarding to vendor
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Backbone: - pentest_credits wallet table (balance + lifetime totals + last_grant_source), backfill of 1 trial credit per existing org, drop empty pentest_subscriptions / organization_billing legacy tables - credit_refunded_at column on security_penetration_test_runs for webhook refund idempotency - 'pentest' added to AuditLogEntityType enum Service: - PentestCreditsService with idempotent grantInitialTrial, grant, atomic debitOrThrow, refund (no-op on missing wallet row), audit-log entries on grant/refund attributed to org owner - createReport: debit-first ordering blocks concurrent fast-clicks before hitting Maced; refund-on-Maced-failure; refund-on-ownership-persist-failure; 402 attempts also written to audit log - handleWebhook: refund on pentest.failed / pentest.cancelled with claim- before-refund pattern (updateMany WHERE creditRefundedAt IS NULL); audit entry on pentest.completed with finding count, duration, agent count - persistRunOwnership upsert hardened to never overwrite organizationId on conflict (defensive — Maced generates unique IDs in practice) - @maced/api-client@0.9.1 retry wrapper disabled (maxAttempts: 1) to avoid the "Cannot construct a Request..." cryptic error on retriable 5xx; jest transformIgnorePatterns added so specs can load the ESM SDK Admin panel: - New "Pentest credits" tab on org detail page - POST /v1/admin/organizations/:orgId/pentest-credits/grant (PlatformAdminGuard, source=manual, capped at 1000/grant, audited via AdminAuditLogInterceptor + per-org audit log entry) Frontend UX: - Quota footer in scan sidebar (X scans remaining / trial used messaging) - "+ New scan" disabled at zero balance with explicit tooltip + create-form banner; no extra clutter in the header - Overview pane: onboarding state for 0 scans; posture stats (completed, coverage, avg duration, cadence), Latest Assessment card with download + view-detail, Recent Scans, Stale Coverage. Real data only — no fabricated severity rollup or trend charts. - Clean-state detail pane redesigned: neutral hero, no decorative checks, symmetric Markdown/PDF buttons, framework-agnostic CTA copy, suppressed misleading "<1m" duration when Maced doesn't bump updatedAt, suppressed "Last update" when equal to "Started", removed redundant right-column - Findings detail: severity left-bar accent on neutral card (no full-card tint that overwhelmed at any severity) - Sidebar: filter button + agent count removed (misleading at low data volumes); status pill stays "Completed" everywhere (clean variant dropped to keep sidebar/detail badges consistent) - Execution trace opens by default; events containing "maced" filtered out (white-label cleanup) plus TodoWrite noise - Tamed running pulse animation; elapsed time computed client-side from createdAt instead of trusting Maced progress.elapsedMs - Softer medium severity tokens; full-bleed split-view (negative margins around app-shell padding) - is-security-enabled feature flag check removed (RBAC pentest:read alone gates access; flag was breaking dev after PostHog/session restarts) Tenant isolation: - All per-run endpoints flow through assertRunOwnership returning 404 (not 403) so org B cannot enumerate org A's run ids - listReports filters via listOwnedRunIds before mapping - Audit log entries scoped to organizationId of the affected org Tests: - 17 unit tests for PentestCreditsService covering status, grant, debit (including concurrent-debit race), refund, audit-log writes, no-owner edge case, audit failure tolerance Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
tofikwest
changed the title
Resolves modify/delete conflict on packages/db/prisma/schema/organization-billing.prisma by keeping the delete from this branch — the legacy `OrganizationBilling` model was empty in production and its only code consumer (apps/app/src/app/api/webhooks/stripe-pentest/route.ts) was already removed in db0e7e7. Schema state remains consistent: no orphan relations from Organization or elsewhere. All other overlapping files auto-merged cleanly: - apps/api/package.json (jest transformIgnorePatterns + main's new deps) - apps/api/src/admin-organizations/admin-organizations.module.ts (AdminPentestCreditsController + main's PurgeOrganization* services) - apps/app/src/app/(app)/[orgId]/admin/organizations/[adminOrgId]/components/AdminOrgTabs.tsx (Pentest credits tab + main's "Delete Permanently" action) - bun.lock - packages/db/prisma/schema/organization.prisma - packages/db/prisma/schema/security-penetration-test-run.prisma Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
21 issues found across 68 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/app/src/app/(app)/[orgId]/security/penetration-tests/_components/AgentActivityLog.tsx">
<violation number="1" location="apps/app/src/app/(app)/[orgId]/security/penetration-tests/_components/AgentActivityLog.tsx:50">
P1: Sensitive supplier/internal event data is redacted only at render time, so it is still exposed in the client API payload.</violation>
</file>
<file name="apps/app/src/app/(app)/[orgId]/security/penetration-tests/_components/FindingsTable.tsx">
<violation number="1" location="apps/app/src/app/(app)/[orgId]/security/penetration-tests/_components/FindingsTable.tsx:66">
P2: Interactive table rows are mouse-only. Add keyboard activation and focus semantics when `onRowClick` is enabled.</violation>
</file>
<file name="apps/app/src/app/(app)/[orgId]/security/penetration-tests/_components/SplitView.tsx">
<violation number="1" location="apps/app/src/app/(app)/[orgId]/security/penetration-tests/_components/SplitView.tsx:67">
P2: Coalescing `credits?.balance` to `0` removes the loading/unknown state, making `balance === undefined` checks dead and prematurely disabling creation UI.</violation>
<violation number="2" location="apps/app/src/app/(app)/[orgId]/security/penetration-tests/_components/SplitView.tsx:214">
P2: The `Accept` header is derived from whether `filename` exists, so markdown downloads incorrectly request `application/pdf`.</violation>
</file>
<file name="apps/app/src/app/(app)/[orgId]/security/penetration-tests/_components/overview-internals.tsx">
<violation number="1" location="apps/app/src/app/(app)/[orgId]/security/penetration-tests/_components/overview-internals.tsx:249">
P3: `relativeTime` labels ~1–2 minute old timestamps as `just now` due to `minutes <= 1` after flooring. Use `< 1` so 1 minute displays as `1 min ago`.</violation>
</file>
<file name="apps/app/src/app/(app)/[orgId]/security/penetration-tests/_components/pentest-tokens.css">
<violation number="1" location="apps/app/src/app/(app)/[orgId]/security/penetration-tests/_components/pentest-tokens.css:71">
P2: Respect reduced-motion preferences for the new infinite pulse animation.</violation>
</file>
<file name="apps/app/src/app/(app)/[orgId]/security/penetration-tests/_components/DetailPane.tsx">
<violation number="1" location="apps/app/src/app/(app)/[orgId]/security/penetration-tests/_components/DetailPane.tsx:56">
P2: Don’t treat `error` alone as a hard-failure state here; SWR can return stale `run` data alongside an `error`, and this condition hides valid run details.</violation>
</file>
<file name="packages/db/prisma/schema/pentest-credits.prisma">
<violation number="1" location="packages/db/prisma/schema/pentest-credits.prisma:18">
P1: `balance` is documented as never negative, but there is no DB-level constraint enforcing that invariant.</violation>
<violation number="2" location="packages/db/prisma/schema/pentest-credits.prisma:35">
P3: `@@index([organizationId])` is redundant because `organizationId` is already indexed by `@unique`.</violation>
</file>
<file name="apps/api/src/main.ts">
<violation number="1" location="apps/api/src/main.ts:92">
P1: `rawBody` is being attached for all JSON requests, which duplicates payload memory up to the 150MB parser limit. Restrict this to webhook routes that actually require signature verification.</violation>
</file>
<file name="apps/app/src/app/(app)/[orgId]/security/penetration-tests/hooks/use-penetration-tests.ts">
<violation number="1" location="apps/app/src/app/(app)/[orgId]/security/penetration-tests/hooks/use-penetration-tests.ts:233">
P2: `usePenetrationTestIssues` does not guarantee the promised final refresh after a run completes, so the last issues can be missed.</violation>
</file>
<file name="apps/app/src/app/(app)/[orgId]/security/penetration-tests/_components/FindingDetail.tsx">
<violation number="1" location="apps/app/src/app/(app)/[orgId]/security/penetration-tests/_components/FindingDetail.tsx:92">
P2: Use consistent emptiness checks for PoC content; empty-string values currently render a blank state instead of the fallback message.</violation>
</file>
<file name="apps/api/src/security-penetration-tests/security-penetration-tests.service.spec.ts">
<violation number="1" location="apps/api/src/security-penetration-tests/security-penetration-tests.service.spec.ts:612">
P2: Do not leave webhook-signature and MACED_API_KEY behavior as TODOs; replace this block with real tests so these security-critical paths stay regression-protected.</violation>
</file>
<file name="apps/api/src/security-penetration-tests/security-penetration-tests.service.ts">
<violation number="1" location="apps/api/src/security-penetration-tests/security-penetration-tests.service.ts:476">
P2: `pentest.completed` webhook retries will create duplicate completion audit entries because there is no idempotency/deduplication check before writing the audit row.</violation>
<violation number="2" location="apps/api/src/security-penetration-tests/security-penetration-tests.service.ts:537">
P1: `creditRefundedAt` is set before refund succeeds, so a transient refund failure can permanently suppress retries and leave the customer unrefunded.</violation>
</file>
<file name="tasks/todo.md">
<violation number="1" location="tasks/todo.md:104">
P1: The proposed create flow is not credit-atomic: checking balance before the external create call can allow concurrent requests to create more runs than available credits.</violation>
</file>
<file name="apps/api/src/security-penetration-tests/pentest-credits.service.ts">
<violation number="1" location="apps/api/src/security-penetration-tests/pentest-credits.service.ts:179">
P2: Use the same Prisma client for the post-debit status read. Calling `getStatus()` here bypasses the optional transaction client and can return a stale balance when `debitOrThrow` runs inside a transaction.</violation>
</file>
<file name="apps/app/src/app/(app)/[orgId]/security/penetration-tests/_components/RunningDetail.tsx">
<violation number="1" location="apps/app/src/app/(app)/[orgId]/security/penetration-tests/_components/RunningDetail.tsx:122">
P2: Reset `useNewFindingHighlights` state when the run changes; otherwise switching runs can incorrectly flash all existing findings in the newly selected run as “new.”</violation>
</file>
<file name="apps/api/src/admin-organizations/admin-pentest-credits.controller.ts">
<violation number="1" location="apps/api/src/admin-organizations/admin-pentest-credits.controller.ts:55">
P2: This route shape is misparsed by `AdminAuditLogInterceptor`, causing incorrect audit metadata (`entityId` becomes `'grant'` and action context is lost).</violation>
<violation number="2" location="apps/api/src/admin-organizations/admin-pentest-credits.controller.ts:66">
P2: Manual credit grants are recorded under the org owner instead of the acting platform admin, which makes the audit trail inaccurate for admin actions.</violation>
</file>
<file name="apps/app/src/app/(app)/[orgId]/admin/organizations/[adminOrgId]/components/PentestCreditsTab.tsx">
<violation number="1" location="apps/app/src/app/(app)/[orgId]/admin/organizations/[adminOrgId]/components/PentestCreditsTab.tsx:45">
P2: Grant amount validation uses `parseInt`, which silently truncates decimal/scientific input and can grant an unintended amount; validate with `Number` + integer/range checks instead.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| defaultOpen = true, | ||
| }: AgentActivityLogProps) { | ||
| const recent = [...events] | ||
| .filter(isCustomerVisible) |
Contributor
There was a problem hiding this comment.
P1: Sensitive supplier/internal event data is redacted only at render time, so it is still exposed in the client API payload.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/app/src/app/(app)/[orgId]/security/penetration-tests/_components/AgentActivityLog.tsx, line 50:
<comment>Sensitive supplier/internal event data is redacted only at render time, so it is still exposed in the client API payload.</comment>
<file context>
@@ -0,0 +1,137 @@
+ defaultOpen = true,
+}: AgentActivityLogProps) {
+ const recent = [...events]
+ .filter(isCustomerVisible)
+ .sort((a, b) => b.timestamp - a.timestamp)
+ .slice(0, 200);
</file context>
API:
- refundOnTerminalFailure now wraps claim+refund in a single Prisma
transaction; if the wallet write fails, the creditRefundedAt claim
rolls back so a redelivered webhook can retry. Previously a transient
failure would permanently suppress retries and leave the customer
unrefunded.
- pentest-credits.refund() accepts an optional tx client so the webhook
path can run it transactionally.
- pentest.completed audit dedup via new completed_audit_at column on
security_penetration_test_runs (atomic claim before write — webhook
redelivery can no longer create duplicate audit log rows).
- debitOrThrow's post-debit getStatus now uses the optional tx client
rather than always going through the bare `db` connection (would
return stale balance when called inside a transaction).
- Restrict raw-body preservation in main.ts to webhook routes only;
attaching `rawBody` to every JSON request was duplicating up to 150MB
of payload memory per request.
- Restructure admin grant route POST `/:orgId/pentest-credits/grant`
→ POST `/:orgId/pentest-credits` so AdminAuditLogInterceptor's URL
parser doesn't treat "grant" as the entityId. Added `pentest-credits`
to SEGMENT_TO_RESOURCE so the audit row gets entityType=pentest.
Database:
- New migration 20260429120000_pentest_credits_balance_check:
- CHECK constraint enforcing balance >= 0 (defense-in-depth on the
atomic decrement we already do)
- Drop the redundant @@index on organization_id (already covered by
@unique's btree)
- Add completed_audit_at column for the audit dedup above
Frontend:
- SplitView: keep `balance` undefined while credits are loading; the
previous `?? 0` collapsed loading into "out of credits" and disabled
the +New button before we knew the user's real balance.
- SplitView download: derive Accept header from the filename's
extension; previously markdown downloads requested application/pdf
because the check was "filename ? pdf : markdown" but both formats
pass a filename.
- FindingDetail PoC: use truthiness so empty strings render the
fallback message instead of a blank block.
- PentestCreditsTab grant validation: Number() + isInteger() rather
than parseInt(), which silently truncated "1.5" → 1 and accepted
"1e3" as 1000.
- useNewFindingHighlights now keys on runId — switching scans no longer
flashes existing findings as newly arrived.
- usePenetrationTestIssues: trigger one final mutate() when status
transitions out of in-progress, so the last batch Maced writes
during completion is fetched.
- FindingsTable rows: role=button, tabIndex, onKeyDown(Enter/Space) so
keyboard users can open findings.
- relativeTime: "just now" only for < 1 minute (was <= 1, treating a
full minute as "just now").
- Pulse animation respects prefers-reduced-motion.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
These two specs were broken on `main` since the earlier SDK swap and GitHub-OAuth removal: - lib.test.ts referenced fields (userId, organizationId, sandboxId, workflowId, sessionId) that no longer exist on PentestRun - penetration-tests-page-client.test.tsx referenced useGithubRepos hook that was removed when we dropped GitHub OAuth token forwarding They've been throwing typecheck errors for weeks while providing zero real coverage. Deleting rather than rewriting — the few utility functions in lib.ts (statusLabel, isReportInProgress, etc.) can get a fresh small spec if needed in a follow-up; the page-client tests would need a full rewrite against the new SplitView architecture which is a separate effort. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… files - getReportEvents() now filters Maced-internal events at the API layer instead of trusting the frontend filter alone. The previous client-only filter left supplier names visible in DevTools / curl / any non-browser consumer of /v1/...:id/events. This is defense in depth against cubic's P1 — same predicate as the frontend's isCustomerVisible, but enforced server-side so the policy is in one place and bypass-proof. - pentest-credits.prisma: added a doc-comment on `balance` referencing the CHECK constraint migration (Prisma's schema DSL doesn't support CHECK constraints natively, so the invariant lives in 20260429120000_pentest_credits_balance_check). The earlier cubic flag predates that migration; this comment ensures future readers see both the code-side and DB-side enforcement. - Deleted apps/api/src/security-penetration-tests/README.md — module README that grew stale (referenced an older endpoint set / env vars) and no longer matched the implementation. The endpoint list is the source of truth in the controller; OpenAPI docs are the external contract. - Deleted tasks/todo.md — local planning doc that ended up tracked. It described an earlier proposed flow (check-then-create) that we superseded with the credit-atomic debit-first ordering in createReport. Cubic correctly flagged the doc's pseudocode as non-atomic; deleting the doc is the right answer rather than rewriting it to match shipped behavior. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
|
@cubic-dev-ai review it |
Contributor
@tofikwest I have started the AI code review. It will take a few minutes to complete. |
Contributor
There was a problem hiding this comment.
9 issues found across 71 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/app/src/app/(app)/[orgId]/security/penetration-tests/_components/RunList.tsx">
<violation number="1" location="apps/app/src/app/(app)/[orgId]/security/penetration-tests/_components/RunList.tsx:18">
P3: `overviewActive` is declared and passed from the parent, but never used in `RunList`, so the documented overview-highlight state is currently non-functional.</violation>
</file>
<file name="apps/app/src/app/(app)/[orgId]/security/penetration-tests/_components/AgentActivityLog.tsx">
<violation number="1" location="apps/app/src/app/(app)/[orgId]/security/penetration-tests/_components/AgentActivityLog.tsx:56">
P2: `open={defaultOpen}` makes the `</violation>
</file>
<file name="apps/app/src/app/(app)/[orgId]/security/penetration-tests/_components/OverviewPane.tsx">
<violation number="1" location="apps/app/src/app/(app)/[orgId]/security/penetration-tests/_components/OverviewPane.tsx:131">
P2: Coverage and stale-target calculations include non-completed runs, which can mark failed/cancelled/in-progress targets as covered and fresh.</violation>
</file>
<file name="apps/app/src/app/(app)/[orgId]/security/penetration-tests/_components/AgentProgressGrid.tsx">
<violation number="1" location="apps/app/src/app/(app)/[orgId]/security/penetration-tests/_components/AgentProgressGrid.tsx:37">
P2: Clamp `done` to the grid size before rendering completed cells; otherwise `done > total` renders extra cells beyond the configured columns.</violation>
</file>
<file name="apps/api/src/security-penetration-tests/security-penetration-tests.service.ts">
<violation number="1" location="apps/api/src/security-penetration-tests/security-penetration-tests.service.ts:159">
P1: Do not swallow refund transaction failures in webhook handling; rethrow so the webhook is retried and the credit refund is not dropped.</violation>
</file>
<file name="apps/app/src/app/(app)/[orgId]/security/penetration-tests/hooks/use-penetration-tests.ts">
<violation number="1" location="apps/app/src/app/(app)/[orgId]/security/penetration-tests/hooks/use-penetration-tests.ts:272">
P2: `usePenetrationTestEvents` stops polling on completion but does not perform a final revalidation, so the last events can be missed.</violation>
</file>
<file name="apps/app/src/app/(app)/[orgId]/security/penetration-tests/_components/StatusPill.tsx">
<violation number="1" location="apps/app/src/app/(app)/[orgId]/security/penetration-tests/_components/StatusPill.tsx:62">
P2: Unknown status values currently fall back to "Provisioning", which can display an incorrect in-progress state to users.</violation>
</file>
<file name="apps/app/src/app/(app)/[orgId]/security/penetration-tests/_components/RunningDetail.tsx">
<violation number="1" location="apps/app/src/app/(app)/[orgId]/security/penetration-tests/_components/RunningDetail.tsx:168">
P2: Clearing the timeout in effect cleanup can permanently skip highlight removal when `issues` changes before 2s, leaving stale highlighted IDs.</violation>
</file>
<file name="apps/api/src/security-penetration-tests/pentest-credits.service.ts">
<violation number="1" location="apps/api/src/security-penetration-tests/pentest-credits.service.ts:83">
P1: `grantInitialTrial` is never called outside tests, so new organizations will not automatically receive the intended trial credit.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
API:
- refundOnTerminalFailure no longer swallows errors. Failures now
propagate through handleWebhook → 5xx response → Maced redelivery →
rolled-back creditRefundedAt allows retry. Previously a transient
refund-tx blip would silently drop the customer's refund forever.
- Removed dead grantInitialTrial method. Trial credits are granted
via Prisma nested-create on Organization in the org-create server
actions, atomic with org insert. The unused service method just
invited future divergence; existing orgs were backfilled by the
20260427000000_pentest_credits migration. Added a comment in
pentest-credits.service explaining where the trial actually fires.
- Service spec updated (-1 dead test for the deleted method, total 19).
Frontend:
- AgentActivityLog: track open state in React so user collapse isn't
fought by the next parent re-render. Was rendering as
`<details open={defaultOpen}>` which forced the panel re-open every
SWR poll.
- OverviewPane: coverage and stale-target stats now derive from
`completed` runs only, not the full runs list. A target with only
failed scans is no longer counted as "covered" or "fresh."
- AgentProgressGrid: clamp `done` to grid size so an out-of-sync
Maced progress payload (`done > total`) doesn't render extra cells
past the configured columns.
- usePenetrationTestEvents: added the same final-revalidation pattern
as usePenetrationTestIssues so the last events written during the
completion phase aren't missed when polling stops.
- StatusPill: unknown status values now render as "Unknown" instead
of falling back to "Provisioning" (which would mislead users into
thinking the run was still starting up).
- RunningDetail useNewFindingHighlights: removed the timeout-cleanup
that was canceling pending highlight removals when issues changed
in <2s (during a 3s SWR poll, very common). Each batch now
schedules its own fire-and-forget removal so they can't step on
each other.
- RunList: removed unused `overviewActive` prop (declared but never
consumed) and stopped passing it from SplitView.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Build was failing because the new PurgeOrganizationSnapshotService (added on main while this branch was open) queried db.organizationBilling and db.pentestSubscription, both of which we dropped in migration 20260427000000_pentest_credits. The git auto-merge couldn't catch this — it's a logical conflict, not a textual one. Replace the queries with null literals. The snapshot's `stripe.customerId` and `stripe.subscriptionId` shape is preserved so downstream consumers (purge orchestrator, restore tooling) keep working; the legacy tables had no production data anyway, and v2 billing will add new fields here when it lands. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
|
🎉 This PR is included in version 3.36.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
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
2 participants
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
Adds a credit wallet for penetration tests (debit on create, refund on failure, idempotent under webhook redelivery), an admin tab to grant credits, audit-log entries for the full credit lifecycle, and a UX pass on the entire pentest experience driven by review feedback. Real data only — no fabricated severity rollups, durations, or trends in any surface.
Highlights
Credits backbone
pentest_creditswallet table with backfill (1 trial credit per existing org); legacy emptypentest_subscriptions/organization_billingtables droppedcredit_refunded_atcolumn onsecurity_penetration_test_runsfor atomic webhook-refund idempotencyPentestCreditsServicewithgrantInitialTrial(idempotent),grant, atomicdebitOrThrow(updateMany WHERE balance > 0),refund(no-op when no wallet row), all writing audit-log entries attributed to the org owner'pentest'added toAuditLogEntityTypeenumcreateReporthardeningpersistRunOwnershipupsert hardened to never overwriteorganizationId(defensive — Maced generates unique IDs in practice)Webhook handling
pentest.failed/pentest.cancelledwith claim-before-refund (updateMany WHERE creditRefundedAt IS NULL)pentest.completedcapturing finding count, duration, agent count@maced/api-client@0.9.1retry wrapper disabled (maxAttempts: 1) to avoid the crypticCannot construct a Request...error on retriable 5xx (their wrapper reuses a consumed Request body — bug reported)Admin panel
POST /v1/admin/organizations/:orgId/pentest-credits/grant(PlatformAdminGuard,source=manual, capped at 1000/grant, double-audited)Frontend
X scans remaining/ trial-used messaging)+ New scandisabled at zero balance with explicit tooltip + create-form banner<1mduration when Maced doesn't bumpupdatedAt, suppressedLast updatewhen equal toStarted, removed redundant right columnCompletedeverywhere (clean variant dropped to keep sidebar/detail consistent)macedfiltered out (white-label) plusTodoWritenoisecreatedAtinstead of trusting Macedprogress.elapsedMsis-security-enabledPostHog flag check removed from security/layout (RBACpentest:readalone gates access; flag was breaking dev after PostHog/session restarts)Tenant isolation
assertRunOwnershipreturning 404 (not 403) — org B can't enumerate org A's run idslistReportsfilters vialistOwnedRunIdsbefore mappingorganizationIdof the affected orgcompOrganizationId/compEnvironment/compApiVersionfor disaster-recovery of ownership tableTests
PentestCreditsServicecovering status, grant, debit (including concurrent-debit race), refund, audit-log writes, no-owner edge case, audit-failure tolerancetransformIgnorePatternsadded to api jest config so specs can load the ESM SDK (@maced/api-client,better-auth)Test plan
cd packages/db && bunx prisma migrate devpentest_creditstable exists with one row per existing org (balance: 1)/security/penetration-tests— sidebar footer shows1 scan remaining+ New scan, submit a target — credit decrements to 0, scan starts (or fails fast if Maced is misbehaving — that's their TRIGGER_SECRET_KEY infra issue)No findings reported in this scan, calm caveat below, Markdown + PDF in equal-weight outline buttons in the muted CTA card1 scan remaining)/<orgId>/admin/organizations/<otherOrgId>→Pentest creditstab. Grant5. Verify other org'sBalancejumps to5 + previous. Verify two audit log entries (one for the admin action, one in the receiving org's per-org audit log)GET /v1/security-penetration-tests/<orgA-run-id>— expect 404cd apps/api && npx jest src/security-penetration-tests/pentest-credits.service.spec→ all 17 passKnown limitations / follow-ups
/v1/pentests/:id/issuessometimes returns[]even when the report has findings (their structured Issues writer drops findings); we acknowledge this with copy "the downloaded report is the complete assessment record"updatedAtnot bumped on completion (we suppress dependent UI when timestamps collide)maxAttempts: 1)/severity-summaryaggregation endpoint to avoid N fan-out fetches for per-run issueslib.test.tsandpenetration-tests-page-client.test.tsxreference removed shapes (userIdonPentestRun,useGithubRepos) — broken onmainbefore this branch, not regressed by it. Worth deleting in a small cleanup PRdurationMspersistence: a follow-up would add aduration_mscolumn onsecurity_penetration_test_runswritten from thepentest.completedwebhook payload, so we could show real durations even when Maced'supdatedAtis staleMigration order on deploy
The migrations are non-destructive for tenant data: dropped tables (
pentest_subscriptions,organization_billing) were verified empty;pentest_creditsis new with a backfill; the new column is nullable.🤖 Generated with Claude Code