Fix/onboarding without trigger worker - #3570
Conversation
…access review Implements the admin-audit service (previously implemented: false) and fixes a correctness bug in the access review, then removes the duplicated user-scoping logic that made the two diverge. Admin Audit service Two checks over the Reports API admin audit log: - admin-privilege-changes: role and privilege grants/revocations. Super admin grants escalate to high severity; changes by configured approved actors pass with evidence rather than failing. - admin-security-events: admin console changes that weaken posture (2SV enforcement, OAuth API access, admin password reset), each with its own severity and remediation. Fix: admin roles assigned to groups were invisible employee-access treated every role assignment's assignedTo as a user id. Google sets assigneeType: 'group' on group assignments, where assignedTo is a group id from a different id space, so those matched no user and anyone holding admin access through a group was absent from the access review entirely. Group assignments are now expanded to their members, and each grant records whether it was direct or via a group -- provenance matters because a role held through a group is revoked by changing the group, not the user. One user-scope filter for sync and checks sync.controller.ts re-implemented the OU and include/exclude rules that check-user-filter.ts already had, kept aligned only by comments. Both now share one implementation, so the personnel list and the access review cannot disagree about the population. The shared filter is explicitly staged because the two callers legitimately differ: employee sync needs suspended users in scope to drive offboarding, while security checks exclude them. isGoogleWorkspaceUserInScope covers org unit, group and domain; isGoogleWorkspaceUserSelectedBySyncTerms covers the include/exclude selection; shouldIncludeGoogleWorkspaceUserForCheck composes both plus the activeness rule. The OU logic is unchanged. Group and domain filtering (Drata parity) New target_groups and target_domains variables, applied to both sync and checks. Direct group members only -- nested groups are skipped with a warning. A selected group with no members excludes everyone rather than silently disabling the filter. New OAuth scopes admin.directory.group.readonly, admin.directory.domain.readonly and admin.reports.audit.readonly. Existing connections predate all three and must reconnect for full function. Each degrades to an actionable finding naming the missing permission rather than erroring the run, so an unresolvable group assignment or unreadable audit log is reported instead of silently under-counting who has admin access. Test suite repair sync-gws.controller.spec.ts could not instantiate SyncController -- GenericDeviceSyncService was missing from the test module, so all 20 tests failed on a DI error and the offboarding coverage was dark. Adding the provider also surfaced a stale assertion: reactivation clears offboardDate (97636c4) but the test still expected the old payload. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
CMMC 2.0 Level 2 as an importable framework: 110 practices across 14 families aligned to NIST SP 800-171 Rev 2, 36 control templates covering every requirement, 14 policy templates with body content, and 25 task templates. Identifiers use the CMMC practice format (AC.L2-3.1.1). Family counts match the published structure: AC 22, AT 3, AU 9, CM 9, IA 11, IR 3, MA 6, MP 9, PS 2, PE 6, RA 3, CA 4, SC 16, SI 7. The payload matches ImportFrameworkDto, so the supported path is POST /v1/framework-editor/framework/import. import-cmmc.ts is a fallback for headless use, since that route sits behind PlatformAdminGuard and needs a browser session; it mirrors FrameworkExportService.import() and refuses to run if a framework of the same name already exists. Requirement statements paraphrase NIST SP 800-171 Rev 2 and should be verified against the official publication before use in a real assessment. The control groupings, policies and tasks are an editorial layer, not part of the standard. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
setup-comp.sh bootstraps a local stack; start-comp.sh starts Postgres, MinIO and the three dev servers, and is idempotent. start-comp.sh uses the dev:no-trigger scripts rather than `bun run dev`, because the default dev script runs `trigger dev` under `concurrently --kill-others` -- without a Trigger.dev account that process exits immediately and takes Next and Nest down with it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
filterUsersByOrgUnits became dead production code when Google Workspace employee sync moved onto the shared scope filter -- isGoogleWorkspaceUserInScope carries identical OU logic, and the only remaining caller was its own test. Its coverage moves to the shared filter's suite rather than being lost: exact and nested path matching, child OU inclusion, partial-segment rejection, root OU behaviour, and multiple target OUs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
authType and authenticatedUser were hand-written into each controller method -- the same seven-line block repeated 81 times across 14 of 94 controllers -- so the documented response contract was honoured by 17 controllers and silently absent from the other 77. /v1/tasks and /v1/frameworks returned no authType while /v1/vendors and /v1/policies did, for no principled reason. A global interceptor now attaches the fields, so the contract is uniform. This matters beyond tidiness: the OpenAPI-derived MCP server generates a client per endpoint, and inconsistent response shapes across endpoints become inconsistent generated tools. Deliberately conservative about what it touches. Only plain JSON objects are annotated; arrays, primitives, null, Buffers, streams, StreamableFile, Date and class instances pass through untouched, because grafting fields onto those corrupts the payload rather than annotating it. Requests with no auth context and @public() routes are skipped, and @SkipAuthContextResponse() opts out endpoints whose shape is externally constrained (webhooks, trust portal). A controller that still sets the fields itself wins, so the 81 hand-rolled copies can be removed file by file without double-writing. Also fixes a type error introduced when Google Workspace sync moved onto the shared scope filter: employee sync holds a narrower local GoogleWorkspaceUser than the checks do, so the filter now accepts a minimal GoogleWorkspaceFilterableUser (id, primaryEmail, orgUnitPath, suspended, archived) rather than demanding the full directory shape. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Feat/google workspace admin audit
refactor(api): drop sync-ou-filter now that scoping is shared
…ponses feat(api): attach auth context to responses via a global interceptor
Finishing onboarding failed on any deployment without a Trigger.dev worker.
`tasks.trigger('onboard-organization')` threw ApiClientMissingError, which
propagated out of the action as `success: false`, so the user saw "Failed to
complete onboarding" and was never redirected.
The organization was fine. Every write happens before that call — the org row,
`onboardingCompleted: true`, framework initialisation and the onboarding record
had all committed. Only the enrichment dispatch failed, and it took the whole
action down with it.
Running without a worker is a supported configuration for self-hosted installs,
so both dispatches are now non-fatal and independent of each other. When the
enrichment job is not accepted, `triggerJobId` stays null and no
publicAccessToken cookie is set — which downstream pages already handle, since
they read a null job id as "nothing in progress".
The cost of no worker is unchanged and unhidden: AI enrichment (risks, vendors,
tailored policies) does not run. The organization is otherwise fully usable,
and the failure is logged with the organization id rather than swallowed.
Extracted to `dispatchOnboardingJobs` so the behaviour is testable — the app
has no harness for exercising a next-safe-action server action, and building
one for this would have been disproportionate.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
There was a problem hiding this comment.
37 issues found across 39 files
Confidence score: 1/5
apps/api/src/integration-platform/controllers/sync.controller.tsandpackages/integration-platform/src/manifests/google-workspace/directory-client.tscan turn a selected-group scope or a 403/429 response into a broader or partial deletion set, deactivating valid same-domain employees. Preserve scope and propagate incomplete membership reads before applying deactivations.packages/integration-platform/src/manifests/google-workspace/role-assignments.tsignores Google’s uppercaseGROUPassignments and treats API failures as complete reviews, hiding group-assigned and custom admin access. Normalize the enum and return an explicit incomplete state on fetch failure.setup-comp.shis currently unable to reliably provision or start development environments: BSD macOSsortrejects every Bun version,SECRET_KEYis not passed into the API environment, and the no-Trigger path still launchestrigger dev. Use portable version checks, pass the API env file tofill_secret, and provide a true root no-Trigger command.packages/db/scripts/import-cmmc.tsbypasses the importer’s RBAC/audit boundary and drops document types from imported controls, creating both governance exposure and incomplete evidence mappings. Route through the authorized importer or enforce equivalent checks, and persistdocumentLinkstransactionally.
Not reviewed (too large): frameworks/cmmc-level-2/cmmc-level-2.import.json (~5,269 lines) - if these are generated or fixture files, add them to ignored paths to exclude them from future reviews.
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/api/src/integration-platform/controllers/sync.controller.ts">
<violation number="1" location="apps/api/src/integration-platform/controllers/sync.controller.ts:316">
P3: Removing the old `GOOGLE_WORKSPACE_SYNC_FILTER_MODES` validation leaves the `GOOGLE_WORKSPACE_SYNC_FILTER_MODES` constant and the `GoogleWorkspaceSyncFilterMode` type (lines 76-79) unused, and the new code no longer sanitizes `sync_user_filter_mode`. The subsequent warning now fires with misleading text whenever the stored mode isn't literally one of all/exclude/include (e.g. an empty or malformed value), saying 'with an empty list' even though the list isn't the cause. Delete the now-dead constant and make the warning message match the actual reason (`userFilterMode` invalid vs include-with-empty-list).</violation>
<violation number="2" location="apps/api/src/integration-platform/controllers/sync.controller.ts:492">
P1: When `target_groups` is configured, this line makes selected group members a domain-only deactivation scope. The deactivation loop then treats every same-domain member outside the selected group as deleted because they are absent from `scopedUsers`; gate offboarding by the configured scope instead of domain alone.</violation>
</file>
<file name="packages/integration-platform/src/manifests/google-workspace/__tests__/admin-audit.test.ts">
<violation number="1" location="packages/integration-platform/src/manifests/google-workspace/__tests__/admin-audit.test.ts:58">
P3: The mock `fetch` in `runCheck` returns all provided activities regardless of the `startTime` that `fetchAdminActivities` sends, so none of these tests validate the core audit-window behavior. A regression where the check drops or miscomputes the lookback `startTime` would pass unnoticed, since even the 'honours a configured lookback window' test only asserts the `evidence.lookbackDays` value derived from parsing, not the window actually requested from the API. Have the mock capture the `startTime` param and assert it, or add a test that verifies the requested window.</violation>
</file>
<file name="frameworks/cmmc-level-2/generator/add_content.py">
<violation number="1" location="frameworks/cmmc-level-2/generator/add_content.py:5">
P2: On any checkout without the author's private scratchpad, the documented regeneration command fails with `FileNotFoundError` before adding policy content. Resolve paths relative to the repository or accept explicit input/output paths, and write the payload consumed by the import workflow.</violation>
</file>
<file name="packages/integration-platform/src/manifests/google-workspace/directory-client.ts">
<violation number="1" location="packages/integration-platform/src/manifests/google-workspace/directory-client.ts:123">
P1: When one selected group returns 403 or 429, this catch converts the failure into a partial membership set. The employee sync can then treat members of that unreadable group as deleted and deactivate them, especially when another selected group shares the domain. Propagate the resolution failure and abort the sync, or preserve known membership, instead of returning a partial set.</violation>
</file>
<file name="packages/integration-platform/src/manifests/google-workspace/admin-audit-events.ts">
<violation number="1" location="packages/integration-platform/src/manifests/google-workspace/admin-audit-events.ts:160">
P1: When a tenant has more than 20,000 activities in the lookback window, `fetchAdminActivities` returns partial data and both checks can report a clean pass despite not reviewing the full history. Propagate truncation to the checks and fail or mark the result incomplete instead of allowing truncated results to pass.</violation>
</file>
<file name="apps/api/src/auth/auth-context-response.interceptor.ts">
<violation number="1" location="apps/api/src/auth/auth-context-response.interceptor.ts:74">
P2: When an authenticated response has a domain-level `authType`, this guard mistakes it for the auth-context field and omits the caller context. Mark fixed-contract endpoints with `@SkipAuthContextResponse()` or move the context field to a non-colliding shape.</violation>
<violation number="2" location="apps/api/src/auth/auth-context-response.interceptor.ts:81">
P2: This global APP_INTERCEPTOR appends `authenticatedUser` (id + email) to every session-authenticated JSON response, yet the `@SkipAuthContextResponse()` opt-out it ships with is not applied to a single endpoint. The decorator's own docs say to use it on trust-portal and externally consumed payloads 'where leaking an internal user id/email would be a disclosure', but no endpoint opts out while the interceptor is enabled for all 94 controllers. Any such endpoint now echoes the user's email in its body for the first time. Audit which endpoints are externally consumed and apply `@SkipAuthContextResponse()` to them before enabling this globally.</violation>
</file>
<file name="apps/app/src/app/(app)/onboarding/lib/dispatch-onboarding-jobs.ts">
<violation number="1" location="apps/app/src/app/(app)/onboarding/lib/dispatch-onboarding-jobs.ts:47">
P2: When Trigger.dev has a transient API or network failure, this catch drops the job instead of arranging a retry. Because `completeOnboarding` ignores `failed` and stores no `triggerJobId`, the completed-organization redirect leaves no recovery path for AI enrichment or fleet-label creation. Persist a retryable outbox/failure or schedule retries while keeping onboarding successful.</violation>
</file>
<file name="packages/integration-platform/src/manifests/google-workspace/checks/admin-security-events.ts">
<violation number="1" location="packages/integration-platform/src/manifests/google-workspace/checks/admin-security-events.ts:72">
P2: When an audited setting is changed to a stronger value, this loop still creates a failure because it never evaluates `NEW_VALUE`. Distinguish weakening values from hardening values before calling `ctx.fail()`.</violation>
<violation number="2" location="packages/integration-platform/src/manifests/google-workspace/checks/admin-security-events.ts:72">
P2: `REVOKE_ADMIN_PRIVILEGE` produces a duplicate finding in this settings check and in `admin-privilege-changes`. Exclude privilege events here or split the shared map so this check remains limited to security-setting events.</violation>
<violation number="3" location="packages/integration-platform/src/manifests/google-workspace/checks/admin-security-events.ts:85">
P2: When two same-named audit events share a timestamp, this ID collides and marking one finding as an exception applies to both. Include the activity `uniqueQualifier` and an event-specific discriminator in `resourceId`.</violation>
</file>
<file name="apps/api/src/app.module.ts">
<violation number="1" location="apps/api/src/app.module.ts:153">
P2: Platform-admin endpoints will not receive the auth context fields because `PlatformAdminGuard` does not populate `request.authType`. Either populate the same auth context used by the interceptor or explicitly exclude these routes from the uniform-response contract.</violation>
</file>
<file name="packages/db/scripts/import-cmmc.ts">
<violation number="1" location="packages/db/scripts/import-cmmc.ts:20">
P1: This executable bypasses the framework importer’s RBAC boundary, allowing any process that can run it with database credentials to create platform framework data without an API permission check or audit context. Route the operation through an authenticated RBAC-protected API or add an explicitly authorized administrative execution boundary.</violation>
<violation number="2" location="packages/db/scripts/import-cmmc.ts:50">
P2: Payloads from an unsupported export format are accepted here and may be written with incompatible semantics instead of being rejected. Reject versions other than `"1"` before validating or importing the payload.</violation>
<violation number="3" location="packages/db/scripts/import-cmmc.ts:134">
P1: When the payload contains document types, this script silently drops them, so imported controls lose their required evidence-document mappings. Create and persist `documentLinks` inside the transaction just like the API importer.</violation>
</file>
<file name="start-comp.sh">
<violation number="1" location="start-comp.sh:13">
P2: When a developer uses Linux or Intel macOS and Postgres is not already running, the hardcoded `/opt/homebrew`/`postgresql@17` path prevents this launcher from starting the documented local stack. Resolve Homebrew dynamically or provide a platform-independent database path and an explicit platform check.</violation>
<violation number="2" location="start-comp.sh:25">
P2: When another checkout runs the same development command, `./start-comp.sh stop` terminates that checkout too because `pkill -f` is system-wide. Record the launched PIDs or process group and stop only processes started by this script.</violation>
<violation number="3" location="start-comp.sh:35">
P2: When the Homebrew cluster has not been initialized with the repository’s `comp` database and credentials, `pg_isready` reports success but the API cannot connect, and the launcher continues. Use the repository’s database service or verify and initialize the configured database before declaring Postgres ready.</violation>
<violation number="4" location="start-comp.sh:46">
P2: When the default environment is used, starting MinIO does not route uploads to it because this script never configures the S3 endpoint or credentials for the child servers. Set the local S3 configuration before launching or remove the MinIO readiness claim until the apps are configured to use it.</violation>
<violation number="5" location="start-comp.sh:47">
P2: When MinIO responds with 503 or another HTTP error, these probes still report it as ready because `curl` lacks `--fail`, so the script skips startup and hides the upload failure. Use `curl -fsS` for every health probe.</violation>
<violation number="6" location="start-comp.sh:64">
P2: When an unrelated process already owns one of these ports, the launcher reports the Comp service as already running and skips it, so the frontends can use the wrong server. Validate the listener identity or the service health endpoint before accepting an occupied port.</violation>
</file>
<file name="packages/integration-platform/src/manifests/google-workspace/role-assignments.ts">
<violation number="1" location="packages/integration-platform/src/manifests/google-workspace/role-assignments.ts:29">
P1: Google returns the role-assignment enum as `GROUP`, but `isGroupAssignment` only recognizes lowercase `group`, so group-assigned admin roles remain invisible to every user. Normalize the API value or compare against the provider’s uppercase enum.</violation>
<violation number="2" location="packages/integration-platform/src/manifests/google-workspace/role-assignments.ts:78">
P1: When the role-assignment API fails, this fallback makes the access review look complete while dropping custom and group-assigned admin roles. Propagate the failure or return an explicit incomplete state that `employeeAccessCheck` records as a check failure rather than treating partial assignments as authoritative.</violation>
</file>
<file name="setup-comp.sh">
<violation number="1" location="setup-comp.sh:14">
P2: The default repo path is hardcoded to the author's personal machine directory `/Users/chris/Code/comp`. Any other developer running `./setup-comp.sh` without an argument fails with "Repo not found" because that path does not exist on their machine, and the author's home path is needlessly embedded in the script. Default to the current directory (or the repo root) instead.</violation>
<violation number="2" location="setup-comp.sh:35">
P1: On macOS, BSD `sort` has no `-V` option, so this pipeline errors and rejects every Bun version. Use a portable numeric version comparison instead of `sort -V`.</violation>
<violation number="3" location="setup-comp.sh:64">
P3: `concurrently` and `turbo` are already root devDependencies (`^9.2.4` and `^2.10.5`), so the setup script's `bun add -d concurrently` and global `bun add -g turbo` are redundant and mutate the repo's package.json/bun.lock (and install a global turbo) purely as setup side effects. Drop these commands and let `bun install` handle them.</violation>
<violation number="4" location="setup-comp.sh:104">
P2: The secret examples scanned by `fill_secret` include inline comments, but this regex requires the assignment to end immediately after the empty value. `fill_secret` therefore silently leaves `BETTER_AUTH_SECRET` empty; allow trailing whitespace and comments in the match.</violation>
<violation number="5" location="setup-comp.sh:113">
P1: The script creates `apps/api/.env` but never passes it to `fill_secret`, leaving its required `SECRET_KEY` empty. The API then throws `SECRET_KEY environment variable is required` at startup; include the API env file in this loop.</violation>
<violation number="6" location="setup-comp.sh:137">
P2: The instructions tell users to configure `GOOGLE_ID` and `GOOGLE_SECRET`, but the env templates and auth configuration use `AUTH_GOOGLE_ID` and `AUTH_GOOGLE_SECRET`. Following this step leaves Google OAuth unset; print the exact variable names.</violation>
<violation number="7" location="setup-comp.sh:154">
P1: The migration command runs from the repository root, but root `package.json` has no `db:migrate` script, so onboarding stops with `Script not found`. Run the migration script from `packages/db` or add a root script.</violation>
<violation number="8" location="setup-comp.sh:155">
P1: This claims local startup does not require Trigger, but `bun run dev` still launches `trigger dev` for both the app and API. Use a root no-trigger development path, because the current concurrent commands can terminate the app when the Trigger process is unavailable.</violation>
</file>
<file name="frameworks/cmmc-level-2/generator/build.py">
<violation number="1" location="frameworks/cmmc-level-2/generator/build.py:2">
P1: The generator hardcodes the author's private scratchpad path (`/private/tmp/claude-501/...chris-Code-comp...`) for `sys.path.insert` and the payload output. On any other machine `python3 generator/build.py` fails to import `practices`, and the payload is written outside the repo instead of the committed `cmmc-level-2.import.json`, so the README's documented regeneration command cannot run and cannot update the import payload. Derive paths from the script location (e.g. `Path(__file__).parent`) and write to `frameworks/cmmc-level-2/cmmc-level-2.import.json`.</violation>
<violation number="2" location="frameworks/cmmc-level-2/generator/build.py:142">
P1: The documented build command cannot run in a clean checkout because this output directory does not exist, so `json.dump` raises `FileNotFoundError`; it also writes the result outside the repository. Use a repository-relative or CLI-configured output path, and keep it consistent with `add_content.py` and `cmmc-level-2.import.json`.</violation>
</file>
<file name="packages/integration-platform/src/manifests/google-workspace/checks/admin-privilege-changes.ts">
<violation number="1" location="packages/integration-platform/src/manifests/google-workspace/checks/admin-privilege-changes.ts:33">
P2: When Google returns a renamed, localized, or otherwise unlisted role name, `touchesSuperAdmin` reports the change at medium severity instead of high. Resolve the role through Google’s role-management API and use its authoritative super-admin metadata rather than a hardcoded role-name gate.</violation>
</file>
<file name="frameworks/cmmc-level-2/import-cmmc.ts">
<violation number="1" location="frameworks/cmmc-level-2/import-cmmc.ts:10">
P2: This 178-line file is an exact duplicate of `packages/db/scripts/import-cmmc.ts` (verified byte-identical). The README only documents the `packages/db` copy as the runnable script, so this copy is dead code that must be kept in sync whenever the API import service changes. Delete one copy and reference the single script from the README.</violation>
</file>
<file name="packages/integration-platform/src/manifests/google-workspace/variables.ts">
<violation number="1" location="packages/integration-platform/src/manifests/google-workspace/variables.ts:147">
P1: The new `targetGroupsVariable`/`targetDomainsVariable` `fetchOptions` pass the `VariableFetchContext` (whose `fetch` only accepts a path and never forwards query params) into `listGroups`/`listDomains`, which depend on `options.params`. The required `customer` param is dropped, so Google's Groups.list request fails and the catch returns an empty picker. Pass a context whose fetch supports params (or have the picker build the URL with params itself) before wiring these variables.</violation>
</file>
Tip: instead of fixing issues one by one fix them all with cubic
Re-trigger cubic
|
|
||
| const deactivationGwDomains = new Set( | ||
| ouFilteredUsers.map((u) => u.primaryEmail.split('@')[1]?.toLowerCase()), | ||
| scopedUsers.map((u) => u.primaryEmail.split('@')[1]?.toLowerCase()), |
There was a problem hiding this comment.
P1: When target_groups is configured, this line makes selected group members a domain-only deactivation scope. The deactivation loop then treats every same-domain member outside the selected group as deleted because they are absent from scopedUsers; gate offboarding by the configured scope instead of domain alone.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/api/src/integration-platform/controllers/sync.controller.ts, line 492:
<comment>When `target_groups` is configured, this line makes selected group members a domain-only deactivation scope. The deactivation loop then treats every same-domain member outside the selected group as deleted because they are absent from `scopedUsers`; gate offboarding by the configured scope instead of domain alone.</comment>
<file context>
@@ -493,7 +489,7 @@ export class SyncController {
const deactivationGwDomains = new Set(
- ouFilteredUsers.map((u) => u.primaryEmail.split('@')[1]?.toLowerCase()),
+ scopedUsers.map((u) => u.primaryEmail.split('@')[1]?.toLowerCase()),
);
const deactivationSuspendedEmails =
</file context>
| for (const id of await fetchGroupMemberUserIds({ client, groupId })) { | ||
| memberIds.add(id); | ||
| } | ||
| } catch { |
There was a problem hiding this comment.
P1: When one selected group returns 403 or 429, this catch converts the failure into a partial membership set. The employee sync can then treat members of that unreadable group as deleted and deactivate them, especially when another selected group shares the domain. Propagate the resolution failure and abort the sync, or preserve known membership, instead of returning a partial set.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/integration-platform/src/manifests/google-workspace/directory-client.ts, line 123:
<comment>When one selected group returns 403 or 429, this catch converts the failure into a partial membership set. The employee sync can then treat members of that unreadable group as deleted and deactivate them, especially when another selected group shares the domain. Propagate the resolution failure and abort the sync, or preserve known membership, instead of returning a partial set.</comment>
<file context>
@@ -0,0 +1,166 @@
+ for (const id of await fetchGroupMemberUserIds({ client, groupId })) {
+ memberIds.add(id);
+ }
+ } catch {
+ // Surface loudly: a group that cannot be read would otherwise silently
+ // shrink the synced population.
</file context>
| pageToken = response.nextPageToken; | ||
| pages += 1; | ||
|
|
||
| if (pageToken && pages >= MAX_PAGES) { |
There was a problem hiding this comment.
P1: When a tenant has more than 20,000 activities in the lookback window, fetchAdminActivities returns partial data and both checks can report a clean pass despite not reviewing the full history. Propagate truncation to the checks and fail or mark the result incomplete instead of allowing truncated results to pass.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/integration-platform/src/manifests/google-workspace/admin-audit-events.ts, line 160:
<comment>When a tenant has more than 20,000 activities in the lookback window, `fetchAdminActivities` returns partial data and both checks can report a clean pass despite not reviewing the full history. Propagate truncation to the checks and fail or mark the result incomplete instead of allowing truncated results to pass.</comment>
<file context>
@@ -0,0 +1,167 @@
+ pageToken = response.nextPageToken;
+ pages += 1;
+
+ if (pageToken && pages >= MAX_PAGES) {
+ ctx.warn(`Stopped paging admin activity after ${MAX_PAGES} pages; results may be truncated`);
+ break;
</file context>
| const dto = JSON.parse(fs.readFileSync(payloadPath, 'utf-8')); | ||
|
|
||
| const adapter = new PrismaPg({ connectionString: process.env.DATABASE_URL! }); | ||
| const prisma = new PrismaClient({ adapter }); |
There was a problem hiding this comment.
P1: This executable bypasses the framework importer’s RBAC boundary, allowing any process that can run it with database credentials to create platform framework data without an API permission check or audit context. Route the operation through an authenticated RBAC-protected API or add an explicitly authorized administrative execution boundary.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/db/scripts/import-cmmc.ts, line 20:
<comment>This executable bypasses the framework importer’s RBAC boundary, allowing any process that can run it with database credentials to create platform framework data without an API permission check or audit context. Route the operation through an authenticated RBAC-protected API or add an explicitly authorized administrative execution boundary.</comment>
<file context>
@@ -0,0 +1,178 @@
+const dto = JSON.parse(fs.readFileSync(payloadPath, 'utf-8'));
+
+const adapter = new PrismaPg({ connectionString: process.env.DATABASE_URL! });
+const prisma = new PrismaClient({ adapter });
+
+function normalizeTipTapDoc(content: unknown): Record<string, unknown> {
</file context>
| ); | ||
| } | ||
|
|
||
| const policyLinks = (dto.controlTemplates ?? []).flatMap((ct: any, ci: number) => |
There was a problem hiding this comment.
P1: When the payload contains document types, this script silently drops them, so imported controls lose their required evidence-document mappings. Create and persist documentLinks inside the transaction just like the API importer.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/db/scripts/import-cmmc.ts, line 134:
<comment>When the payload contains document types, this script silently drops them, so imported controls lose their required evidence-document mappings. Create and persist `documentLinks` inside the transaction just like the API importer.</comment>
<file context>
@@ -0,0 +1,178 @@
+ );
+ }
+
+ const policyLinks = (dto.controlTemplates ?? []).flatMap((ct: any, ci: number) =>
+ (ct.policyTemplateIndices ?? []).map((pi: number) => ({
+ frameworkId: fw.id,
</file context>
| const fields: AuthContextResponseFields = { authType: request.authType }; | ||
|
|
||
| if (request.userId && request.userEmail) { | ||
| fields.authenticatedUser = { |
There was a problem hiding this comment.
P2: This global APP_INTERCEPTOR appends authenticatedUser (id + email) to every session-authenticated JSON response, yet the @SkipAuthContextResponse() opt-out it ships with is not applied to a single endpoint. The decorator's own docs say to use it on trust-portal and externally consumed payloads 'where leaking an internal user id/email would be a disclosure', but no endpoint opts out while the interceptor is enabled for all 94 controllers. Any such endpoint now echoes the user's email in its body for the first time. Audit which endpoints are externally consumed and apply @SkipAuthContextResponse() to them before enabling this globally.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/api/src/auth/auth-context-response.interceptor.ts, line 81:
<comment>This global APP_INTERCEPTOR appends `authenticatedUser` (id + email) to every session-authenticated JSON response, yet the `@SkipAuthContextResponse()` opt-out it ships with is not applied to a single endpoint. The decorator's own docs say to use it on trust-portal and externally consumed payloads 'where leaking an internal user id/email would be a disclosure', but no endpoint opts out while the interceptor is enabled for all 94 controllers. Any such endpoint now echoes the user's email in its body for the first time. Audit which endpoints are externally consumed and apply `@SkipAuthContextResponse()` to them before enabling this globally.</comment>
<file context>
@@ -0,0 +1,109 @@
+ const fields: AuthContextResponseFields = { authType: request.authType };
+
+ if (request.userId && request.userEmail) {
+ fields.authenticatedUser = {
+ id: request.userId,
+ email: request.userEmail,
</file context>
|
|
||
| set -euo pipefail | ||
|
|
||
| REPO="${1:-/Users/chris/Code/comp}" |
There was a problem hiding this comment.
P2: The default repo path is hardcoded to the author's personal machine directory /Users/chris/Code/comp. Any other developer running ./setup-comp.sh without an argument fails with "Repo not found" because that path does not exist on their machine, and the author's home path is needlessly embedded in the script. Default to the current directory (or the repo root) instead.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At setup-comp.sh, line 14:
<comment>The default repo path is hardcoded to the author's personal machine directory `/Users/chris/Code/comp`. Any other developer running `./setup-comp.sh` without an argument fails with "Repo not found" because that path does not exist on their machine, and the author's home path is needlessly embedded in the script. Default to the current directory (or the repo root) instead.</comment>
<file context>
@@ -0,0 +1,165 @@
+
+set -euo pipefail
+
+REPO="${1:-/Users/chris/Code/comp}"
+
+bold() { printf '\033[1m%s\033[0m\n' "$*"; }
</file context>
| REPO="${1:-/Users/chris/Code/comp}" | |
| REPO="${1:-$(pwd)}" # or: ${1:-$PWD} |
| const scopedUsers = users.filter((user) => | ||
| isGoogleWorkspaceUserInScope(user, filterConfig), | ||
| ); | ||
| const effectiveSyncFilterMode = resolveEffectiveSyncFilterMode(filterConfig); |
There was a problem hiding this comment.
P3: Removing the old GOOGLE_WORKSPACE_SYNC_FILTER_MODES validation leaves the GOOGLE_WORKSPACE_SYNC_FILTER_MODES constant and the GoogleWorkspaceSyncFilterMode type (lines 76-79) unused, and the new code no longer sanitizes sync_user_filter_mode. The subsequent warning now fires with misleading text whenever the stored mode isn't literally one of all/exclude/include (e.g. an empty or malformed value), saying 'with an empty list' even though the list isn't the cause. Delete the now-dead constant and make the warning message match the actual reason (userFilterMode invalid vs include-with-empty-list).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/api/src/integration-platform/controllers/sync.controller.ts, line 316:
<comment>Removing the old `GOOGLE_WORKSPACE_SYNC_FILTER_MODES` validation leaves the `GOOGLE_WORKSPACE_SYNC_FILTER_MODES` constant and the `GoogleWorkspaceSyncFilterMode` type (lines 76-79) unused, and the new code no longer sanitizes `sync_user_filter_mode`. The subsequent warning now fires with misleading text whenever the stored mode isn't literally one of all/exclude/include (e.g. an empty or malformed value), saying 'with an empty list' even though the list isn't the cause. Delete the now-dead constant and make the warning message match the actual reason (`userFilterMode` invalid vs include-with-empty-list).</comment>
<file context>
@@ -285,57 +292,46 @@ export class SyncController {
+ const scopedUsers = users.filter((user) =>
+ isGoogleWorkspaceUserInScope(user, filterConfig),
);
+ const effectiveSyncFilterMode = resolveEffectiveSyncFilterMode(filterConfig);
+ const excludedTerms = filterConfig.excludedTerms;
</file context>
| }, | ||
| fetch: (async <T,>(): Promise<T> => { | ||
| if (fetchError) throw fetchError; | ||
| return { items: activities } as unknown as T; |
There was a problem hiding this comment.
P3: The mock fetch in runCheck returns all provided activities regardless of the startTime that fetchAdminActivities sends, so none of these tests validate the core audit-window behavior. A regression where the check drops or miscomputes the lookback startTime would pass unnoticed, since even the 'honours a configured lookback window' test only asserts the evidence.lookbackDays value derived from parsing, not the window actually requested from the API. Have the mock capture the startTime param and assert it, or add a test that verifies the requested window.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/integration-platform/src/manifests/google-workspace/__tests__/admin-audit.test.ts, line 58:
<comment>The mock `fetch` in `runCheck` returns all provided activities regardless of the `startTime` that `fetchAdminActivities` sends, so none of these tests validate the core audit-window behavior. A regression where the check drops or miscomputes the lookback `startTime` would pass unnoticed, since even the 'honours a configured lookback window' test only asserts the `evidence.lookbackDays` value derived from parsing, not the window actually requested from the API. Have the mock capture the `startTime` param and assert it, or add a test that verifies the requested window.</comment>
<file context>
@@ -0,0 +1,231 @@
+ },
+ fetch: (async <T,>(): Promise<T> => {
+ if (fetchError) throw fetchError;
+ return { items: activities } as unknown as T;
+ }) as CheckContext['fetch'],
+ } as CheckContext;
</file context>
| bun install | ||
| ok "bun install" | ||
|
|
||
| bun add -d concurrently >/dev/null 2>&1 && ok "concurrently" || warn "concurrently install skipped" |
There was a problem hiding this comment.
P3: concurrently and turbo are already root devDependencies (^9.2.4 and ^2.10.5), so the setup script's bun add -d concurrently and global bun add -g turbo are redundant and mutate the repo's package.json/bun.lock (and install a global turbo) purely as setup side effects. Drop these commands and let bun install handle them.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At setup-comp.sh, line 64:
<comment>`concurrently` and `turbo` are already root devDependencies (`^9.2.4` and `^2.10.5`), so the setup script's `bun add -d concurrently` and global `bun add -g turbo` are redundant and mutate the repo's package.json/bun.lock (and install a global turbo) purely as setup side effects. Drop these commands and let `bun install` handle them.</comment>
<file context>
@@ -0,0 +1,165 @@
+bun install
+ok "bun install"
+
+bun add -d concurrently >/dev/null 2>&1 && ok "concurrently" || warn "concurrently install skipped"
+
+if ! command -v turbo >/dev/null 2>&1; then
</file context>
What does this PR do?
Visual Demo (For contributors especially)
A visual demonstration is strongly recommended, for both the original and new change (video / image - any one).
Video Demo (if applicable):
Image Demo (if applicable):
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?
Checklist
Summary by cubic
Fixes onboarding on deployments without a Trigger.dev worker, adds the Google Workspace admin audit service and a group-aware access review, and includes a CMMC Level 2 framework, a global auth-context response interceptor, and local dev scripts.
Onboarding without a worker
triggerJobIdstays null and AI enrichment is skipped; the organization is otherwise fully usable.Google Workspace integration
admin-privilege-changesandadmin-security-eventschecks over the Reports API audit log.Written for commit 9df829b. Summary will update on new commits.