feat(api): credential-backed auto re-login for browser automations#3410
feat(api): credential-backed auto re-login for browser automations#3410tofikwest wants to merge 1 commit into
Conversation
Browser automations can now sign back in on their own when a vendor session expires, instead of always requiring a person to re-authenticate. - Store a profile's login (username, password, optional authenticator setup key) in a per-organization 1Password vault via a new credential-storage service and a POST profiles/:id/credentials endpoint. - Resolve those credentials just-in-time during a run through the previously-stubbed credential vault adapter, now backed by 1Password, and drive sign-in with Stagehand variable substitution so secret values are never sent to the model. - Retry once with a freshly generated one-time code to cover the rotation boundary; fall back to human re-authentication when a login needs a step the agent cannot complete (SMS/email/push/SSO). - Let the scheduler attempt a run for a needs-reauth profile that has stored credentials, and mark a profile verified again after a successful run. - Load the 1Password SDK lazily and mark it external for Trigger.dev builds; the feature stays inert unless OP_SERVICE_ACCOUNT_TOKEN is set. Adds unit tests for the credential resolver and the re-login flow.
There was a problem hiding this comment.
15 issues found across 21 files
Confidence score: 2/5
- In
apps/api/src/browserbase/onepassword-credential-vault.adapter.ts, credential lookup is not tenant-bound, so a misconfigured or tampered profile reference could resolve another organization’s 1Password item if access permits; this is the highest-risk cross-tenant exposure in the PR — enforce organization/tenant scoping during item resolution before merging. - In
apps/api/src/browserbase/browser-automation-execution.service.ts, profile verification can be marked successful from a live session that is not owned by that profile, which can incorrectly bless the wrong profile and hide real auth state issues — move/require the profile-session ownership check before execution and verification updates. - Across
apps/api/src/browserbase/onepassword-credential-vault.adapter.tsandapps/api/src/browserbase/browser-credential-login.ts, transient vault failures and partial secret resolution can degrade into placeholder-based or partial login attempts, increasing flaky automation and incorrect fallbacks to human re-auth — fail fast unless both username and password are resolved and split login actions into atomic steps. - In
apps/api/src/browserbase/browser-credential-storage.service.ts, rotation/create flows can leave stale or orphaned secrets when updates fail, which accumulates sensitive data and can duplicate credentials on retry — add rollback/cleanup around DB update failures and retire or update prior vault items after successful rotation.
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/browserbase/browser-credential-vault.factory.ts">
<violation number="1" location="apps/api/src/browserbase/browser-credential-vault.factory.ts:14">
P3: Unattended re-login can be silently disabled if this selection logic regresses to the Noop adapter, and the current suite would not detect it. A focused factory test covering both a trimmed token and an unset/blank token would lock down the credential-activation behavior.</violation>
</file>
<file name="apps/api/src/browserbase/browser-evidence-execution.ts">
<violation number="1" location="apps/api/src/browserbase/browser-evidence-execution.ts:97">
P2: A successful re-login can switch the active tab/page, but this assignment is discarded before screenshot and evaluation: `resolveEvidencePage` is called with the stale `initialPage` instead. Making the post-login page the resolver's anchor would prevent evidence from being captured from the wrong page when the login flow changes pages.</violation>
</file>
<file name="apps/api/src/browserbase/browser-automation-execution.service.ts">
<violation number="1" location="apps/api/src/browserbase/browser-automation-execution.service.ts:144">
P2: Profiles with only `vaultProvider` metadata are allowed into the auto-relogin path even though no credential item may be resolvable. Gate this on the required external reference as well so unusable profiles remain in the human re-auth flow.</violation>
<violation number="2" location="apps/api/src/browserbase/browser-automation-execution.service.ts:199">
P1: A successful live-session execution can now mark the run's profile verified even when the supplied Browserbase session is not that profile's session. The profile/session ownership check should occur before execution, otherwise an unrelated session can clear `needs_reauth` or `blocked` state.</violation>
</file>
<file name="apps/api/src/browserbase/browser-auth-profiles.controller.ts">
<violation number="1" location="apps/api/src/browserbase/browser-auth-profiles.controller.ts:125">
P3: Clients receive 201 from this route while the generated OpenAPI contract declares 200, so integrations that follow the documented response can mis-handle successful credential storage. The contract would be consistent if this documented 201 or the method explicitly returned 200.</violation>
<violation number="2" location="apps/api/src/browserbase/browser-auth-profiles.controller.ts:126">
P3: The new credential endpoint has no controller coverage for RBAC protection, organization/profile forwarding, DTO mapping, or service errors, leaving the security-sensitive API boundary unverified. A focused controller spec would make these behaviors regression-tested.</violation>
</file>
<file name="apps/api/src/browserbase/browser-auth-profile.service.ts">
<violation number="1" location="apps/api/src/browserbase/browser-auth-profile.service.ts:209">
P3: The new successful-run profile transition is currently untested, so regressions in status, `lastVerifiedAt`, or blocked-reason clearing can pass the API suite unnoticed. Adding service tests for an unverified/reauth profile, an already-verified profile, and a missing profile would cover this behavior.</violation>
<violation number="2" location="apps/api/src/browserbase/browser-auth-profile.service.ts:215">
P3: Successful runs for an already-verified profile leave `lastVerifiedAt` stale because this return skips the metadata update; `applyProfileResult` invokes `markVerified` after every successful run. Removing the short-circuit (or guarding only when all verification metadata is already current) keeps profile verification state accurate.</violation>
</file>
<file name="apps/api/src/browserbase/onepassword-credential-vault.adapter.ts">
<violation number="1" location="apps/api/src/browserbase/onepassword-credential-vault.adapter.ts:34">
P1: Credential resolution is not tenant-bound: a profile's stored reference is trusted verbatim, allowing a misconfigured or tampered profile to resolve another organization's 1Password item if the service account can access it. Derive the vault/item reference server-side from the organization or validate the reference against an organization-owned vault before calling `secrets.resolve()`; the adapter currently lacks the organization context needed for that check.</violation>
<violation number="2" location="apps/api/src/browserbase/onepassword-credential-vault.adapter.ts:71">
P2: A transient or authorization failure in 1Password is treated as an absent optional field, so automated runs can attempt login with incomplete variables or silently fall back to the human re-auth path instead of surfacing a vault outage. Restrict this handling to a confirmed missing-field error and propagate authentication/transport failures, or return a distinct resolution failure to the caller.</violation>
</file>
<file name="apps/api/src/browserbase/browser-credential-storage.service.ts">
<violation number="1" location="apps/api/src/browserbase/browser-credential-storage.service.ts:38">
P3: The credential write path is untested, so regressions in vault enumeration, secret-field mapping, profile scoping, and external-create/database-update failures can pass CI unnoticed. A colocated service spec with mocked 1Password and Prisma clients would cover the storage contract and error cases.</violation>
<violation number="2" location="apps/api/src/browserbase/browser-credential-storage.service.ts:68">
P2: Credential rotation creates a new Login item while leaving the previous password and TOTP seed in the organization vault indefinitely. Updating the existing referenced item, or deleting the old item after a successful replacement, would prevent stale credentials from accumulating and remaining accessible.</violation>
<violation number="3" location="apps/api/src/browserbase/browser-credential-storage.service.ts:68">
P2: The 1Password item is created before the profile update, so a DB failure after `items.create()` leaves an orphaned secret item and makes retries duplicate credentials. A small cleanup/rollback path around the update would keep the vault and profile metadata in sync.</violation>
</file>
<file name="apps/api/src/browserbase/browser-credential-login.ts">
<violation number="1" location="apps/api/src/browserbase/browser-credential-login.ts:33">
P2: A partially resolved vault item sends an unbound `%username%` or `%password%` placeholder to Stagehand, so the credential action cannot reliably fill the form. Requiring both username and password before issuing this two-field instruction, or generating separate actions for the fields that exist, would fail safely into the human re-auth path.</violation>
<violation number="2" location="apps/api/src/browserbase/browser-credential-login.ts:36">
P2: The automated login can stop after handling only part of the form because this `act()` call bundles multiple actions, including conditional navigation and submission. Splitting the flow into atomic username, password, and submit actions would make ordinary and multi-step login forms reliably complete.</violation>
</file>
Tip: instead of fixing issues one by one fix them all with cubic
Re-trigger cubic
| // A successful run proves the session is valid again — clear any prior | ||
| // needs_reauth/blocked state (e.g. after a credential-backed auto sign-in). | ||
| if (input.result.success) { | ||
| await this.profiles.markVerified({ |
There was a problem hiding this comment.
P1: A successful live-session execution can now mark the run's profile verified even when the supplied Browserbase session is not that profile's session. The profile/session ownership check should occur before execution, otherwise an unrelated session can clear needs_reauth or blocked state.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/api/src/browserbase/browser-automation-execution.service.ts, line 199:
<comment>A successful live-session execution can now mark the run's profile verified even when the supplied Browserbase session is not that profile's session. The profile/session ownership check should occur before execution, otherwise an unrelated session can clear `needs_reauth` or `blocked` state.</comment>
<file context>
@@ -188,6 +193,16 @@ export class BrowserAutomationExecutionService {
+ // A successful run proves the session is valid again — clear any prior
+ // needs_reauth/blocked state (e.g. after a credential-backed auto sign-in).
+ if (input.result.success) {
+ await this.profiles.markVerified({
+ organizationId: input.organizationId,
+ profileId: input.profileId,
</file context>
| }): Promise<RuntimeCredentialMaterial | null> { | ||
| if (params.provider !== ONEPASSWORD_PROVIDER) return null; | ||
|
|
||
| const itemRef = params.externalItemRef?.trim(); |
There was a problem hiding this comment.
P1: Credential resolution is not tenant-bound: a profile's stored reference is trusted verbatim, allowing a misconfigured or tampered profile to resolve another organization's 1Password item if the service account can access it. Derive the vault/item reference server-side from the organization or validate the reference against an organization-owned vault before calling secrets.resolve(); the adapter currently lacks the organization context needed for that check.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/api/src/browserbase/onepassword-credential-vault.adapter.ts, line 34:
<comment>Credential resolution is not tenant-bound: a profile's stored reference is trusted verbatim, allowing a misconfigured or tampered profile to resolve another organization's 1Password item if the service account can access it. Derive the vault/item reference server-side from the organization or validate the reference against an organization-owned vault before calling `secrets.resolve()`; the adapter currently lacks the organization context needed for that check.</comment>
<file context>
@@ -0,0 +1,78 @@
+ }): Promise<RuntimeCredentialMaterial | null> {
+ if (params.provider !== ONEPASSWORD_PROVIDER) return null;
+
+ const itemRef = params.externalItemRef?.trim();
+ if (!itemRef) return null;
+
</file context>
| (await checkAuth(activeStagehand)).isLoggedIn, | ||
| log: (message) => log('auth', message), | ||
| }); | ||
| page = relogin.page ?? page; |
There was a problem hiding this comment.
P2: A successful re-login can switch the active tab/page, but this assignment is discarded before screenshot and evaluation: resolveEvidencePage is called with the stale initialPage instead. Making the post-login page the resolver's anchor would prevent evidence from being captured from the wrong page when the login flow changes pages.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/api/src/browserbase/browser-evidence-execution.ts, line 97:
<comment>A successful re-login can switch the active tab/page, but this assignment is discarded before screenshot and evaluation: `resolveEvidencePage` is called with the stale `initialPage` instead. Making the post-login page the resolver's anchor would prevent evidence from being captured from the wrong page when the login flow changes pages.</comment>
<file context>
@@ -74,12 +81,38 @@ export async function executeBrowserEvidence({
+ (await checkAuth(activeStagehand)).isLoggedIn,
+ log: (message) => log('auth', message),
+ });
+ page = relogin.page ?? page;
+
+ if (!relogin.isLoggedIn) {
</file context>
| // own, so let a needs_reauth profile attempt the run instead of blocking it. | ||
| // Profiles that are blocked or never verified still require a human. | ||
| const canAutoRelogin = | ||
| profile.status === 'needs_reauth' && Boolean(profile.vaultProvider); |
There was a problem hiding this comment.
P2: Profiles with only vaultProvider metadata are allowed into the auto-relogin path even though no credential item may be resolvable. Gate this on the required external reference as well so unusable profiles remain in the human re-auth flow.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/api/src/browserbase/browser-automation-execution.service.ts, line 144:
<comment>Profiles with only `vaultProvider` metadata are allowed into the auto-relogin path even though no credential item may be resolvable. Gate this on the required external reference as well so unusable profiles remain in the human re-auth flow.</comment>
<file context>
@@ -137,7 +137,12 @@ export class BrowserAutomationExecutionService {
+ // own, so let a needs_reauth profile attempt the run instead of blocking it.
+ // Profiles that are blocked or never verified still require a human.
+ const canAutoRelogin =
+ profile.status === 'needs_reauth' && Boolean(profile.vaultProvider);
+ if (profile.status !== 'verified' && !canAutoRelogin) {
const result = this.profileBlockedResult(profile.status);
</file context>
| profile.status === 'needs_reauth' && Boolean(profile.vaultProvider); | |
| profile.status === 'needs_reauth' && | |
| Boolean(profile.vaultProvider && profile.vaultExternalItemRef); |
| try { | ||
| const value = await client.secrets.resolve(reference); | ||
| return value?.trim() ? value : null; | ||
| } catch { |
There was a problem hiding this comment.
P2: A transient or authorization failure in 1Password is treated as an absent optional field, so automated runs can attempt login with incomplete variables or silently fall back to the human re-auth path instead of surfacing a vault outage. Restrict this handling to a confirmed missing-field error and propagate authentication/transport failures, or return a distinct resolution failure to the caller.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/api/src/browserbase/onepassword-credential-vault.adapter.ts, line 71:
<comment>A transient or authorization failure in 1Password is treated as an absent optional field, so automated runs can attempt login with incomplete variables or silently fall back to the human re-auth path instead of surfacing a vault outage. Restrict this handling to a confirmed missing-field error and propagate authentication/transport failures, or return a distinct resolution failure to the caller.</comment>
<file context>
@@ -0,0 +1,78 @@
+ try {
+ const value = await client.secrets.resolve(reference);
+ return value?.trim() ? value : null;
+ } catch {
+ // A missing optional field (e.g. a login with no TOTP configured) resolves
+ // as an error; treat it as absent rather than failing the whole sign-in.
</file context>
| @ApiParam({ name: 'profileId', description: 'Browser auth profile ID' }) | ||
| @ApiBody({ type: StoreAuthProfileCredentialsDto }) | ||
| @ApiResponse({ status: 200, type: BrowserAuthProfileResponseDto }) | ||
| async storeProfileCredentials( |
There was a problem hiding this comment.
P3: The new credential endpoint has no controller coverage for RBAC protection, organization/profile forwarding, DTO mapping, or service errors, leaving the security-sensitive API boundary unverified. A focused controller spec would make these behaviors regression-tested.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/api/src/browserbase/browser-auth-profiles.controller.ts, line 126:
<comment>The new credential endpoint has no controller coverage for RBAC protection, organization/profile forwarding, DTO mapping, or service errors, leaving the security-sensitive API boundary unverified. A focused controller spec would make these behaviors regression-tested.</comment>
<file context>
@@ -112,6 +113,30 @@ export class BrowserAuthProfilesController {
+ @ApiParam({ name: 'profileId', description: 'Browser auth profile ID' })
+ @ApiBody({ type: StoreAuthProfileCredentialsDto })
+ @ApiResponse({ status: 200, type: BrowserAuthProfileResponseDto })
+ async storeProfileCredentials(
+ @OrganizationId() organizationId: string,
+ @Param('profileId') profileId: string,
</file context>
| }) | ||
| @ApiParam({ name: 'profileId', description: 'Browser auth profile ID' }) | ||
| @ApiBody({ type: StoreAuthProfileCredentialsDto }) | ||
| @ApiResponse({ status: 200, type: BrowserAuthProfileResponseDto }) |
There was a problem hiding this comment.
P3: Clients receive 201 from this route while the generated OpenAPI contract declares 200, so integrations that follow the documented response can mis-handle successful credential storage. The contract would be consistent if this documented 201 or the method explicitly returned 200.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/api/src/browserbase/browser-auth-profiles.controller.ts, line 125:
<comment>Clients receive 201 from this route while the generated OpenAPI contract declares 200, so integrations that follow the documented response can mis-handle successful credential storage. The contract would be consistent if this documented 201 or the method explicitly returned 200.</comment>
<file context>
@@ -112,6 +113,30 @@ export class BrowserAuthProfilesController {
+ })
+ @ApiParam({ name: 'profileId', description: 'Browser auth profile ID' })
+ @ApiBody({ type: StoreAuthProfileCredentialsDto })
+ @ApiResponse({ status: 200, type: BrowserAuthProfileResponseDto })
+ async storeProfileCredentials(
+ @OrganizationId() organizationId: string,
</file context>
| @ApiResponse({ status: 200, type: BrowserAuthProfileResponseDto }) | |
| @ApiResponse({ status: 201, type: BrowserAuthProfileResponseDto }) |
| return { profile: updated, auth }; | ||
| } | ||
|
|
||
| async markVerified(input: { organizationId: string; profileId: string }) { |
There was a problem hiding this comment.
P3: The new successful-run profile transition is currently untested, so regressions in status, lastVerifiedAt, or blocked-reason clearing can pass the API suite unnoticed. Adding service tests for an unverified/reauth profile, an already-verified profile, and a missing profile would cover this behavior.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/api/src/browserbase/browser-auth-profile.service.ts, line 209:
<comment>The new successful-run profile transition is currently untested, so regressions in status, `lastVerifiedAt`, or blocked-reason clearing can pass the API suite unnoticed. Adding service tests for an unverified/reauth profile, an already-verified profile, and a missing profile would cover this behavior.</comment>
<file context>
@@ -206,6 +206,24 @@ export class BrowserAuthProfileService {
return { profile: updated, auth };
}
+ async markVerified(input: { organizationId: string; profileId: string }) {
+ const profile = await this.getProfile(input);
+ if (!profile) {
</file context>
| throw new NotFoundException('Browser auth profile not found'); | ||
| } | ||
| // Avoid a needless write when the profile is already verified. | ||
| if (profile.status === 'verified') return profile; |
There was a problem hiding this comment.
P3: Successful runs for an already-verified profile leave lastVerifiedAt stale because this return skips the metadata update; applyProfileResult invokes markVerified after every successful run. Removing the short-circuit (or guarding only when all verification metadata is already current) keeps profile verification state accurate.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/api/src/browserbase/browser-auth-profile.service.ts, line 215:
<comment>Successful runs for an already-verified profile leave `lastVerifiedAt` stale because this return skips the metadata update; `applyProfileResult` invokes `markVerified` after every successful run. Removing the short-circuit (or guarding only when all verification metadata is already current) keeps profile verification state accurate.</comment>
<file context>
@@ -206,6 +206,24 @@ export class BrowserAuthProfileService {
+ throw new NotFoundException('Browser auth profile not found');
+ }
+ // Avoid a needless write when the profile is already verified.
+ if (profile.status === 'verified') return profile;
+
+ return db.browserAuthProfile.update({
</file context>
| if (profile.status === 'verified') return profile; | |
| // Refresh verification metadata after every successful run. |
| * item. Comp never persists the raw secrets itself — only the `op://` reference. | ||
| */ | ||
| @Injectable() | ||
| export class BrowserCredentialStorageService { |
There was a problem hiding this comment.
P3: The credential write path is untested, so regressions in vault enumeration, secret-field mapping, profile scoping, and external-create/database-update failures can pass CI unnoticed. A colocated service spec with mocked 1Password and Prisma clients would cover the storage contract and error cases.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/api/src/browserbase/browser-credential-storage.service.ts, line 38:
<comment>The credential write path is untested, so regressions in vault enumeration, secret-field mapping, profile scoping, and external-create/database-update failures can pass CI unnoticed. A colocated service spec with mocked 1Password and Prisma clients would cover the storage contract and error cases.</comment>
<file context>
@@ -0,0 +1,148 @@
+ * item. Comp never persists the raw secrets itself — only the `op://` reference.
+ */
+@Injectable()
+export class BrowserCredentialStorageService {
+ private readonly logger = new Logger(BrowserCredentialStorageService.name);
+
</file context>
What this does
Browser automations capture compliance evidence (screenshots) from vendor dashboards that have no API. Today, when a vendor session expires, the run stops and a person has to log back in manually — which breaks unattended scheduled runs.
This PR adds the backend foundation for automatic re-login: when a run finds the session expired, the agent signs back in using credentials stored in a Comp-managed 1Password vault (username + password + a live authenticator TOTP code), then continues. From the customer's side, re-login becomes invisible and the scheduler keeps working.
How it works
BrowserCredentialStorageServicewrites a profile's login (username, password, optional authenticator setup key) into a per-organization 1Password vault and stores only theop://reference on the profile. Exposed viaPOST /v1/browserbase/profiles/:id/credentials(integration:update).OnePasswordCredentialVaultAdapter), resolving username/password and a live TOTP code just-in-time during a run.reloginWithStoredCredentialsdrives the login via Stagehandact()with variable substitution, so secret values are injected into the page at execution time and are never sent to the model. It retries once with a freshly generated code to cover the TOTP rotation boundary.Scope
In this PR (backend only): storage + adapter + re-login engine + scheduler change + endpoint + unit tests. Everything is behind the existing feature flag and inert unless
OP_SERVICE_ACCOUNT_TOKENis set (falls back to the current Noop adapter).Intentionally not here:
Notes for reviewers
externalintrigger.config.ts. Bundling in the deployed Trigger.dev worker still needs a live check — flagged rather than assumed.@1password/sdkis pinned to0.4.0(pre-1.0; the API surface shifts between minors).op://references appear at debug level).Tests
onepassword-credential-vault.adapter.spec.ts— resolves username/password/live TOTP; treats a missing TOTP field as absent; returns null when nothing resolves or the provider doesn't match.browser-credential-login.spec.ts— secrets are passed asactvariables and never appear in the prompt text; success path returns to the target URL; retries once with a fresh code; gives up cleanly when sign-in can't complete.All 91 tests in
apps/api/src/browserbasepass; the touched files type-check clean.Summary by cubic
Adds credential-backed auto re-login for browser automations using 1Password so runs recover from expired sessions without human steps. Secrets are resolved at runtime and injected via Stagehand variables; they are never logged or sent to models.
New Features
BrowserCredentialStorageServicesaves a profile’s login in a per-org vault; profiles store only anop://reference.OnePasswordCredentialVaultAdapterresolves username/password and a live TOTP code at run time.POST /v1/browserbase/profiles/:profileId/credentialssaves username, password, and optional TOTP seed (requiresintegration:update).needs_reauthprofiles with stored creds; successful runs mark the profile verified; human fallback remains for non-automatable steps.Migration
@1password/sdk@0.4.0(loaded lazily and marked external in Trigger.dev builds).Written for commit df9ebd4. Summary will update on new commits.