Conversation
…age asks for (#3520) * fix(api): switch a passkey 2FA prompt to the authenticator-code method Vendors like GitHub default the 2FA step to a passkey / security-key prompt (github.com/sessions/two-factor/webauthn) — which we can't use and which has no 6-digit field, so the panel said 'enter the code' while the page showed no place to type it. After sign-in, best-effort switch to the authenticator-app option (More options -> Use authenticator app / Enter a code) so the code field is actually shown — for us to fill a stored code, or for the user to type one during take-over. +test. * fix: tailor the 2FA take-over to what the page actually asks for Make the two-factor take-over universal instead of assuming an authenticator code: - Classify the verification page into how it wants the user to verify (a code, a passkey that can be switched away from, a passkey-only login, or another challenge) and return it from the sign-in result. - When Comp AI has a stored code, auto-switch off a passkey so the code field appears, then fill it. When a human is taking over, don't pick a method for them — just reveal the vendor's other options so they choose the one they can complete. - Show exact guidance in the take-over panel from that classification: enter the code, switch off the passkey, or a plain notice that a passkey-only login can't be automated (with the confirm button hidden, since there's nothing to complete). No vendor names are hard-coded. Adds unit coverage for the classifier, the take-over vs stored-code login behavior, and the panel copy for each method state. * refactor: address review on the 2FA take-over changes - Split the passkey→code switch into two single-action steps (reveal the other options, then choose the authenticator/code method). act() performs one action per call, so a combined 'click More options, then Use authenticator app' could stop after the first click and never reach the code field. - Trust an explicit method classification when routing the take-over panel: an 'other' verification (device approval, CAPTCHA, link) now uses the generic finish panel instead of code-entry guidance, even when the outcome is needs_2fa. Falls back to the failure code only when no method was detected. - Extract the page classifiers (classifyLoginOutcome, classifyTwoFactorMethod, safeOriginAndPath) into a focused browser-login-classifier module, bringing the login file back under the 300-line limit. Tests moved/updated alongside; 186 browserbase tests green, typecheck clean.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
All reported issues were addressed across 9 files
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
A passkey prompt (e.g. GitHub's webauthn step) asks for a security key rather than a code, so the outcome classifier can land on 'unknown' instead of 'needs_2fa'. Classify the verification method on 'unknown' as well so the take-over panel still recognizes the passkey and guides the user to a code method. The classifier degrades to 'other' for a genuinely non-verification page, so the panel stays generic there.
* feat(security-questionnaire): add browser extension
* fix(security-questionnaire-extension): address review findings
- Use a closed shadow root for the domain confirmation dialog so the host
page cannot reach its controls.
- Escape leading formula characters when building Sheets paste TSV, so
answers starting with "-" stay text instead of being evaluated.
- Restrict the wildcard trusted-origin match to HTTPS; explicit localhost
entries are unaffected.
- Stop generate-all from overwriting approved or manually edited answers.
- Hold the inline preview locally so concurrent generates cannot render a
result into another field's preview.
- Finalize queue item status when single generation fails, instead of
leaving the item stuck in "generating".
- Implement the Google Docs footer copy action, which had no handler, and
label/enable the button to match.
Adds coverage for the paste plan, batch candidate selection, docs footer,
clipboard text, and the origin policy.
* fix(security-questionnaire-extension): make Google OAuth optional in release, add docs
The release workflow required a Google OAuth client id and failed the build
when the manifest had none, so the extension could not ship without direct
Google Sheets API access. Treat it as optional: verify the manifest only
when the secret is set, and drop it from the required-secret check.
Adds the extension documentation page and links it from the docs nav.
* fix(security-questionnaire-extension): resolve remaining review findings
Detection and data correctness:
- Stop dropping questionnaire rows whose text looks like a column label.
Any cell under 48 characters containing "control", "requirement" or
"question" was discarded, which silently lost common questions such as
"Are security controls documented?".
- Ignore answer-style statements in the keyword fallback so filled-in
answers are not re-detected as new questions.
- Reject row/column 0 when parsing sheet targets; A1 notation is 1-based.
- Reject negative sheet ids when building formatting requests.
Concurrency and data loss:
- Apply batch results to the stored queue rather than a snapshot, and skip
items the user edited or approved while generation was in flight.
- Serialize storage read-merge-writes so concurrent updates are not lost.
- Keep the sheet mapping dialog populated when a save fails.
- Surface autosave failures instead of dropping them.
Robustness:
- Guard JSON parsing in the API client, GViz table parsing and gid decoding.
- Let one Sheets endpoint failure fall through to the next.
- Fall back to local detection when the background message fails.
- Add Vary headers to CORS responses, including rejected origins.
- Trim organization ids and reject empty question ids and whitespace-only
approvals.
- Tighten isItemResponse to validate the queue it claims to carry.
Cleanup: drop the unused ReviewPanel and the unhandled
comp:generate-visible-page message, dedupe getHost and surface validation.
* fix(api): keep HEAD in the CORS allowed methods
The replacement CORS middleware dropped HEAD, which app.enableCors allowed
by default, so cross-origin HEAD preflights would have started failing.
* fix(security-questionnaire-extension): publish via service account on the working API
Switches Chrome Web Store auth from a personal refresh token to a service
account JWT, which does not expire and is not tied to an individual account.
Verified against the live API: the token exchange succeeds and the service
account can read the store item. Every v2 endpoint returns 404 for this
publisher while v1.1 works with the same token, so the upload, status and
publish calls now use v1.1. That API is supported until 2026-10-15.
Adds a publish_target choice so a manual run can ship to trusted testers
before going public.
* fix(security-questionnaire-extension): isolate failed queue writes in batch generation
A rejected save left the serialized write chain permanently rejected, so
every answer generated afterwards was computed and then silently dropped.
Keep the chain alive, collect per-item write failures instead of throwing
from a pool worker (which abandoned the questions queued behind it), and
report them once the batch has persisted everything it could.
* fix(security-questionnaire-extension): repair the manifest version check
The check embedded a JS template literal inside a double-quoted shell
string, so bash tried to expand ${process.env.VERSION} and aborted the
step with 'bad substitution'. The release workflow has never run, so this
would have failed on the first publish. Use string concatenation instead.
* fix(security-questionnaire-extension): drop the unused activeTab permission
Every activeTab occurrence in the source is a local variable name; the
permission itself is never relied on, and the <all_urls> host permission
already covers the same access. The store rejects versions that request
permissions they do not need.
Also refuses to publish a package whose manifest still points at
localhost, which is what the build produces when the public URL env vars
are missing.
* fix(security-questionnaire-extension): keep failed questions retryable
When a queue write failed the question stayed marked `generating`, and
that status is excluded from batch candidates — so every later Generate
All skipped it and the answer could never be recovered. Reset the item to
pending after a failed write.
* fix(security-questionnaire-extension): restore prior status after a failed write
Resetting a failed question to pending discarded whatever state it had
before the batch, so a transient storage error downgraded a question that
already held a generated or flagged answer. Restore the status captured
before the batch marked it generating.
…ins (#3523) * test(api): lock in that production origins still pass CORS The CORS layer was replaced in #3064 to support the questionnaire extension. These assert the four origin classes that actually reach production — the app, the portal, an arbitrary trycomp.ai subdomain, and a customer custom domain — still receive the right headers, that an untrusted origin is still rejected, and that resolution does not depend on AUTH_TRUSTED_ORIGINS, which the API is not given in production. Tests only; no production code changes. * test(api): use reserved example domains in the auth specs The specs hardcoded a developer's real local unpacked-extension id and used evil.com / acmecorp.com, both of which are real registrable domains, in a public repository. Replace them with a synthetic extension id and RFC 2606 reserved names (untrusted.example, trust.example.com), and drop the attack framing. Behaviour of the tests is unchanged.
* fix(training): remove rbac gate from mark-complete endpoint
## Problem
A user with a custom role lacking portal:update permission is unable to mark training videos complete, receiving a 403 error and seeing "failed to mark video as completed" in the UI. The user can accept policies and complete device setup successfully, but gets blocked at the training endpoint.
## Root cause
The POST /v1/training/completions/:videoId/complete endpoint is gated by @RequirePermission('portal','update') in training.controller.ts. This creates an inconsistent permission model: policy acceptance uses session + member-ownership only (no RBAC check), while training completion enforces a portal:update requirement that custom roles may lack. Existing custom roles created before permission enforcement was added still lack this permission.
## Fix
Remove the @RequirePermission('portal','update') guard from the mark-complete endpoint. Training completion is a user action on their own record (like policy acceptance), not a privileged administrative operation, so it should authenticate by session + member-ownership only, not RBAC.
## Explicitly NOT touched
Policy enforcement at role creation time (from PR #3455) remains in place. Other training endpoints and their permission gates are unchanged. Portal write permissions on other operations are unaffected.
## Verification
Added regression test asserting that a user with a custom role lacking portal:update can successfully mark a training video complete. Existing training controller unit tests pass locally ✅.
* fix(todo): address cubic review
Address review findings.
* fix(todo): address cubic review
Address review findings.
* fix: address cubic review
Address review findings.
* fix(portal): log training completion-email failures in production (CS-774)
The completion-email failure path logged via the `logger` util, which no-ops
outside development — so in production a failed completion email was swallowed
silently (negating the response.ok check). Use console.error for this
operational error so it is visible in prod. Test now asserts console.error.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
… unlinking (#3513) * fix(policies): remove archived policies from framework controls after unlinking ## Problem A customer manually unlinked archived policies from controls. However the policies still appear in the control's policy list, even though viewing the policy directly shows it as unlinked from any controls. This creates a contradiction where the control says the policy is linked but the policy says it is not. ## Root cause Two bugs compound the issue. (1) removePolicyControl in policies.controller.ts only deletes the FrameworkControlPolicyLink join row for custom frameworks (`customFrameworkId:{not:null}` guard at line 1027). Platform framework links (like SOC 2) are never deleted from the join table. Meanwhile findOneForFramework in controls.service.ts reads a platform control's policy list entirely from this join table, so stale rows remain visible. (2) Control and framework reads filter policies only on `archivedAt:null`, omitting the `isArchived:false` check. This contradicts the policy.prisma rule (lines 37-41) which hides user-archived policies. So policies marked as archived by the user still display and score against framework progress. ## Fix Removed the `customFrameworkId:{not:null}` gate in removePolicyControl so the join row deletion runs for all framework types, not just custom ones. Relaxed the before-guard to let deletion proceed when the m2m link exists. Added `isArchived:false` filter to all control/framework/score policy queries to respect the archive state. ## Explicitly NOT touched No schema migration. No RBAC or organizationId scoping change. Auth rules remain unchanged. ## Verification Added regression test asserting that unlinking a policy from a platform framework (SOC 2) removes it from both the join table and the control's returned policy list. Confirmed that filtering on `isArchived:false` prevents user-archived policies from appearing. Unit tests pass locally ✅ * fix(policies): always sever framework join rows on unlink (CS-780 join-only links) The unlink still gated the FrameworkControlPolicyLink deleteMany on an implicit m2m link existing (`before?.controls.length`). Framework-scoped links created via LinkPolicySheet make ONLY a FrameworkControlPolicyLink (no implicit m2m), so unlinking one skipped the delete and the policy kept showing against the control — the same CS-780 contradiction. Run the org-scoped deleteMany unconditionally (idempotent; the org-scoped policy.update still aborts for foreign/missing policies). Regression test covers the no-m2m-link case. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…creen step (#3525) Returning to a task with an unfinished vendor connect used to auto-reopen that sign-in full-screen, trapping the user in it — there was no way to create a new (or different) automation without cancelling out. Now the in-progress connect is surfaced as a resumable band (mirroring the existing drafts strip): the user lands on their automations and chooses — Resume to finish it, Discard to drop it, or just Create/Connect a new one. Starting a fresh connect clears any half-finished one so the flow never resumes into a stale draft. Same resumable steps as before (analysis / method choice); only the forced takeover is gone. Adds ResumeConnectStrip + a test.
* feat: surface connection longevity and a 'Make permanent' 2FA flow Make it obvious, per connection, whether a saved vendor sign-in keeps running on its own — and give a one-click way to fix the ones that don't. Connections page: - Each connection now shows a longevity state read live from the vault: Stays signed in (password + authenticator), Signed in for now (password, no key — may pause when the vendor asks for a code), Signed in (SSO — occasional re-sign-in), or Reconnect needed. Each with a plain-language hint. - A dismissible banner nudges when connections can be made permanent. - A focused 'Make permanent' sheet stores the authenticator setup key (with per-vendor guidance, key validation, and a success state). We never change the vendor's 2FA — the user enables it and pastes the one-time setup key. - New batch endpoint GET /profiles/totp-statuses returns 2FA status for all password connections in one round-trip (reuses the live vault read; no schema change). Task flow: - The connection menu gains 'Keep signed in (2FA)', reusing the same sheet, so a connection can be made permanent without leaving the task. Tests: batch status service (per-connection, degrades unreadable items); row states + actions; sheet validation and save/success. API + app typecheck clean. * fix: address review on the connection-permanence changes - Scope the 2FA-status SWR cache to the org so switching orgs can't show another org's statuses. - Treat an unreadable/failed status as UNKNOWN, never 'no key': the batch endpoint omits connections it couldn't read (instead of reporting false), the hook surfaces its error, and rows show 'Status unavailable' with no 'Make permanent' prompt — so an outage never invites overwriting an existing key. The at-risk banner counts only confirmed 'no key' connections. - Bound the 1Password fan-out in the batch endpoint (was unbounded per org). - Refresh statuses after a connect/reconnect so a new row isn't stale. - Treat a never-verified session as 'reconnect', not 'stays signed in', even with a stored key. - Make the 'Make permanent' sheet resilient: a rejected save (or a failed status refresh after a successful save) returns to the form with an error instead of hanging on 'Saving…'; store the normalized Base32 key (spaces/hyphens stripped). - Remove the task-flow 'Keep signed in (2FA)' menu item: ConnectionManageMenu is not mounted anywhere and connection management deliberately lives on the Connections page, so it was dead code. Deferred: splitting the browser-auth-profiles controller (already over the file limit before this change) into a focused module. * refactor(api): extract the 2FA (TOTP) endpoints into a focused controller Moves the four TOTP routes (batch status, per-connection get/set/clear) out of BrowserAuthProfilesController into a dedicated BrowserAuthTotpController, bringing the profiles controller back under the 300-line limit and grouping the 2FA routes. Same paths, guards, permissions, and behavior — no route or contract change. * fix(api): validate the authenticator setup key server-side + cover the TOTP controller - setProfileTotp now validates and normalizes the key on the server (the client check is only UX): it accepts a Base32 secret or an otpauth:// URI and rejects a rotating one-time code or junk, so a direct API caller can't silently replace a working seed with something that can't generate codes — which would quietly break unattended 2FA. Extracted as normalizeTotpSecret with unit tests. - Add controller-level tests for BrowserAuthTotpController: organization + profile ids reach the storage service on all four routes, and reads require integration:read while writes require integration:update. * fix(api): keep the setup-key parser linear on untrusted input Strip trailing Base32 padding with a plain scan instead of a start-anchorless regex (which can backtrack polynomially on untrusted input), and bound the input length up front. Behavior is unchanged for valid keys; adds padding + long-input tests. Clears the CodeQL polynomial-regexp alert.
…o view (#3526) ## Problem Clicking the + Add button on the AWS Integrations page appears unresponsive. No modal opens, no feedback is given, and the page does not navigate or scroll. Users cannot see the form to add a new AWS account. ## Root cause When a user clicks + Add on an AWS integration card, the form to add a new account is rendered but placed outside the page's main Stack container it mounts thousands of pixels below the fold after the evidence tasks section, cloud tests link, and the 46-card AWS services grid. Without a Dialog wrapper or scrollIntoView call, the viewport never changes and the form remains invisible to the user. The React #418 hydration error in the console is a secondary red herring; the click handler binds correctly (focus ring confirms the click reached the button), so the real blocker is the off-screen form rendering. ## Fix Wrapped the AWS add account form in a Dialog component and call scrollIntoView on mount. This ensures the form appears on top of the page content in a modal, is immediately visible to the user, and provides the expected UX feedback when the button is clicked. ## Explicitly NOT touched - OAuth integrations or multi-step auth flows - AWS manifest configuration or connection logic - Other integration providers - Hydration or SSR behavior ## Verification - Added regression test asserting that clicking + Add renders the form inside a Dialog and the form is visible in the viewport - Existing AWS integration unit tests pass ✅
* fix: harden sign-in classification and align take-over messaging Addresses review findings on the browser sign-in path: - Treat the browser's current URL as untrusted data in the classification prompt (labeled, delimited) so page-path text can't steer the outcome; the query and fragment were already stripped. - Tighten the 2FA-method classifier so a device approval, push notification, or email/SMS link is classified as 'other' rather than over-matching a passkey. - Align the take-over toast and panel to one decision, so they can't contradict (e.g. an 'enter your code' toast over a 'finish sign-in' panel). - Add direct unit tests for classifyLoginOutcome (each outcome, throw -> unknown, URL passed as untrusted with secrets stripped, content-only when no URL). * fix(api): keep the URL hint as data — ignore non-http schemes + XML-escape A non-http(s) page URL (data:, about:, javascript:, …) isn't percent-encoded and could carry characters that break out of the <current_url> delimiter. Restrict the URL hint to http(s) origins and XML-escape the value before interpolation, so it always stays inside its tag as data. Adds tests for both.
#3528) Tighten the 2FA-method classifier so 'passkey' requires a CODE-based alternative (authenticator app, SMS, or email code). A passkey / security-key prompt whose only alternative is a device approval, push, or email/SMS link is now 'other' — so the take-over panel doesn't tell the user to 'enter a code' when no code method is actually available.
#3529) * fix(training): defer completion email and share canonical training IDs The portal's complete-training route awaited the completion-certificate email before responding. That API call resolves the member, renders a PDF and calls the email provider, so every final-video completion made the employee wait seconds for work whose outcome they never see — and Node's fetch has no default request timeout, so an unreachable API could stall the response for minutes even though the completion was already durable. Defer it with `after` so it runs once the response is sent, and bound the request with an AbortSignal timeout so a hung API can't pin the invocation open. The email path performs no DB or storage writes, only an in-memory PDF render and a send, so nothing the UI reads afterwards depends on it having finished. The route also restated the valid-video-ID list and the HIPAA eligibility rule that apps/api owns, a fifth copy of constants already duplicated across both apps and two API modules. Move them to @trycompai/company — the existing shared-definitions package for api/app/portal — so the two completion paths cannot disagree about which videos exist or which orgs may complete HIPAA training. The two apps' training data files link to the canonical IDs by type only. They are imported by client components and by a Trigger.dev task, and @trycompai/company requires @trycompai/db at runtime, so a value re-export would have pulled Prisma into those bundles. The type-level tie still fails the build on drift, in both directions: an unknown ID in a rendered list, or a canonical ID that no longer matches. Also fixes the pre-existing training-hipaa spec, whose @db mock was never given frameworkInstance when the HIPAA gate was added, and adds coverage for the gate's rejection path. * fix(training): make the canonical/rendered video lists exhaustive both ways `satisfies readonly CanonicalTrainingVideo[]` only proved that every rendered video is canonical. The reverse — a canonical ID with no rendered video — still compiled, and that is the dangerous direction: completion requires a row for every canonical ID, so adding one without a matching video would leave employees permanently unable to finish training. Key the data by the canonical union instead. `Record<GeneralTrainingVideoId, ...>` is exhaustive by construction, so both drifts now fail the build: a missing key (TS2741) and an unknown key (TS2353). Verified by introducing each drift and compiling. The `Exclude<GeneralTrainingVideoId, (typeof trainingVideos)[number]['id']>` form would not have worked here — the `readonly TrainingVideo[]` annotation widens `id` to `string`, so the Exclude resolves to `never` and the guard passes vacuously.
Contributor
|
🎉 This PR is included in version 3.111.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.
This is an automated pull request to release the candidate branch into production, which will trigger a deployment.
It was created by the [Production PR] action.
Summary by cubic
Ships universal 2FA hand‑off, the security‑questionnaire Chrome extension, stricter route‑scoped CORS/origin policy, a policies bulk upload flow, a resumable connect strip, and connection longevity with a one‑click “Make permanent” 2FA flow. Also hardens sign‑in classification and aligns take‑over messaging, fixes policy unlinking/visibility and training completion auth, adds a HIPAA completion email endpoint, requires
logGroupNamein CloudTrail→CloudWatch remediation, and shows the AWS “+ Add account” form in a dialog.New Features
@trycompai/security-questionnaire-extension(MV3) with vendor/Google Sheets/Docs support, a release workflow, and optional Google OAuth; add an origin policy plus custom CORS with HTTPS‑only wildcards,COMP_EXTENSION_TRUSTED_ORIGINS, and route‑scoped allowlists.otpauth://); add batch status endpointGET /v1/browserbase/profiles/totp-statuses.Bug Fixes
isArchived:false) and always sever framework join rows on unlink, including join‑only cases.portal:update; log completion‑email failures in production; defer sending the completion email until after the response with a timeout; share canonical training video IDs via@trycompai/companyand key app video lists by those IDs to prevent drift.organizationIdwhen saving generated answers.post()calls so one failure doesn’t reject the whole batch, and preserve any createdpolicyIdfor retry.logGroupNameforCreateLogGroupCommandto fail fast; only backfill a name when derivable fromCloudWatchLogsLogGroupArnto avoid false successes; add tests.Written for commit fecb556. Summary will update on new commits.