diff --git a/.agents/skills/add-block/SKILL.md b/.agents/skills/add-block/SKILL.md index d580061b37c..82cf55267f0 100644 --- a/.agents/skills/add-block/SKILL.md +++ b/.agents/skills/add-block/SKILL.md @@ -177,7 +177,7 @@ When adding or changing an OAuth integration block: `packages/sim-setup/src/capability-config.ts`. The CLI catalog is exhaustively typed and checked against the runtime field list; do not infer secrecy from the field name. 4. If the canonical OAuth service declares `serviceAccountProviderId`, run - `bun run deployment-config:generate`; this regenerates the provider-ID facts in + `bun run generate:deployment-config`; this regenerates the provider-ID facts in `packages/deployment-config/src/service-account-providers.generated.ts`. Never hand-edit that generated map. Add `deploymentRequirement` policy in `packages/deployment-config/src/service-account-metadata.ts` only when the service-account path @@ -615,7 +615,7 @@ tools: { ## Outputs Definition -**IMPORTANT:** Block outputs have a simpler schema than tool outputs. Block outputs do NOT support: +Block outputs have a simpler schema than tool outputs. They do not support: - `optional: true` - This is only for tool outputs - `items` property - This is only for tool outputs with array types @@ -729,7 +729,7 @@ export const ServiceBlock: BlockConfig = { longDescription: 'Full description for documentation...', docsLink: 'https://docs.sim.ai/integrations/service', category: 'tools', - integrationType: IntegrationType.DeveloperTools, + integrationType: IntegrationType.DevOps, bgColor: '#FF6B6B', icon: ServiceIcon, authMode: AuthMode.OAuth, @@ -959,11 +959,11 @@ Verify it with `bun run check:block-successors`. Adding a block on its own needs no **tool metadata** regeneration — a block references existing tool IDs through `tools.access` and does not change any tool's shape. -But if the same change also adds, edits **or removes** a tool, run `bun run tool-metadata:generate` and commit the result, or CI fails on stale artifacts. That matters here because a block's `outputs` are authored to match its tools' outputs, and the UI reads those from the generated metadata, not the executable registry — an unregenerated tool change makes the block's outputs disagree with what the panel renders. See `.agents/skills/tool-registry-boundary/SKILL.md`. +But if the same change also adds, edits **or removes** a tool, run `bun run generate:tool-metadata` and commit the result, or CI fails on stale artifacts. That matters here because a block's `outputs` are authored to match its tools' outputs, and the UI reads those from the generated metadata, not the executable registry — an unregenerated tool change makes the block's outputs disagree with what the panel renders. See `.agents/skills/tool-registry-boundary/SKILL.md`. A visible integration block does require the generated integration catalog and docs to be refreshed: -`bun run tool-metadata:generate` (only when a tool changed), `bun run scripts/generate-docs.ts`, -`bun run deployment-config:generate`, then `bun run check:audits`. Also run +`bun run generate:tool-metadata` (only when a tool changed), `bun run scripts/generate-docs.ts`, +`bun run generate:deployment-config`, then `bun run check:audits`. Also run `bun run apps/sim/scripts/check-block-registry.ts origin/staging` (CI runs it outside `check:audits`). Commit the full generator output. For what each check verifies, see the `validate-integration` skill → Regenerate Derived Artifacts. @@ -986,10 +986,10 @@ Regenerate Derived Artifacts. - [ ] Outputs match tool outputs - [ ] Block + meta registered in registry-maps.ts (`BLOCK_REGISTRY` / `BLOCK_META_REGISTRY`) - [ ] If `sunset.replacedBy` changed: regenerated and committed the block successor map; `bun run check:block-successors` passes -- [ ] If any tool was added, changed or removed alongside the block: ran `bun run tool-metadata:generate` and committed the artifacts +- [ ] If any tool was added, changed or removed alongside the block: ran `bun run generate:tool-metadata` and committed the artifacts - [ ] Ran `bun run scripts/generate-docs.ts`, reviewed the generated diff, and committed the integration catalog changes -- [ ] `bun run integration-catalog:check` passes -- [ ] `bun run docs:check` passes (CI gate — fails on any stale generated docs page) +- [ ] `bun run check:integration-catalog` passes +- [ ] `bun run check:docs` passes (CI gate — fails on any stale generated docs page) - [ ] If icon missing: asked user to provide SVG - [ ] If triggers exist: `triggers` config set, trigger subBlocks spread - [ ] Optional/rarely-used fields set to `mode: 'advanced'` diff --git a/.agents/skills/add-column-type/SKILL.md b/.agents/skills/add-column-type/SKILL.md index 91c27e4083c..d46f1d0ec7d 100644 --- a/.agents/skills/add-column-type/SKILL.md +++ b/.agents/skills/add-column-type/SKILL.md @@ -98,7 +98,7 @@ The three that are easy to get wrong: Add the entry to `COLUMN_TYPE_REGISTRY` in `registry.ts` **and** `COLUMN_TYPE_SERVER_REGISTRY` in `registry.server.ts`. -`COLUMN_TYPES` is declared in `types.ts` (not derived from the registry — the registry is annotated `Record` against it, which is the gate). `constants.ts` re-exports it, so `columnTypeSchema = z.enum(COLUMN_TYPES)` picks your type up with no edit. **Type-specific metadata does not** — see the next step. +`COLUMN_TYPES` is declared in `types.ts` (not derived from the registry — the registry is annotated `Record` against it, which is the gate). `columnTypeSchema` in `lib/api/contracts/tables.ts` is `z.enum(COLUMN_TYPES)`, so it picks your type up with no edit. **Type-specific metadata does not** — see the next step. ## Step 5: Migrations (only if the stored bytes change) @@ -131,6 +131,7 @@ Registering the *type* is compiler-enforced. Registering its *metadata* is not, | `lib/table/types.ts` `ColumnDefinition` | (this one DOES fail — the ownership loop indexes it) | | `column-types/types.ts` `TYPE_SPECIFIC_COLUMN_KEYS` | it is never stripped on conversion, and poisons the target type | | `lib/api/contracts/tables.ts` — the schema slot in all three column schemas, plus `refineColumnOptions` | zod strips it at the boundary; silently never saved | +| `lib/api/contracts/v2/tables.ts` and `lib/table/application/columns.ts` — the same slots for the v2 API and its use cases | the v2 API silently drops it | | `columns/service.ts` `addTableColumn` param type | callers cannot pass it | | A metadata-only update in `lib/table/columns/service.ts` (`updateColumnCurrency` is the model) + a branch in `performUpdateTableColumn` in `lib/table/orchestration/columns.ts` | changing it on an existing column is a silent 200 no-op | | `column-config-sidebar.tsx` | no UI to set it | diff --git a/.agents/skills/add-connector/SKILL.md b/.agents/skills/add-connector/SKILL.md index d856462fb9c..3631f5c3961 100644 --- a/.agents/skills/add-connector/SKILL.md +++ b/.agents/skills/add-connector/SKILL.md @@ -89,7 +89,7 @@ export const {service}ConnectorMeta: ConnectorMeta = { configFields: [ // Rendered dynamically by the add-connector modal UI - // Supports 'short-input', 'dropdown', and 'selector' types — see ConfigField Types below + // Supports 'short-input', 'dropdown', and 'selector' types — see ConnectorConfigField Types below ], // Optional: tag definitions are metadata too — declare them here @@ -167,7 +167,7 @@ export const {service}Connector: ConnectorConfig = { } ``` -## ConfigField Types +## ConnectorConfigField Types The add-connector modal renders these automatically — no custom UI needed. diff --git a/.agents/skills/add-enrichment/SKILL.md b/.agents/skills/add-enrichment/SKILL.md index 32b1bad4417..fcb83525ab9 100644 --- a/.agents/skills/add-enrichment/SKILL.md +++ b/.agents/skills/add-enrichment/SKILL.md @@ -37,7 +37,7 @@ For each output the enrichment produces, decide which existing tool provides it. - Its `params` accept what you can derive from table columns (read the tool's `params`). - Its `outputs` / `transformResponse` actually expose the field you need (read the real output shape — don't assume). -Order providers **cheapest / most-likely-to-hit first**; the cascade stops at the first non-empty result. Apollo / LinkedIn are not hosted-safe (ToS) — don't use them. +Order providers **cheapest / most-likely-to-hit first**; the cascade stops at the first non-empty result. Apollo and LinkedIn APIs are not hosted-safe (ToS) — never call them as providers. ## Step 2: Verify hosted-key support — chain to `/add-hosted-key` if missing @@ -109,7 +109,7 @@ export { myEnrichment } from './my-enrichment' ``` Rules: -- Keep the file **client-safe**: import only `@sim/emcn/icons`, `@sim/utils/*`, `@/enrichments/providers`, and the types. **Never import `@/tools`** here — the runner does the tool call. +- Keep the file **client-safe**: import only `@sim/emcn/icons`, `@sim/utils/*`, `@/enrichments/providers`, `@/enrichments/provider-failures/*`, and the types. **Never import `@/tools`** here — the runner does the tool call. - `buildParams` returns `null` when inputs are insufficient (provider skipped). `mapOutput` returns `null`/empty for a miss (falls through). Use `filterUndefined` when assembling optional tool params; coerce numbers explicitly (don't pass `''` to number outputs). - Output `id`s are the keys `mapOutput` returns; output `name`s are the default column names (the user can rename them in the config). diff --git a/.agents/skills/add-hosted-key/SKILL.md b/.agents/skills/add-hosted-key/SKILL.md index cf265339d2a..773e9e2fa76 100644 --- a/.agents/skills/add-hosted-key/SKILL.md +++ b/.agents/skills/add-hosted-key/SKILL.md @@ -154,6 +154,20 @@ pricing: { **`getCost` must always throw** if it cannot determine cost. Never silently fall back to a default — this would hide billing inaccuracies. +**When the provider charges a flat price per call** — use `per_request` instead of `getCost` (as `tools/brandfetch/get_brand.ts` does): + +```typescript +pricing: { + type: 'per_request', + // $0.04 per call — from https://example.com/pricing + cost: 0.04, +}, +``` + +### Hosted Keys for Some Parameter Combinations + +When only some calls can use the hosted key (for example, one provider of several), gate the config with `enabled: hostedKeyEnabledWhen({ field: 'provider', operator: 'equals', value: 'falai' })` from `@/tools/hosting` (`operator: 'one_of'` takes `values`); `tools/image/generate.ts` is the reference. + ### Capturing Cost Data from the API If the API returns cost info, capture it in `transformResponse` so `getCost` can read it from the output: diff --git a/.agents/skills/add-integration/SKILL.md b/.agents/skills/add-integration/SKILL.md index 74289debc60..94ab90181b0 100644 --- a/.agents/skills/add-integration/SKILL.md +++ b/.agents/skills/add-integration/SKILL.md @@ -26,11 +26,79 @@ Before writing any code: 1. Use Context7 to find official documentation: `mcp__context7__resolve-library-id`, then fetch with `mcp__context7__query-docs` 2. Or use WebFetch to read API docs directly 3. Identify: - - Authentication method (OAuth, API Key, both) + - Supported authentication grants, who owns the app, and which permissions each operation needs - Available operations (CRUD, search, etc.) - Required vs optional parameters - Response structures +### Choose the connection flow before building tools + +Read the provider's current authentication documentation first. “OAuth” describes a protocol; +it does not imply a browser redirect or a deployment-wide client ID and secret. Compare the +supported paths and choose the simplest supported setup for the intended user: + +| Provider method | Sim connection pattern | Verify in the provider docs | +| --- | --- | --- | +| Authorization code | Shared OAuth app and consent flow | Partner approval, redirect URIs, tenant consent, scopes, refresh-token rotation | +| Customer-owned client credentials | Saved `service_account` credential with a client ID and secret | Internal-app eligibility, grant activation, scope syntax, token lifetime and revocation behavior | +| API token or personal access token | Saved token service account when reusable connections are useful | Token permissions, identity verification, expiry and rotation | +| Private key, certificate, or service-account JSON | Existing key-based service-account framework | Signing algorithm, audience, subject, tenant binding and key rotation | + +Do not present customer-owned credentials as a way around a provider's approval requirements +for a shared public integration. Explain the supported account/app type in the setup docs. Keep +existing OAuth connections usable when other users depend on them, unless a migration or removal is explicitly authorized. + +For client credentials, extend the existing descriptors and minter registry under +`apps/sim/lib/credentials/client-credential-accounts/`; for token credentials, use +`token-service-accounts/`. Reuse the connect modal, encrypted storage, authorized credential +operations, and execution-time token resolver. Do not collect reusable secrets on every block, +invent a credential route, or store a short-lived access token as if it were permanent. + +Verify the complete lifecycle, including connect-time verification, concurrent executions on +different workers, expiry, secret rotation, and permission changes. Some providers invalidate the +previous token whenever another is minted. Those providers require shared coordination keyed by +the provider application identity, including across duplicate saved credentials; a process-local +or per-scope token cache is insufficient. A cache hit must never authenticate a wrong secret or +silently grant broader permissions. Bound token responses and retries, prevent credential-bearing +redirects, and never include provider response bodies or secrets in errors. + +Use explicit permission choices when the supported operations have different access needs. Keep +region and permission selections on reconnect unless the user changes them. Verify documented +identity endpoints rather than guessing which person a client-credentials token represents. + +### Pair saved credentials with block and resource selectors + +`oauth-input` is the shared saved-credential picker, including for service accounts that never +redirect to OAuth. Give its basic/advanced pair one `canonicalParamId: 'oauthCredential'`, with +the correct `serviceId`, and wire tool OAuth metadata to that same service. Tokens and trusted +API origins are hidden execution inputs; the model receives a credential ID, never a secret. +Use `credentialKind: 'service-account'` when only service accounts are supported, including in +tool OAuth metadata. Use `credentialKind: 'any'` on the picker when both browser OAuth and saved +service accounts are supported; omitting it defaults the connect action to browser OAuth. Per-connection scope +choices belong in the descriptor and encrypted credential, not an all-permissions block +`requiredScopes` array that would reject read-only connections. + +When a documented list endpoint makes a resource ID discoverable, pair the saved credential +with a dynamic resource selector using the `add-selector` and `validate-selector` skills: + +- Declare the credential subblock and any parent resource in `dependsOn`, and give the resource + selector and its manual advanced input the same canonical parameter. +- Register browser-safe selector metadata and a server attachment through the shared selector + framework. Resolve the credential with the expected provider binding and use a fixed or + credential-bound destination; selectors and execution must use the same auth/region policy. +- Exercise switching credentials, parent resources, pagination, expired tokens, denied access, + and the advanced environment-reference path. Check that stale choices cannot survive a change + of account. Do not fetch provider data or mint tokens in the browser. +- Keep a manual ID path when the provider cannot enumerate a resource. Do not invent an endpoint + solely to provide a dropdown. + +For an existing integration, inspect persisted workflow serialization as well as the visible +form before removing old auth fields. Establish whether existing users need a migration or +compatibility path; do not add permanent legacy branches speculatively when removal is authorized. +When compatibility is needed, prove that old values survive serialization and execution; hiding +a field is not proof. If the user authorizes a production usage check, query only the aggregate usage/auth-shape evidence +needed and keep identities, secrets, and local evidence out of commits and PRs. + ### Hard Rule: No Guessed Response Schemas If the official docs do not clearly show the response JSON shape for an endpoint, you MUST stop and tell the user exactly which outputs are unknown. @@ -225,7 +293,7 @@ export const tools: Record = { Then regenerate the generated tool metadata and commit it: ```bash -bun run tool-metadata:generate +bun run generate:tool-metadata ``` Client code reads `params`/`outputs` from these artifacts rather than importing @@ -274,38 +342,52 @@ export const TRIGGER_REGISTRY: TriggerRegistry = { ## Step 7: Configure Deployment Availability -Do this for every visible OAuth integration. API-key and unauthenticated integrations do not need -an OAuth client capability. +Do this for every integration that uses the shared credential picker. Only a connection that +depends on deployment-wide OAuth client fields needs an OAuth client capability; a customer-owned +service account must remain available without those fields. The block's `oauth-input.serviceId` is the canonical link between the generated integration catalog, the OAuth service configuration, deployment availability, and the setup CLI. -1. Ensure the block has exactly one distinct OAuth `serviceId` and that it matches the canonical - service entry in `apps/sim/lib/oauth/oauth.ts`. -2. Confirm `resolveOAuthClientCapabilityId(serviceId)` resolves to the intended provider entry in +1. Set `authMode: AuthMode.OAuth` for integrations using browser OAuth or customer-owned OAuth + client credentials. Token-only service accounts can retain `AuthMode.ApiKey` with the shared + picker, as Coda does; `oauth-input` alone does not determine the authentication protocol. + Register a new token-only service ID and block type in `tokenCredentialIntegrationTypes` in + `packages/deployment-config/src/integration-availability.ts` so availability and integration + policy recognize the saved credential path. + For OAuth integrations, this value lets the catalog discover the connection flow instead of + routing "Add to Sim" to chat. Ensure the block has exactly one distinct OAuth `serviceId` + matching the canonical service in `apps/sim/lib/oauth/oauth.ts`. The canonical service's + `authType` selects browser OAuth or the service-account modal. Verify the resulting catalog + and block connection actions for the chosen authentication method. +2. For browser OAuth, confirm `resolveOAuthClientCapabilityId(serviceId)` resolves to the intended provider entry in `OAUTH_CLIENT_CAPABILITIES` in `packages/deployment-config/src/env-capabilities.ts`. Google and Microsoft service IDs deliberately share provider-level capabilities. -3. For a new OAuth provider, add the required client fields to `OAUTH_CLIENT_CAPABILITIES`, add +3. For a new browser OAuth provider, add the required client fields to `OAUTH_CLIENT_CAPABILITIES`, add every referenced field to the env schema in `apps/sim/lib/core/config/env.ts`, and add the matching `text` or `secret` entries to `OAUTH_CLIENT_SETUP_FIELDS` in `packages/sim-setup/src/capability-config.ts`. Do not create integration-specific setup logic or infer secret fields from naming; the CLI mapping is exhaustively checked against the runtime fields. 4. If the canonical OAuth service has `serviceAccountProviderId`, run - `bun run deployment-config:generate` to refresh + `bun run generate:deployment-config` to refresh `packages/deployment-config/src/service-account-providers.generated.ts`; never hand-edit the generated provider-ID map. In `packages/deployment-config/src/service-account-metadata.ts`, use: - no `deploymentRequirement` when the service-account path works independently of OAuth client fields; - `'oauth-client'` when it requires the same deployment OAuth client fields; - `'preview-gated'` when availability is controlled by the service-account preview block. + For a service-account-only default, set the canonical service's `authType: 'service_account'` + and `serviceAccountProviderId`. Verify both block availability and the connect modal with no + deployment OAuth credentials configured. An existing browser OAuth path may remain for legacy + credentials without becoming a prerequisite for the new path. -Never add a permissive fallback for missing capability metadata. A visible OAuth integration without -a resolvable capability must fail validation. +Never add a permissive fallback for missing capability metadata. A browser OAuth connection without +a resolvable capability must fail validation; an independent service account uses its own metadata. ## Step 8: Generate and Validate the Catalog -Run `bun run tool-metadata:generate`, `bun run scripts/generate-docs.ts`, -`bun run deployment-config:generate`, then `bun run check:audits` (see the `validate-integration` +Run `bun run generate:tool-metadata`, `bun run scripts/generate-docs.ts`, +`bun run generate:deployment-config`, then `bun run check:audits` (see the `validate-integration` skill → Regenerate Derived Artifacts for the full list and what each check verifies). The docs generator creates `apps/docs/content/docs/integrations/{service}.mdx` — one page per service carrying the block's Actions and, if it has one, its Triggers section. Never hand-edit generated pages; the only editable region is the `{/* MANUAL-CONTENT */}` block (see `scripts/README.md`). @@ -375,7 +457,7 @@ If creating V2 versions (API-aligned outputs): - [ ] All optional outputs have `optional: true` - [ ] Created `index.ts` barrel export - [ ] Registered all tools in `tools/registry.ts` -- [ ] Ran `bun run tool-metadata:generate` and committed the regenerated artifacts +- [ ] Ran `bun run generate:tool-metadata` and committed the regenerated artifacts - [ ] Classified every model-visible, opaque, Sim-durable, and internal-execution request field - [ ] Added shared model-input projection or private provenance only where required; ordinary external resource locators and control inputs retain their request semantics @@ -389,7 +471,8 @@ If creating V2 versions (API-aligned outputs): - [ ] Set `integrationType` to the correct `IntegrationType` enum value - [ ] `{Service}BlockMeta.tags` lists every applicable `IntegrationTag` (tags live on the meta, not the block) - [ ] Defined operation dropdown with all operations -- [ ] Added credential field with `requiredScopes: getScopesForService('{service}')` +- [ ] Added the saved-credential picker with the supported `credentialKind`; browser OAuth scopes + use `getScopesForService('{service}')`, while variable service-account permissions stay on the credential - [ ] Added conditional fields per operation - [ ] Every `short-input`, `long-input`, `code`, and selector subBlock has a `placeholder` - [ ] Set up dependsOn for cascading selectors @@ -407,15 +490,25 @@ If creating V2 versions (API-aligned outputs): - [ ] `canvasPresentation.sentences` covers every operation; `bun run apps/sim/scripts/check-canvas-sentences.ts --block={service}` passes - [ ] `{Service}BlockMeta` also sets `url` (verified external homepage) and `skills` (grounded in `tools.access`, sourced from real use cases) — see add-block → BlockMeta -### OAuth Scopes (if OAuth service) +### Authentication and saved credentials +- [ ] Compared documented authorization code, client credentials, token, and key-based methods +- [ ] Chosen app ownership and approval requirements match the intended user +- [ ] Saved credential picker, tools, and resource selectors share the same provider/region binding +- [ ] Connect verification, expiry, concurrent workers, secret rotation, and scope changes are sound +- [ ] Existing usage and the migration/removal decision are established; any required compatibility is verified through serialization and execution +- [ ] New connection and reconnect flows verified in the running UI + +### Browser OAuth Scopes (if authorization-code flow is supported) - [ ] Defined scopes in `lib/oauth/oauth.ts` under `OAUTH_PROVIDERS` - [ ] Added scope descriptions in `SCOPE_DESCRIPTIONS` within `lib/oauth/utils.ts` - [ ] Used `getCanonicalScopesForProvider()` in `lib/auth/connectors/providers.ts` (never hardcode) -- [ ] Used `getScopesForService()` in block `requiredScopes` (never hardcode) +- [ ] Used `getScopesForService()` for the browser OAuth permissions the block needs (never hardcode) +- [ ] A picker that also accepts service accounts does not require broader scopes than every supported + connection needs; per-connection service-account permissions are validated by the descriptor/minter -### Deployment Availability (if OAuth service) +### Deployment Availability (if using the saved-credential picker) - [ ] Block declares exactly one distinct `oauth-input.serviceId` -- [ ] `resolveOAuthClientCapabilityId(serviceId)` resolves to the intended `OAUTH_CLIENT_CAPABILITIES` entry +- [ ] Browser OAuth resolves to the intended `OAUTH_CLIENT_CAPABILITIES` entry; independent service accounts work without deployment OAuth fields - [ ] Every new OAuth capability field exists in `apps/sim/lib/core/config/env.ts` - [ ] Runtime OAuth fields live in `OAUTH_CLIENT_CAPABILITIES`; matching CLI input modes live in the exhaustively checked `OAUTH_CLIENT_SETUP_FIELDS` - [ ] If `serviceAccountProviderId` is configured, `SERVICE_ACCOUNT_METADATA_BY_OAUTH_SERVICE_ID` has the matching projection and deployment requirement @@ -437,15 +530,15 @@ If creating V2 versions (API-aligned outputs): ### Docs and deployment metadata - [ ] Ran `bun run scripts/generate-docs.ts` -- [ ] Ran `bun run deployment-config:generate` for OAuth or service-account changes +- [ ] Ran `bun run generate:deployment-config` for OAuth or service-account changes - [ ] Verified docs file created - [ ] Wrote the `{/* MANUAL-CONTENT-START:intro */}` section under `` and confirmed it survives a regenerate - [ ] Reviewed and committed the generated `packages/deployment-config/src/integrations.json` change -- [ ] `bun run integration-catalog:check` passes -- [ ] `bun run docs:check` passes — CI fails on stale generated docs, so commit the full generator +- [ ] `bun run check:integration-catalog` passes +- [ ] `bun run check:docs` passes — CI fails on stale generated docs, so commit the full generator output, including catch-up regeneration for pages another PR left stale (never revert it as "unrelated drift") -- [ ] `bun run deployment-config:check` passes +- [ ] `bun run check:deployment-config` passes ### Final Validation (Required) - [ ] Read every tool file and cross-referenced inputs/outputs against the API docs diff --git a/.agents/skills/add-model/SKILL.md b/.agents/skills/add-model/SKILL.md index 75721721c4b..5e9702dc4a1 100644 --- a/.agents/skills/add-model/SKILL.md +++ b/.agents/skills/add-model/SKILL.md @@ -49,13 +49,13 @@ Use a precise WebFetch prompt: *"Extract for {model_id}: exact model id string, |---|---|---| | `temperature` | All providers (passed through if set) | Safe but inert on always-reasoning models that reject it | | `toolUsageControl` | All providers (provider-level default) | Override per model only when that model differs | -| `forcedToolUse` | `anthropic/core.ts` (anthropic, azure-anthropic, kie); defaults to `toolUsageControl` | Ignored by every other provider; set `false` only on a model behind that core that cannot force tools | +| `forcedToolUse` | `anthropic/core.ts` (anthropic, azure-anthropic, kie) defaults it to `toolUsageControl` and reads `thinking.forcedToolUse` for forcing while thinking; `openai/core.ts` and `bedrock/index.ts` treat only an explicit `false` as "cannot force" | Ignored by every other provider; set `false` only on a model that cannot force tools | | `promptCaching` | Caller-placed cache breakpoints | Set only where the vendor charges for opt-in caching (absent for OpenAI/Gemini implicit caching) | | `reasoningEffort` | `openai/core.ts`, `azure-openai`, `xai`, `deepseek`, `groq`, `zai`, `kimi`, `cerebras`, `meta`, `litellm` (each `index.ts`) | Not read by anthropic/gemini (they use `thinking`) or by mistral, openrouter, fireworks, vertex — re-grep before assuming | | `verbosity` | `openai/core.ts`, `azure-openai/index.ts` only | Dead elsewhere | | `thinking` | `anthropic/core.ts`, `gemini/core.ts`; `deepseek`, `groq`, `zai`, `kimi` (each `index.ts`) read the resolved `thinkingLevel` | Dead elsewhere | -| `thinking.streamed` | Docs generator + `getThinkingStreamVisibility` (`models.ts`); `anthropic/core.ts` uses `'summary'` to request `display: 'summarized'` on agent-events runs | **Mandatory on Anthropic-family thinking models** (`agent-stream-docs:check` fails without it); other families fall back to provider defaults | -| `nativeStructuredOutputs` | `anthropic/core.ts`, `bedrock/index.ts` (via `models.ts` `supportsNativeStructuredOutputs`, which reads the flag) | Dead elsewhere — fireworks/baseten/together/openrouter call their own provider-level `supportsNativeStructuredOutputs` that ignores the model flag (always on, always off, or OpenRouter API metadata) | +| `thinking.streamed` | Docs generator + `getThinkingStreamVisibility` (`models.ts`); `anthropic/core.ts` uses `'summary'` to request `display: 'summarized'` on agent-events runs | **Mandatory on Anthropic-family thinking models** (`check:agent-stream-docs` fails without it); other families fall back to provider defaults | +| `nativeStructuredOutputs` | `anthropic/core.ts`, `bedrock/index.ts` (via `models.ts` `supportsNativeStructuredOutputs`, which reads the flag), `nebius/index.ts`, `nvidia/index.ts` (via `getModelCapabilities`) | Dead elsewhere — fireworks/baseten/together/openrouter call their own provider-level `supportsNativeStructuredOutputs` that ignores the model flag (always on, always off, or OpenRouter API metadata) | | `maxOutputTokens` | Read by UI + executor for token estimation | Always meaningful — set if provider documents a cap | | `computerUse` | `providers/utils.ts` (`getComputerUseModels` → `computerUseModels` routing) | Set only on actual computer-use SKUs | | `deepResearch` | UI flag for routing to deep-research SKUs | Set only on actual deep-research model IDs | @@ -143,9 +143,9 @@ The Consumption Matrix (Step 2) tells you which capability *flags* are honored b If the entry has `capabilities.thinking` or `capabilities.reasoningEffort`, it appears in the autogenerated "Streamed thinking and tool calls" table on the Agent block docs page: -- **Anthropic-family (`anthropic`, `azure-anthropic`) thinking models MUST declare `capabilities.thinking.streamed`** (`'full' | 'summary' | 'none'`). Verify against Anthropic's current thinking-display and streaming docs: visible thinking returned by the API is summarized, including when Sim opts models whose default display is `omitted` into `display: 'summarized'` on agent-events runs, so current Claude thinking models use `'summary'`. Use `'full'` only if future official API docs explicitly guarantee raw thinking deltas. `bun run agent-stream-docs:check` (CI) fails if the field is missing. +- **Anthropic-family (`anthropic`, `azure-anthropic`) thinking models MUST declare `capabilities.thinking.streamed`** (`'full' | 'summary' | 'none'`). Verify against Anthropic's current thinking-display and streaming docs: visible thinking returned by the API is summarized, including when Sim opts models whose default display is `omitted` into `display: 'summarized'` on agent-events runs, so current Claude thinking models use `'summary'`. Use `'full'` only if future official API docs explicitly guarantee raw thinking deltas. `bun run check:agent-stream-docs` (CI) fails if the field is missing. - Other families usually omit the field and inherit the provider default in `getThinkingStreamVisibility` (Gemini/OpenAI → summaries; Bedrock/Meta → none; OpenAI-compatible vendors with documented reasoning fields → full deltas). Set it explicitly only when the model deviates from its family. -- After inserting the entry, run `bun run agent-stream-docs:generate` and commit the regenerated `apps/docs/content/docs/workflows/blocks/agent.mdx` — CI diffs it. +- After inserting the entry, run `bun run generate:agent-stream-docs` and commit the regenerated `apps/docs/content/docs/workflows/blocks/agent.mdx` — CI diffs it. - Include the `streamed` value (with its source URL) in the verification report when set. ### Wrong family entirely? @@ -157,7 +157,7 @@ If the entry has `capabilities.thinking` or `capabilities.reasoningEffort`, it a ```bash bun run lint -bun run agent-stream-docs:generate # only when the entry has thinking/reasoningEffort +bun run generate:agent-stream-docs # only when the entry has thinking/reasoningEffort ``` Lint must pass before you report done — fix the entry you wrote, never delete it to make lint pass. diff --git a/.agents/skills/add-permission-group-item/SKILL.md b/.agents/skills/add-permission-group-item/SKILL.md index 68ae7fba2a3..5bcd08ff94c 100644 --- a/.agents/skills/add-permission-group-item/SKILL.md +++ b/.agents/skills/add-permission-group-item/SKILL.md @@ -45,7 +45,7 @@ Allowlist when the safe posture is "only what the admin named" and the member se **Is the decision knowable from the config alone?** A rule needing a request value (an auth mode, a connector id) is *parameterized* and cannot be declared on an operation — see Step 3. -**Is it a gate or a projection?** A key that withholds *fields from a response* rather than the response is a projection. `hideTraceSpans` and `hideCostInfo` work this way: the logs routes declare `capability: 'none'` and strip fields, because refusing the read would withhold the status and error message too. Projections have one owner — `lib/logs/log-projection.ts` (`resolveLogFieldProjection`, `projectExecutionData`, `projectCostTotal`), carrying the `permission-group-enforced:` annotations. Add yours there; two copies of a redaction rule is how one of them stops redacting. Corollary: refuse the query that *selects on* a withheld field — otherwise the projection is a filter oracle; `logQuerySelectsCost` / `assertLogCostQueryAllowed` in that same module are the shape. +**Is it a gate or a projection?** A key that withholds *fields from a response* rather than the response is a projection. `hideTraceSpans` and `hideCostInfo` work this way: the logs routes declare `capability: 'none'` and strip fields, because refusing the read would withhold the status and error message too. Projections have one owner — `lib/logs/projection.ts` (`resolveLogFieldProjection`, `projectExecutionData`, `projectCostTotal`), carrying the `permission-group-enforced:` annotations. Add yours there; two copies of a redaction rule is how one of them stops redacting. Corollary: refuse the query that *selects on* a withheld field — otherwise the projection is a filter oracle; `logQuerySelectsCost` / `assertLogCostQueryAllowed` in that same module are the shape. ## Step 1: Append the field entry — never insert @@ -255,7 +255,7 @@ What a run *does* is still governed by `assertPermissionsAllowed`. An item that ## Checklist Before Finishing - [ ] Kind and `enforcement` chosen deliberately; `ui-only` justified in writing if used -- [ ] It is a gate, not a projection — a projection belongs in `lib/logs/log-projection.ts` with `capability: 'none'` on the routes, and still refuses queries that select on the withheld field +- [ ] It is a gate, not a projection — a projection belongs in `lib/logs/projection.ts` with `capability: 'none'` on the routes, and still refuses queries that select on the withheld field - [ ] Entry **appended** to `PERMISSION_GROUP_FIELDS`, permissive default, restriction-phrased name - [ ] Category present in `PLATFORM_CATEGORY_ORDER`, named after what is withheld - [ ] `hint` says what access is revoked, never "hide" — it is also the active-restriction prose diff --git a/.agents/skills/add-tools/SKILL.md b/.agents/skills/add-tools/SKILL.md index d54af50dd0f..e9d5bb839af 100644 --- a/.agents/skills/add-tools/SKILL.md +++ b/.agents/skills/add-tools/SKILL.md @@ -67,15 +67,12 @@ case with trusted execution context; use the `migrate-application-operation` ski Use this structure only for an absolute external provider API: ```typescript -import type { {ServiceName}{Action}Params } from '@/tools/{service}/types' +import type { + {ServiceName}{Action}Params, + {ServiceName}{Action}Response, +} from '@/tools/{service}/types' import type { ToolConfig } from '@/tools/types' - -interface {ServiceName}{Action}Response { - success: boolean - output: { - // Define output structure here - } -} +import { safeUrlPathSegment } from '@/tools/url-path' export const {serviceName}{Action}Tool: ToolConfig< {ServiceName}{Action}Params, @@ -117,7 +114,8 @@ export const {serviceName}{Action}Tool: ToolConfig< }, request: { - url: (params) => `https://api.service.com/v1/resource/${params.id}`, + url: (params) => + `https://api.service.com/v1/resource/${safeUrlPathSegment(params.someId, 'someId')}`, method: 'POST', headers: (params) => ({ Authorization: `Bearer ${params.accessToken}`, @@ -190,6 +188,8 @@ fallback, or caller-controlled `_context` authority. A required `'hidden'` param needs an `oauth` declaration or `hosting.apiKeyParam` to supply it (`bun run check:tool-param-reachability`). +A declared `timeout` param is an ordinary tool input — put it in the request body or URL yourself if the provider expects it; it becomes Sim's millisecond request deadline only when the tool sets `timeoutParamIsDeadline: true` (e.g. `http_request`). A `method` param on a tool with a fixed `request.method` would be sent as the HTTP verb, so the same audit rejects it. + ### Parameter Types - `'string'` - Text values - `'number'` - Numeric values @@ -360,7 +360,7 @@ Only use bare `type: 'json'` without `properties` when the shape is truly dynami ## Critical Rules for transformResponse ### Handle Nullable Fields -ALWAYS use `?? null` for fields that may be undefined: +Use `?? null` for fields that may be undefined: ```typescript transformResponse: async (response: Response) => { const data = await response.json() @@ -453,7 +453,7 @@ export const tools = { 3. Regenerate the tool metadata artifacts: ```bash -bun run tool-metadata:generate +bun run generate:tool-metadata ``` Client code reads a tool's `params`/`outputs` from generated metadata rather than @@ -463,7 +463,7 @@ these are regenerated — and CI fails on stale artifacts. Commit the result. Se ## Wiring Tools into the Block (Required) -After registering in `tools/registry.ts`, you MUST also update the block definition at `apps/sim/blocks/blocks/{service}.ts`. This is not optional — tools are only usable from the UI if they are wired into the block. +After registering in `tools/registry.ts`, also update the block definition at `apps/sim/blocks/blocks/{service}.ts`: a tool is usable from the UI only once the block wires it. ### 1. Add to `tools.access` @@ -612,10 +612,10 @@ If creating V2 tools (API-aligned outputs), use `_v2` suffix: - [ ] Types file has all interfaces - [ ] Index.ts exports all tools and re-exports types (`export * from './types'`) - [ ] Tools registered in `tools/registry.ts` -- [ ] `bun run tool-metadata:generate` run and the regenerated artifacts committed +- [ ] `bun run generate:tool-metadata` run and the regenerated artifacts committed - [ ] `bun run scripts/generate-docs.ts` run and the refreshed docs committed — the integration's docs page is rendered from each tool's description, params, and outputs, and CI's - `bun run docs:check` fails on stale pages + `bun run check:docs` fails on stale pages - [ ] Block wired: `tools.access`, dropdown options, subBlocks, `tools.config`, outputs, inputs - [ ] Model, durable-storage, and internal-execution boundaries use the shared provenance mechanisms only where a concrete Sim `{{...}}` resolution path requires them diff --git a/.agents/skills/add-trigger/SKILL.md b/.agents/skills/add-trigger/SKILL.md index 69f3657649a..d3ac4406dba 100644 --- a/.agents/skills/add-trigger/SKILL.md +++ b/.agents/skills/add-trigger/SKILL.md @@ -216,9 +216,9 @@ If none apply, you don't need a handler. The default handler provides bearer tok ### Example Handler ```typescript -import crypto from 'crypto' import { createLogger } from '@sim/logger' import { safeCompare } from '@sim/security/compare' +import { hmacSha256Hex } from '@sim/security/hmac' import type { EventMatchContext, FormatInputContext, FormatInputResult, WebhookProviderHandler } from '@/lib/webhooks/providers/types' import { createHmacVerifier } from '@/lib/webhooks/providers/utils' @@ -226,8 +226,7 @@ const logger = createLogger('WebhookProvider:{Service}') function validate{Service}Signature(secret: string, signature: string, body: string): boolean { if (!secret || !signature || !body) return false - const computed = crypto.createHmac('sha256', secret).update(body, 'utf8').digest('hex') - return safeCompare(computed, signature) + return safeCompare(hmacSha256Hex(body, secret), signature) } export const {service}Handler: WebhookProviderHandler = { @@ -299,6 +298,7 @@ If they differ: the tag dropdown shows fields that don't exist, or actual data h If the service API supports programmatic webhook creation, implement `createSubscription` and `deleteSubscription` on the handler. The orchestration layer calls these automatically — **no code touches `route.ts`, `provider-subscriptions.ts`, or `deploy.ts`**. ```typescript +import { readResponseJsonWithLimit } from '@/lib/core/utils/stream-limits' import { getNotificationUrl, getProviderConfig } from '@/lib/webhooks/provider-subscription-utils' import type { DeleteSubscriptionContext, SubscriptionContext, SubscriptionResult } from '@/lib/webhooks/providers/types' @@ -315,7 +315,10 @@ export const {service}Handler: WebhookProviderHandler = { }) if (!res.ok) throw new Error(`{Service} error: ${res.status}`) - const { id } = (await res.json()) as { id: string } + const { id } = await readResponseJsonWithLimit<{ id: string }>(res, { + maxBytes: 1024 * 1024, + label: '{Service} webhook creation response', + }) return { providerConfigUpdates: { externalId: id } } }, @@ -342,6 +345,7 @@ export const {service}Handler: WebhookProviderHandler = { Trigger outputs use the same schema as block outputs (NOT tool outputs). **Supported:** `type` + `description` for leaf fields, nested objects for complex data. +**Also supported:** `nullable: true` and `condition` (`TriggerOutput` in `triggers/types.ts`). **NOT supported:** `optional: true`, `items` (those are tool-output-only features). ```typescript @@ -377,7 +381,7 @@ apps/sim/lib/webhooks/polling/ ```typescript import { pollingIdempotency } from '@/lib/core/idempotency/service' -import type { PollingProviderHandler, PollWebhookContext } from '@/lib/webhooks/polling/types' +import type { PollingProviderHandler, PollOutcome, PollWebhookContext } from '@/lib/webhooks/polling/types' import { markWebhookFailed, markWebhookSuccess, resolveOAuthCredential, updateWebhookProviderConfig } from '@/lib/webhooks/polling/utils' import { processPolledWebhookEvent } from '@/lib/webhooks/processor' @@ -385,7 +389,7 @@ export const {service}PollingHandler: PollingProviderHandler = { provider: '{service}', label: '{Service}', - async pollWebhook(ctx: PollWebhookContext): Promise<'success' | 'failure'> { + async pollWebhook(ctx: PollWebhookContext): Promise { const { webhookData, workflowData, requestId, logger } = ctx const webhookId = webhookData.id @@ -539,5 +543,5 @@ registered `InternalToolConfig.operation`; a `directExecution` property fails - [ ] Manually verify output keys match trigger `outputs` keys - [ ] Trigger UI shows correctly in the block - [ ] Ran `bun run scripts/generate-docs.ts` and committed the refreshed pages — trigger sections - render into the owning integration's docs page, and CI's `bun run docs:check` fails on stale + render into the owning integration's docs page, and CI's `bun run check:docs` fails on stale pages diff --git a/.agents/skills/council/SKILL.md b/.agents/skills/council/SKILL.md index 698121ff012..f1e05979990 100644 --- a/.agents/skills/council/SKILL.md +++ b/.agents/skills/council/SKILL.md @@ -2,13 +2,6 @@ name: council description: Spawn parallel task agents to explore a given area of the codebase from multiple angles, then use their findings to answer the question or build a plan. Use when a task needs broad fan-out exploration across many files before acting. argument-hint: -# No agents/openai.yaml by design: council is a meta/exploration utility (like cleanup, ship, you-might-not-need-*), not a service-integration builder, so it intentionally ships no standalone agent card. --- -Based on the given area of interest, please: - -1. Dig around the codebase in terms of that given area of interest, gather general information such as keywords and architecture overview. -2. Spawn off n=10 (unless specified otherwise) task agents to dig deeper into the codebase in terms of that given area of interest, some of them should be out of the box for variance. -3. Once the task agents are done, use the information to do what the user wants. - -If user is in plan mode, use the information to create the plan. +Map the area of interest first (keywords, architecture), then fan out parallel agents, each exploring a distinct angle, including a few unconventional ones. Size the fan-out to the area (the user may name a number). Use their findings to answer the question, or to write the plan in plan mode. diff --git a/.agents/skills/db-migrate/SKILL.md b/.agents/skills/db-migrate/SKILL.md index a2c05a96a86..e39dd005cf0 100644 --- a/.agents/skills/db-migrate/SKILL.md +++ b/.agents/skills/db-migrate/SKILL.md @@ -33,7 +33,7 @@ Never put expand and contract in the same PR. If this PR both removes the code t | Drop a column/table | stop all reads/writes in code; ship it | `DROP` (annotate) | | Change a column type | add a new column of the new type; dual-write | backfill, swap reads, drop old | | Add FK / CHECK | `ADD CONSTRAINT ... NOT VALID` | `VALIDATE CONSTRAINT` separately | -| Index an existing table | `COMMIT;` breakpoint → `SET lock_timeout = 0` → `CREATE INDEX CONCURRENTLY IF NOT EXISTS` (see `packages/db/scripts/migrate.ts`) | — | +| Index an existing table | `COMMIT;` breakpoint → `SET lock_timeout = 0` → `CREATE INDEX CONCURRENTLY IF NOT EXISTS` → `SET lock_timeout = '5s'` (see `packages/db/scripts/migrate.ts`) | — | | Drop an index | `COMMIT;` breakpoint → `DROP INDEX CONCURRENTLY IF EXISTS` — plain `DROP INDEX` takes ACCESS EXCLUSIVE on the table | — | | Backfill data | batched + idempotent `UPDATE` (keyset/`WHERE`, bounded) | — | diff --git a/.agents/skills/design-taste-frontend/SKILL.md b/.agents/skills/design-taste-frontend/SKILL.md index 428c7adf9c9..303a2ad1af0 100644 --- a/.agents/skills/design-taste-frontend/SKILL.md +++ b/.agents/skills/design-taste-frontend/SKILL.md @@ -4,7 +4,7 @@ source: https://github.com/leonxlnx/taste-skill — skills/taste-skill/SKILL.md description: Anti-slop frontend skill for landing pages, portfolios, and redesigns. The agent reads the brief, infers the right design direction, and ships interfaces that do not look templated. Real design systems when applicable, audit-first on redesigns, strict pre-flight check. --- -> **In this repo:** Tailwind 4 (CSS-first config in `apps/sim/app/_styles/globals.css`); animation via `import { motion } from 'framer-motion'` (not `motion/react`); icons from `@sim/emcn/icons`; colors through the CSS-variable tokens in `.claude/rules/sim-styling.md` (no hardcoded `text-gray-*`/hex/`zinc` utilities, no paired `dark:` utilities). This note overrides any conflicting guidance or code sample anywhere in this file. Fonts are fixed (Season body, Inter); never introduce new families, and never use Martian Mono on landing (`apps/sim/app/(landing)/CLAUDE.md`). Font weight is only `font-normal`/`font-medium`/`font-semibold`. Elevation uses the `shadow-subtle|medium|overlay|card` tokens. Type size uses named tokens, never `text-[Npx]`. Every labeled field inside a `ChipModalBody` is a `ChipModalField`. Use `bunx`, never `npx`. Do not add GSAP, Lenis, Three, or shadcn. Landing copy and SEO follow `.claude/rules/constitution.md` and `.claude/rules/landing-seo-geo.md`. +> **In this repo:** Tailwind 4 (CSS-first config in `apps/sim/app/_styles/globals.css`); animation via `import { motion } from 'framer-motion'` (not `motion/react`); icons from `@sim/emcn/icons`; colors through the CSS-variable tokens in `.claude/rules/sim-styling.md` (no hardcoded `text-gray-*`/hex/`zinc` utilities, no paired `dark:` utilities). This note overrides any conflicting guidance or code sample anywhere in this file. Fonts are fixed (Season body, Inter); never introduce new families, and never use Martian Mono on landing (`apps/sim/app/(landing)/CLAUDE.md`). Font weight is only `font-normal`/`font-medium`/`font-semibold`. Elevation uses the `shadow-subtle|medium|overlay|card` tokens. Type size uses named tokens, never `text-[Npx]`. Every labeled field inside a `ChipModalBody` is a `ChipModalField`. Use `bunx`, never `npx`. Do not add GSAP, Lenis, Three, or shadcn. On landing pages, motion is CSS first; an animation library stays out of the initial bundle and loads below the fold through `next/dynamic` (`apps/sim/app/(landing)/CLAUDE.md`). Landing copy and SEO follow `.claude/rules/constitution.md` and `.claude/rules/landing-seo-geo.md`. # tasteskill: Anti-Slop Frontend Skill diff --git a/.agents/skills/emcn-design-review/SKILL.md b/.agents/skills/emcn-design-review/SKILL.md index fef7bf5a1c3..8a211f53c6b 100644 --- a/.agents/skills/emcn-design-review/SKILL.md +++ b/.agents/skills/emcn-design-review/SKILL.md @@ -7,7 +7,7 @@ argument-hint: "[scope] [fix=true|false]" # EMCN Design Review Arguments: -- scope: what to review (default: your current changes). Examples: "diff to main", "PR #123", "src/components/", "whole codebase" +- scope: what to review (default: your current changes). Examples: "diff to staging", "PR #123", "src/components/", "whole codebase" - fix: whether to apply fixes (default: true). Set to false to only propose changes. User arguments: $ARGUMENTS @@ -44,7 +44,7 @@ Use CSS variable pattern (`text-[var(--text-body)]`), never Tailwind semantics ( ## Buttons and chips -Header/action chrome is `Chip`/`ChipLink` (variants `primary`, `destructive`, `outline`, `border`, `border-shadow`, bare). Selection and toggles use the `active` prop, never a variant. A single-resource Delete is a plain chip behind `ChipConfirmModal`; `destructive` is only for at-scale actions (`.claude/rules/sim-settings-pages.md` "Deleting a resource"). `Button` is only for icon-only toolbar controls (`ghost`/`quiet`, `size='icon'`). +Header/action chrome is `Chip`/`ChipLink` (variants `primary`, `destructive`, `outline`, `border`, `border-shadow`; omit `variant` for the bare chip, and `filled` is reserved for chip fields and triggers). Selection and toggles use the `active` prop, never a variant. A single-resource Delete is a plain chip behind `ChipConfirmModal`; `destructive` is only for at-scale actions (`.claude/rules/sim-settings-pages.md` "Deleting a resource"). `Button` is only for icon-only toolbar controls (`ghost`/`quiet`, `size='icon'`). ## Delete/Remove Confirmations @@ -62,6 +62,10 @@ Use `ChipConfirmModal` (title "Delete/Remove {ItemType}", `confirm={{ label, onC Default: `size-[14px]`. Color: `text-[var(--text-icon)]`. Scale: 14px > 16px > 12px > 20px. Use the `size-*` shorthand — flag `h-[Npx] w-[Npx]` and `h-N w-N` pairs as refactor targets. +## Mobile + +Check at 320px and 390px with touch, then desktop and fullscreen. Use `Chip`/`ChipLink`'s `mobileIconOnly` for familiar mobile toolbar actions, preserving accessible names; keep labels for ambiguous choices. Keep navigation, primary actions, and dismissal reachable without hover or dragging; aim for 44px touch targets with compact visible icons/button faces and 16px editable text. Match behavior to the actual container or viewport breakpoint. Contain horizontal scrolling to intentional tables/code, fit overlays to the dynamic viewport, and keep the composer/actions reachable with the keyboard open. Reuse EMCN tokens and brief motion with reduced-motion support; preserve desktop geometry. + ## Anti-patterns to flag - Raw ` + +` + +type Bridge = typeof globalThis & { + simDesktop: SimDesktopApi + pageDialog?: BrowserPageDialog | null + pageIssue?: string | null + boundsTimer?: number +} + +/** + * A page's alert, confirm, and leave-site question belong to whoever is using + * the page: the user on the tab they are looking at, the agent during its own + * action. These checks drive the real shell and native tab views. + */ +test('page dialogs wait for the user on their page and stay automatic for the agent', async () => { + const reportPath = + process.env.DESKTOP_BROWSER_DIALOGS_REPORT_PATH ?? + test.info().outputPath('browser-page-dialogs.json') + const checks: { + name: string + status: 'passed' | 'failed' + durationMs: number + error?: string + }[] = [] + const check = async (name: string, run: () => Promise) => { + const started = Date.now() + try { + await test.step(name, run) + checks.push({ name, status: 'passed', durationMs: Date.now() - started }) + } catch (error) { + checks.push({ + name, + status: 'failed', + durationMs: Date.now() - started, + error: getErrorMessage(error), + }) + throw error + } + } + const browserComponents = join( + SIM_DIR, + 'app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/browser-session' + ) + const bundle = await build({ + stdin: { + contents: `import { createElement } from 'react'; +import { createRoot } from 'react-dom/client'; +import { useBrowserSessionStore as store } from ${JSON.stringify(join(SIM_DIR, 'stores/browser-session/store.ts'))}; +import { BrowserPageDialogModal } from ${JSON.stringify(join(browserComponents, 'browser-page-dialog.tsx'))}; +import { useBrowserPanelOcclusion } from ${JSON.stringify(join(browserComponents, 'browser-panel-occlusion.ts'))}; +const scope = ${JSON.stringify(SCOPE)}; +const api = globalThis.simDesktop.browserAgent; +api.onPageState(state => store.getState().setPageState(state)); +store.subscribe(state => { + globalThis.pageDialog = state.sessions[scope]?.pageState?.dialog ?? null; + globalThis.pageIssue = state.sessions[scope]?.pageState?.issue?.kind ?? null; +}); +function Fixture() { + const page = store(state => state.sessions[scope]?.pageState); + useBrowserPanelOcclusion(scope, page?.tabId ?? null, true, () => document.getElementById('native-panel')?.getBoundingClientRect() ?? null); + return createElement(BrowserPageDialogModal, { + dialog: page?.dialog, + open: Boolean(page?.dialog), + onAnswer: (requestId, allowed) => api.panelAction({action:'respond-dialog',requestId,allowed},scope), + }); +} +createRoot(document.getElementById('root')).render(createElement(Fixture));`, + resolveDir: SIM_DIR, + loader: 'tsx', + }, + bundle: true, + write: false, + outfile: test.info().outputPath('fixture.js'), + external: ['node:async_hooks'], + banner: { js: 'var process={env:{NODE_ENV:"development"},browser:true};' }, + format: 'iife', + platform: 'browser', + tsconfig: join(SIM_DIR, 'tsconfig.json'), + define: { 'process.env.NODE_ENV': '"development"' }, + }) + const config = await loadPostcssConfig({}, SIM_DIR) + const stylesheet = join(SIM_DIR, 'app/_styles/globals.css') + const css = await postcss(config.plugins).process(readFileSync(stylesheet, 'utf8'), { + from: stylesheet, + }) + const calls = new Map() + const server: Server = createServer(async (request, response) => { + const path = new URL(request.url ?? '/', 'http://localhost').pathname + if (path === '/fixture.js' || path === '/fixture.css') { + response.setHeader('Content-Type', path.endsWith('.js') ? 'text/javascript' : 'text/css') + response.end( + path.endsWith('.js') + ? bundle.outputFiles.find((file) => file.path.endsWith('.js'))?.text + : css.css + ) + return + } + if (path === '/api/desktop/tool/authorize') { + let body = '' + for await (const chunk of request) body += chunk.toString() + const authorization = calls.get(JSON.parse(body).toolCallId) + response.writeHead(authorization ? 200 : 403, { 'Content-Type': 'application/json' }) + response.end(JSON.stringify(authorization ?? {})) + return + } + if (path === '/broken') { + response.destroy() + return + } + // The app origin serves the shell; the same server on localhost is the web. + const isSite = request.headers.host?.startsWith('localhost') === true + response.writeHead(200, { + 'Content-Type': 'text/html', + ...(isSite ? {} : { 'Set-Cookie': 'better-auth.session_token=fixture; HttpOnly; Path=/' }), + }) + response.end( + !isSite + ? SHELL_FIXTURE + : path === '/form' || path === '/agent' + ? FORM_FIXTURE + : 'next' + ) + }) + const userData = mkdtempSync(join(tmpdir(), 'sim-browser-dialogs-e2e-')) + let app: ElectronApplication | undefined + let passed = false + try { + await new Promise((resolve) => server.listen(0, '127.0.0.1', resolve)) + const address = server.address() + if (!address || typeof address === 'string') throw new Error('Missing fixture address') + const origin = `http://127.0.0.1:${address.port}` + const site = origin.replace('127.0.0.1', 'localhost') + const shellApp = await electron.launch({ + args: [process.env.SIM_DESKTOP_E2E_MAIN ?? '.'], + cwd: DESKTOP_DIR, + env: { ...process.env, SIM_DESKTOP_ORIGIN: origin, SIM_DESKTOP_USER_DATA: userData }, + }) + app = shellApp + // A dialog listener stops Playwright auto-dismissing page dialogs, so the desktop's own + // handling decides their outcome exactly as it does in production. + const nativeDialogs: Dialog[] = [] + const leaveDialogsToDesktop = (page: Page) => + page.on('dialog', (dialog) => nativeDialogs.push(dialog)) + const nextNativeDialog = async () => { + await expect.poll(() => nativeDialogs.length).toBeGreaterThan(0) + const dialog = nativeDialogs.shift() + if (!dialog) throw new Error('Missing native dialog') + return dialog + } + shellApp.context().pages().forEach(leaveDialogsToDesktop) + shellApp.context().on('page', leaveDialogsToDesktop) + const shell = await shellApp.firstWindow() + await shellApp.evaluate(({ app, BrowserWindow }) => { + const host = BrowserWindow.getAllWindows()[0] + if (!host) throw new Error('Missing host window') + app.focus({ steal: true }) + host.focus() + }) + await expect(shell.getByRole('heading')).toHaveText('Browser dialogs fixture') + await shell.evaluate(async (scope) => { + const bridge = globalThis as Bridge + const api = bridge.simDesktop.browserAgent + await api.activateScope(scope) + const updateBounds = () => + api.setPanelBounds( + { x: 0, y: 80, width: innerWidth, height: innerHeight - 80 }, + null, + scope + ) + updateBounds() + bridge.boundsTimer = window.setInterval(updateBounds, 200) + bridge.pageDialog = null + }, SCOPE) + + let callCount = 0 + const execute = async (tool: BrowserToolName, args: Record) => { + const callId = `browser-dialogs-${++callCount}` + calls.set(callId, { chatId: SCOPE, toolName: tool, args }) + const result = await shell.evaluate( + ({ callId, tool, args, scope }) => + (globalThis as Bridge).simDesktop.browserAgent.executeTool(callId, tool, args, scope), + { callId, tool, args, scope: SCOPE } + ) + expect(result.ok).toBe(true) + return result.ok ? result.result : undefined + } + /** Browser-chrome actions are user gestures, so each follows a real click in Sim. */ + const panelAction = async (action: Record) => { + if (action.action === 'respond-dialog' && (await shell.getByRole('dialog').count())) { + await shell + .getByRole('button', { + name: action.allowed ? 'Leave' : 'Stay', + exact: true, + }) + .click() + return + } + await shell.getByRole('heading').click() + await shell.evaluate( + ({ action, scope }) => + (globalThis as Bridge).simDesktop.browserAgent.panelAction( + action as unknown as Parameters[0], + scope + ), + { action, scope: SCOPE } + ) + } + const pageDialog = () => shell.evaluate(() => (globalThis as Bridge).pageDialog ?? null) + const inPage = (script: string) => + shellApp.evaluate( + async ({ webContents }, { script, url }) => + (await webContents + .getAllWebContents() + .find((contents) => contents.getURL().startsWith(url)) + ?.executeJavaScript(script)) as T, + { script, url: site } + ) + /** Read from the shell: a script cannot run in a page while its dialog is open. */ + const pageTitle = () => + shellApp.evaluate( + ({ webContents }, url) => + webContents + .getAllWebContents() + .find((contents) => contents.getURL().startsWith(url)) + ?.getTitle() ?? null, + site + ) + const pageUrl = () => + shellApp.evaluate( + ({ webContents }, url) => + webContents + .getAllWebContents() + .find((contents) => contents.getURL().startsWith(url)) + ?.getURL() ?? null, + site + ) + /** Trusted input into the page, the way the user's own mouse and keys arrive. */ + const userInput = (selector: string, text = '') => + shellApp.evaluate( + async ({ webContents }, { selector, text, url }) => { + const contents = webContents + .getAllWebContents() + .find((candidate) => candidate.getURL().startsWith(url)) + if (!contents) throw new Error('No page') + const rect: { x: number; y: number } = await contents.executeJavaScript( + `(() => { const { x, y } = document.querySelector(${JSON.stringify(selector)}).getBoundingClientRect(); return { x, y } })()` + ) + const point = { x: Math.round(rect.x + 5), y: Math.round(rect.y + 5) } + contents.sendInputEvent({ type: 'mouseDown', ...point, button: 'left', clickCount: 1 }) + contents.sendInputEvent({ type: 'mouseUp', ...point, button: 'left', clickCount: 1 }) + for (const character of text) + contents.sendInputEvent({ type: 'char', keyCode: character }) + }, + { selector, text, url: site } + ) + + await check('without a renderer that shows dialogs, the shell still answers them', async () => { + await execute('browser_open_url', { url: `${site}/form` }) + await panelAction({ action: 'switch-tab', tabId: '1' }) + await userInput('#delete') + await expect.poll(() => pageTitle()).toBe('confirm:false') + expect(await pageDialog()).toBeNull() + }) + + await panelAction({ action: 'enable-page-dialogs' }) + await inPage("document.title = 'form'") + + nativeDialogs.length = 0 + await check("the user's confirm waits for their answer without a duplicate modal", async () => { + await userInput('#delete') + const dialog = await nextNativeDialog() + expect(dialog.type()).toBe('confirm') + expect(dialog.message()).toBe('Delete the report?') + expect(await pageTitle()).toBe('form') + expect(await pageDialog()).toBeNull() + await expect(shell.getByRole('dialog')).toHaveCount(0) + await dialog.accept() + await expect.poll(() => pageTitle()).toBe('confirm:true') + }) + + await check('a user sees the complete decision text beyond the diagnostic limit', async () => { + await userInput('#long-message') + const dialog = await nextNativeDialog() + expect(dialog.message()).toBe(`${'Details '.repeat(150)}Final decision detail`) + await dialog.accept() + }) + + await check('automation leaves a user-owned confirmation unanswered', async () => { + await inPage("document.title = 'form'") + await userInput('#delete') + const dialog = await nextNativeDialog() + const callId = `browser-dialogs-${++callCount}` + calls.set(callId, { chatId: SCOPE, toolName: 'browser_snapshot', args: {} }) + const result = await shell.evaluate( + ({ callId, scope }) => + (globalThis as Bridge).simDesktop.browserAgent.executeTool( + callId, + 'browser_snapshot', + {}, + scope + ), + { callId, scope: SCOPE } + ) + expect(result.ok).toBe(false) + await shell.evaluate( + ({ callId, scope }) => + (globalThis as Bridge).simDesktop.browserAgent.cancelTool?.(callId, scope), + { callId, scope: SCOPE } + ) + expect(await pageTitle()).toBe('form') + await dialog.accept() + await expect.poll(() => pageTitle()).toBe('confirm:true') + }) + + await check("the agent's dialogs never wait on the user", async () => { + await inPage("document.title = 'form'") + const snapshot = await execute('browser_snapshot', {}) + const ref = /button "Delete" \[ref=(\d+)\]/.exec( + String((snapshot as { outline?: string }).outline) + )?.[1] + expect(ref, 'snapshot lists the Delete button').toBeTruthy() + await execute('browser_click', { elementId: Number(ref) }) + await expect.poll(() => pageTitle()).toBe('confirm:false') + expect(await pageDialog()).toBeNull() + await inPage("document.title = 'form'") + await execute('browser_click', { elementId: Number(ref), dialog: { accept: true } }) + await expect.poll(() => pageTitle()).toBe('confirm:true') + expect(await pageDialog()).toBeNull() + }) + + // CDP answers the JavaScript call but Electron owns its native sheet. Reload + // ends the native dialogs before testing the renderer's leave-site modal. + await panelAction({ action: 'reload' }) + await expect.poll(() => pageTitle()).toBe('form') + + await check('clean reloads and navigation never ask to discard changes', async () => { + await inPage("document.documentElement.dataset.reloadProbe = 'before'") + await panelAction({ action: 'reload' }) + await expect + .poll(() => inPage('document.documentElement.dataset.reloadProbe')) + .toBeUndefined() + await expect.poll(() => pageTitle()).toBe('form') + expect(await pageDialog()).toBeNull() + await panelAction({ action: 'navigate', url: `${site}/next` }) + await expect.poll(pageUrl).toBe(`${site}/next`) + expect(await pageDialog()).toBeNull() + await panelAction({ action: 'back' }) + await expect.poll(pageUrl).toBe(`${site}/form`) + expect(await pageDialog()).toBeNull() + }) + + await check('clearing a draft removes the leave warning', async () => { + await userInput('#draft', 'temporary draft') + await inPage("document.getElementById('draft').value = ''") + await panelAction({ action: 'navigate', url: `${site}/next` }) + await expect.poll(pageUrl).toBe(`${site}/next`) + expect(await pageDialog()).toBeNull() + await panelAction({ action: 'navigate', url: `${site}/form` }) + await expect.poll(pageUrl).toBe(`${site}/form`) + }) + + await check('same-page navigation preserves a draft without a leave warning', async () => { + await userInput('#draft', 'same-page draft') + await panelAction({ action: 'navigate', url: `${site}/form#section` }) + await expect.poll(pageUrl).toBe(`${site}/form#section`) + expect(await pageDialog()).toBeNull() + expect(await inPage("document.getElementById('draft').value")).toBe('same-page draft') + await panelAction({ action: 'back' }) + await expect.poll(pageUrl).toBe(`${site}/form`) + expect(await pageDialog()).toBeNull() + await inPage(`setTimeout(() => { location.href = ${JSON.stringify(`${site}/next`)} })`) + await expect.poll(pageUrl).toBe(`${site}/next`) + expect(await pageDialog()).toBeNull() + await panelAction({ action: 'navigate', url: `${site}/form` }) + await expect.poll(pageUrl).toBe(`${site}/form`) + }) + + await check('leaving a draft from the URL bar asks, and Stay keeps it', async () => { + await userInput('#draft', 'draft') + await expect + .poll(() => inPage("document.getElementById('draft').value")) + .toBe('draft') + await panelAction({ action: 'navigate', url: `${site}/next` }) + await expect.poll(pageDialog).toMatchObject({ kind: 'beforeunload' }) + const dialog = await pageDialog() + await panelAction({ action: 'respond-dialog', requestId: dialog?.requestId, allowed: false }) + await expect.poll(pageDialog).toBeNull() + expect(await pageUrl()).toBe(`${site}/form`) + expect(await inPage("document.getElementById('draft').value")).toBe('draft') + }) + + await check('Reload of a draft asks too, and Stay keeps it', async () => { + await panelAction({ action: 'reload' }) + await expect.poll(pageDialog).toMatchObject({ kind: 'beforeunload' }) + const dialog = await pageDialog() + await panelAction({ action: 'respond-dialog', requestId: dialog?.requestId, allowed: false }) + await expect.poll(pageDialog).toBeNull() + expect(await inPage("document.getElementById('draft').value")).toBe('draft') + }) + + await check('Escape keeps the draft and returns typing to the page', async () => { + await userInput('#draft') + await panelAction({ action: 'reload' }) + await expect(shell.getByRole('button', { name: 'Stay', exact: true })).toBeFocused() + await shell.getByRole('dialog').screenshot({ + path: test.info().outputPath('leave-page-modal.png'), + animations: 'allow', + caret: 'initial', + }) + await expect(shell.getByRole('button', { name: 'Stay', exact: true })).toBeFocused() + await shell.keyboard.press('Escape') + await expect.poll(pageDialog).toBeNull() + await expect + .poll(() => + shellApp.evaluate(({ webContents }) => webContents.getFocusedWebContents()?.getURL()) + ) + .toBe(`${site}/form`) + await shellApp.evaluate(({ webContents }) => { + const contents = webContents.getFocusedWebContents() + if (!contents) throw new Error('Missing focused page') + contents.sendInputEvent({ type: 'char', keyCode: 'x' }) + }) + expect(await inPage("document.getElementById('draft').value")).toBe('xdraft') + }) + + await check('Leave lets the navigation through without asking again', async () => { + await panelAction({ action: 'navigate', url: `${site}/next` }) + await expect.poll(pageDialog).toMatchObject({ kind: 'beforeunload' }) + const dialog = await pageDialog() + await panelAction({ action: 'respond-dialog', requestId: dialog?.requestId, allowed: true }) + await expect.poll(pageUrl).toBe(`${site}/next`) + expect(await pageDialog()).toBeNull() + }) + await check('a crashed page drops its pending Leave decision', async () => { + await panelAction({ action: 'navigate', url: `${site}/form` }) + await expect.poll(pageUrl).toBe(`${site}/form`) + await userInput('#draft', 'draft') + await panelAction({ action: 'reload' }) + await expect.poll(pageDialog).toMatchObject({ kind: 'beforeunload' }) + const stale = await pageDialog() + await shellApp.evaluate(({ webContents }, url) => { + webContents + .getAllWebContents() + .find((contents) => contents.getURL() === url) + ?.forcefullyCrashRenderer() + }, `${site}/form`) + await expect.poll(pageDialog).toBeNull() + await panelAction({ action: 'respond-dialog', requestId: stale?.requestId, allowed: true }) + await panelAction({ action: 'reload' }) + await expect.poll(() => pageTitle()).toBe('form') + await expect.poll(pageDialog).toBeNull() + }) + + await check('Back from a failed load does not capture a later page navigation', async () => { + await userInput('#draft', 'draft') + await panelAction({ action: 'navigate', url: `${site}/broken` }) + await expect.poll(pageDialog).toMatchObject({ kind: 'beforeunload' }) + const dialog = await pageDialog() + await panelAction({ action: 'respond-dialog', requestId: dialog?.requestId, allowed: true }) + await expect + .poll(() => shell.evaluate(() => (globalThis as Bridge).pageIssue)) + .toBe('load-error') + await panelAction({ action: 'back' }) + await inPage(`setTimeout(() => { location.href = ${JSON.stringify(`${site}/next`)} })`) + await expect.poll(pageUrl).toBe(`${site}/next`) + expect(await pageDialog()).toBeNull() + }) + + /** Opens a page-initiated confirm (no user gesture) on the tab at `path`. */ + const confirmFromPage = (path: string) => + shellApp.evaluate(({ webContents }, url) => { + const contents = webContents.getAllWebContents().find((c) => c.getURL() === url) + if (!contents) throw new Error(`No page at ${url}`) + void contents.executeJavaScript( + "setTimeout(() => { document.title = 'confirm:' + confirm('Continue?') })" + ) + }, `${site}${path}`) + const titleAt = (path: string) => + shellApp.evaluate( + ({ webContents }, url) => + webContents + .getAllWebContents() + .find((c) => c.getURL() === url) + ?.getTitle() ?? null, + `${site}${path}` + ) + + await check( + "a dialog on the agent's page stays automatic until the user takes it", + async () => { + await execute('browser_open_tab', { url: `${site}/agent` }) + const tabs = await shell.evaluate( + async (scope) => + (await (globalThis as Bridge).simDesktop.browserAgent.activateScope(scope)).tabs, + SCOPE + ) + const agentTabId = tabs.find((tab) => tab.url === `${site}/agent`)?.tabId + // The resource strip mirrors the agent's tab on screen without the user claiming it. + await panelAction({ action: 'switch-tab', tabId: agentTabId, claim: false }) + await confirmFromPage('/agent') + await expect.poll(() => titleAt('/agent')).toBe('confirm:false') + expect(await pageDialog()).toBeNull() + } + ) + + await check('a dialog while the browser is off screen stays automatic', async () => { + const tabs = await shell.evaluate( + async (scope) => + (await (globalThis as Bridge).simDesktop.browserAgent.activateScope(scope)).tabs, + SCOPE + ) + const tabId = tabs.find((tab) => tab.url === `${site}/agent`)?.tabId + expect(tabId).toBeTruthy() + await panelAction({ action: 'switch-tab', tabId }) + await shell.evaluate((scope) => { + const bridge = globalThis as Bridge + window.clearInterval(bridge.boundsTimer) + bridge.simDesktop.browserAgent.setPanelBounds(null, null, scope) + }, SCOPE) + await confirmFromPage('/agent') + await expect.poll(() => titleAt('/agent')).toBe('confirm:false') + expect(await pageDialog()).toBeNull() + }) + passed = true + } finally { + mkdirSync(dirname(reportPath), { recursive: true }) + writeFileSync(reportPath, JSON.stringify({ passed, checks }, null, 2)) + await app?.close() + await new Promise((resolve) => server.close(() => resolve())) + rmSync(userData, { recursive: true, force: true }) + } +}) diff --git a/apps/desktop/e2e/desktop-tools-live-sim.spec.ts b/apps/desktop/e2e/desktop-tools-live-sim.spec.ts index 6a4fe359933..20b2262b17b 100644 --- a/apps/desktop/e2e/desktop-tools-live-sim.spec.ts +++ b/apps/desktop/e2e/desktop-tools-live-sim.spec.ts @@ -245,6 +245,12 @@ test.describe('desktop tools against a live Sim', () => { }) const page = await app.firstWindow({ timeout }) pageErrors = [] + app.on('window', (permission) => { + void permission + .getByRole('button', { name: 'Allow folder', exact: true }) + .click({ timeout: 10_000 }) + .catch((error) => pageErrors.push(`Folder approval failed: ${String(error)}`)) + }) page.on('pageerror', (error) => pageErrors.push(error.message)) page.on('console', (message) => { if (message.type() === 'error') pageErrors.push(message.text()) diff --git a/apps/desktop/e2e/executor-sim.ts b/apps/desktop/e2e/executor-sim.ts index 0fd7f3d3153..f2b8e381042 100644 --- a/apps/desktop/e2e/executor-sim.ts +++ b/apps/desktop/e2e/executor-sim.ts @@ -462,6 +462,7 @@ export async function launch( app.context().pages().forEach(leaveDialogsToDesktop) app.context().on('page', leaveDialogsToDesktop) const window = await app.firstWindow() + await window.waitForURL((url) => url.origin === sim.origin, { waitUntil: 'load' }) return { app, window } } diff --git a/apps/desktop/e2e/local-files.spec.ts b/apps/desktop/e2e/local-files.spec.ts index 0a9a08f0678..59e8e60aa4b 100644 --- a/apps/desktop/e2e/local-files.spec.ts +++ b/apps/desktop/e2e/local-files.spec.ts @@ -1,25 +1,52 @@ -import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { + mkdirSync, + mkdtempSync, + readFileSync, + realpathSync, + renameSync, + rmSync, + symlinkSync, + writeFileSync, +} from 'node:fs' import { createServer, type Server } from 'node:http' import { tmpdir } from 'node:os' import { join } from 'node:path' import { fileURLToPath } from 'node:url' import { type ElectronApplication, _electron as electron, expect, test } from '@playwright/test' import type { DesktopLocalFileRequest, SimDesktopApi } from '@sim/desktop-bridge' +import { build } from 'esbuild' +import postcss from 'postcss' +import loadPostcssConfig from 'postcss-load-config' const DESKTOP_DIR = fileURLToPath(new URL('..', import.meta.url)) +const SIM_DIR = fileURLToPath(new URL('../../sim/', import.meta.url)) -test('native file tools read and import through the installed preload without Sim folder grants', async () => { +test('native file tools remember folder consent across chats and restarts until revoked', async () => { + test.setTimeout(180_000) const root = mkdtempSync(join(tmpdir(), 'sim-native-files-e2e-')) const source = join(root, 'Reports') + const outside = join(root, 'Reports-other') + mkdirSync(outside) + writeFileSync(join(outside, 'private.txt'), 'outside contents') mkdirSync(join(source, 'empty'), { recursive: true }) writeFileSync(join(source, 'report.txt'), 'native file contents') const png = 'iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mP8/x8AAusB9Y9Zl1sAAAAASUVORK5CYII=' writeFileSync(join(source, 'image.png'), Buffer.from(png, 'base64')) - /** Calls the server saw claimed; like the server, only an import refuses a second claim. */ const claimed = new Set() - const calls: Record }> = { + const expireAfterAuthorization = new Set() + let signedIn = true + const calls: Record< + string, + { toolName: string; args: Record; chatId?: string } | undefined + > = { + directory: { toolName: 'read_local_file', args: { path: source } }, text: { toolName: 'read_local_file', args: { path: join(source, 'report.txt') } }, + otherChat: { + toolName: 'read_local_file', + args: { path: join(source, 'report.txt') }, + chatId: 'other-chat', + }, image: { toolName: 'read_local_file', args: { path: join(source, 'image.png') } }, import: { toolName: 'import_local_files', @@ -29,17 +56,78 @@ test('native file tools read and import through the installed preload without Si let server: Server | undefined let app: ElectronApplication | undefined try { + const bundle = await build({ + stdin: { + contents: `import { createRoot } from 'react-dom/client'; +import { ToastProvider } from '@sim/emcn'; +import { AppRouterContext } from 'next/dist/shared/lib/app-router-context.shared-runtime'; +import { PathParamsContext } from 'next/dist/shared/lib/hooks-client-context.shared-runtime'; +import { Desktop } from '@/app/workspace/[workspaceId]/settings/components/desktop/desktop'; +import { SettingsHeaderProvider, SettingsHeaderShell } from '@/components/settings/settings-header'; +import { SettingsSectionProvider } from '@/components/settings/settings-panel'; +const router = { bfcacheId: 'fixture', back: () => history.back(), forward: () => history.forward(), refresh: () => location.reload(), push: url => location.assign(url), replace: url => location.replace(url), prefetch: () => {} }; +createRoot(document.getElementById('settings')).render( + + + + + + + +);`, + resolveDir: SIM_DIR, + loader: 'tsx', + }, + bundle: true, + jsx: 'automatic', + write: false, + outfile: test.info().outputPath('settings.js'), + external: ['node:async_hooks', 'postgres'], + banner: { js: 'var process={env:{NODE_ENV:"development"},browser:true};' }, + format: 'iife', + platform: 'browser', + tsconfig: join(SIM_DIR, 'tsconfig.json'), + define: { 'process.env.NODE_ENV': '"development"' }, + }) + const config = await loadPostcssConfig({}, SIM_DIR) + const cssPath = join(SIM_DIR, 'app/_styles/globals.css') + const css = await postcss(config.plugins).process(readFileSync(cssPath, 'utf8'), { + from: cssPath, + }) server = createServer(async (request, response) => { const path = new URL(request.url ?? '/', 'http://127.0.0.1').pathname + if (path === '/settings.js' || path === '/settings.css') { + response.setHeader('Content-Type', path.endsWith('.js') ? 'text/javascript' : 'text/css') + response.end( + path.endsWith('.js') + ? bundle.outputFiles.find((file) => file.path.endsWith('.js'))?.text + : css.css + ) + return + } if (path === '/api/auth/get-session') { response.writeHead(200, { 'Content-Type': 'application/json' }).end( - JSON.stringify({ - user: { id: 'local-file-user' }, - session: { id: 'local-file-session' }, - }) + JSON.stringify( + signedIn + ? { + user: { id: 'local-file-user' }, + session: { id: 'local-file-session' }, + } + : null + ) ) return } + if (path === '/api/auth/sign-out') { + signedIn = false + response + .writeHead(200, { + 'Content-Type': 'application/json', + 'Set-Cookie': 'better-auth.session_token=; HttpOnly; SameSite=Lax; Path=/; Max-Age=0', + }) + .end('{}') + return + } if (path === '/api/desktop/tool/authorize') { let body = '' for await (const chunk of request) body += chunk.toString() @@ -55,30 +143,49 @@ test('native file tools read and import through the installed preload without Si if (input.claim) claimed.add(input.toolCallId) response .writeHead(200, { 'Content-Type': 'application/json' }) - .end(JSON.stringify({ ...call, chatId: 'org-chat' })) + .end(JSON.stringify({ ...call, chatId: call.chatId ?? 'org-chat' })) + if (expireAfterAuthorization.has(input.toolCallId)) calls[input.toolCallId] = undefined return } response .writeHead(200, { 'Content-Type': 'text/html', - 'Set-Cookie': 'better-auth.session_token=fixture; HttpOnly; SameSite=Lax; Path=/', + ...(signedIn + ? { 'Set-Cookie': 'better-auth.session_token=fixture; HttpOnly; SameSite=Lax; Path=/' } + : {}), }) - .end('Local file fixture

Local files

') + .end(`Local file fixture

Local files

+ +
`) }) await new Promise((resolve) => server?.listen(0, '127.0.0.1', resolve)) const address = server.address() if (!address || typeof address === 'string') throw new Error('Missing fixture address') - app = await electron.launch({ - args: ['.'], - cwd: DESKTOP_DIR, - env: { - ...process.env, - SIM_DESKTOP_ORIGIN: `http://127.0.0.1:${address.port}`, - SIM_DESKTOP_USER_DATA: join(root, 'profile'), - }, + const launch = () => + electron.launch({ + args: ['.', '--use-mock-keychain'], + cwd: DESKTOP_DIR, + env: { + ...process.env, + SIM_DESKTOP_ORIGIN: `http://127.0.0.1:${address.port}`, + SIM_DESKTOP_USER_DATA: join(root, 'profile'), + }, + }) + app = await launch() + let window = await app.firstWindow() + await app.evaluate(({ app, BrowserWindow }) => { + const host = BrowserWindow.getAllWindows()[0] + if (!host) throw new Error('Missing host window') + host.webContents.setBackgroundThrottling(false) + app.focus({ steal: true }) + host.focus() }) - const window = await app.firstWindow() - await expect(window.getByRole('heading')).toHaveText('Local files') + await expect(window.getByRole('heading', { name: 'Local files', exact: true })).toHaveText( + 'Local files' + ) const invoke = (input: DesktopLocalFileRequest) => window.evaluate(async (request) => { const api = (globalThis as typeof globalThis & { simDesktop: SimDesktopApi }).simDesktop @@ -86,9 +193,64 @@ test('native file tools read and import through the installed preload without Si return api.localFiles(request) }, input) await expect - .poll(async () => (await invoke({ operation: 'read', toolCallId: 'text' })).ok) + .poll(() => + window.evaluate(async () => { + const api = (globalThis as typeof globalThis & { simDesktop: SimDesktopApi }).simDesktop + return (await api.localFilesystem?.({ operation: 'list_mounts' }))?.ok + }) + ) .toBe(true) - expect(await invoke({ operation: 'read', toolCallId: 'text' })).toMatchObject({ + await window.evaluate(async () => { + const api = (globalThis as typeof globalThis & { simDesktop: SimDesktopApi }).simDesktop + await api.settings.setPreference('browserEnabled', false) + await api.settings.setPreference('terminalEnabled', false) + }) + const deniedPrompt = app.waitForEvent('window', { timeout: 10_000 }) + const deniedReads = ['text', 'otherChat'].map((toolCallId) => + invoke({ operation: 'read', toolCallId }) + ) + const deniedResults: unknown[] = [] + for (const read of deniedReads) + void read.then((result) => deniedResults.push(result)).catch(() => {}) + const denial = await deniedPrompt + await expect(denial.getByRole('button', { name: "Don't allow", exact: true })).toBeFocused() + await denial.screenshot({ + path: + process.env.DESKTOP_LOCAL_FILES_REPORT_PATH ?? + test.info().outputPath('local-file-consent.png'), + }) + await denial + .getByRole('button', { name: "Don't allow", exact: true }) + .click({ noWaitAfter: true }) + await expect.poll(() => deniedResults.length).toBe(2) + expect(deniedResults).toEqual([ + { ok: false, error: expect.any(String) }, + { ok: false, error: expect.any(String) }, + ]) + + const folderPrompt = app.waitForEvent('window') + const folderRead = invoke({ operation: 'read', toolCallId: 'directory' }) + void folderRead.catch(() => {}) + const folderConsent = await folderPrompt + const queuedRead = invoke({ operation: 'read', toolCallId: 'text' }) + void queuedRead.catch(() => {}) + await expect( + folderConsent.getByRole('button', { name: 'Allow folder', exact: true }) + ).toBeVisible() + expect( + await folderConsent.evaluate(() => typeof (globalThis as { simDesktop?: unknown }).simDesktop) + ).toBe('undefined') + await folderConsent + .getByRole('button', { name: 'Allow folder', exact: true }) + .click({ noWaitAfter: true }) + expect(await folderRead).toMatchObject({ ok: true, data: { representation: 'directory' } }) + expect(await queuedRead).toMatchObject({ ok: true, data: { text: 'native file contents' } }) + const canonicalRequest = { + operation: 'read' as const, + toolCallId: 'text', + path: join(outside, 'private.txt'), + } + expect(await invoke(canonicalRequest)).toMatchObject({ ok: true, data: { representation: 'text', text: 'native file contents' }, }) @@ -96,7 +258,580 @@ test('native file tools read and import through the installed preload without Si ok: true, data: { observations: [{ mediaType: 'image/png', data: png }] }, }) - expect([...claimed]).toEqual(['text', 'image']) + const requestPermission = async (request: DesktopLocalFileRequest) => { + if (!app) throw new Error('Desktop app is not running') + const shown = app.waitForEvent('window', { timeout: 10_000 }) + const result = invoke(request) + void result.catch(() => {}) + const prompt = await shown + await expect(prompt.getByRole('button', { name: "Don't allow", exact: true })).toBeVisible() + return { prompt, result } + } + await test.step('File → Folder Access opens over a focused utility window', async () => { + if (!app) throw new Error('Desktop app is not running') + const serverShown = app.waitForEvent('window', { timeout: 10_000 }) + await app.evaluate(({ Menu }) => { + const item = Menu.getApplicationMenu() + ?.items.flatMap((entry) => entry.submenu?.items ?? []) + .find((entry) => entry.label === 'Server…') + if (!item) throw new Error('Server menu item missing') + item.click() + }) + await expect(await serverShown).toHaveTitle('Sim - Server') + await expect + .poll(() => + app?.evaluate(({ BrowserWindow }) => + BrowserWindow.getAllWindows().some( + (win) => win.webContents.getURL().endsWith('server.html') && win.isVisible() + ) + ) + ) + .toBe(true) + const accessMenu = await app.evaluate(({ BrowserWindow, Menu }) => { + const utility = BrowserWindow.getAllWindows().find((win) => + win.webContents.getURL().endsWith('server.html') + ) + if (!utility) throw new Error('Server window missing') + const item = Menu.getApplicationMenu() + ?.items.flatMap((entry) => entry.submenu?.items ?? []) + .find((entry) => entry.label === 'Folder Access…') + if (!item) throw new Error('Folder Access menu item missing') + const popup = Menu.prototype.popup + let shown: { anchoredToUtility: boolean; items: string[] } | undefined + Menu.prototype.popup = function (this: Electron.Menu, options) { + shown = { + anchoredToUtility: options?.window === utility, + items: this.items.map((entry) => entry.label), + } + } + try { + item.click(undefined, utility) + } finally { + Menu.prototype.popup = popup + } + utility.close() + return shown + }) + expect(accessMenu?.anchoredToUtility).toBe(true) + expect(accessMenu?.items).toContain('Add Folder…') + }) + + await test.step('Stop cancels only its own request while chats share a folder prompt', async () => { + const sharedFolder = join(root, 'Shared') + mkdirSync(sharedFolder) + writeFileSync(join(sharedFolder, 'shared.txt'), 'shared contents') + for (const toolCallId of ['sharedLeader', 'sharedFollower', 'sharedSurvivor']) { + calls[toolCallId] = { + toolName: 'read_local_file', + args: { path: join(sharedFolder, 'shared.txt') }, + chatId: toolCallId, + } + } + const leader = await requestPermission({ operation: 'read', toolCallId: 'sharedLeader' }) + let followerResult: unknown + const follower = invoke({ operation: 'read', toolCallId: 'sharedFollower' }).then( + (result) => { + followerResult = result + } + ) + void follower.catch(() => {}) + await expect.poll(() => claimed.has('sharedFollower')).toBe(true) + await leader.prompt.getByRole('dialog').hover() + await invoke({ operation: 'cancel', toolCallId: 'sharedFollower' }) + await expect.poll(() => followerResult).toMatchObject({ ok: false }) + await follower + await expect(leader.prompt.getByRole('dialog')).toBeVisible() + const survivor = invoke({ operation: 'read', toolCallId: 'sharedSurvivor' }) + void survivor.catch(() => {}) + await expect.poll(() => claimed.has('sharedSurvivor')).toBe(true) + await leader.prompt.getByRole('dialog').hover() + await invoke({ operation: 'cancel', toolCallId: 'sharedLeader' }) + expect(await leader.result).toMatchObject({ ok: false }) + await expect(leader.prompt.getByRole('dialog')).toBeVisible() + await leader.prompt + .getByRole('button', { name: 'Allow folder', exact: true }) + .click({ noWaitAfter: true }) + expect(await survivor).toMatchObject({ ok: true, data: { text: 'shared contents' } }) + }) + await test.step('expired requests cannot enable Full file access from a confirmation', async () => { + if (!app) throw new Error('Desktop app is not running') + calls.expiringFullAccess = { + toolName: 'read_local_file', + args: { path: join(outside, 'private.txt') }, + } + const permission = await requestPermission({ + operation: 'read', + toolCallId: 'expiringFullAccess', + }) + const shown = app.waitForEvent('window') + await permission.prompt + .getByRole('button', { name: 'Full file access', exact: true }) + .click({ noWaitAfter: true }) + const confirmation = await shown + calls.expiringFullAccess = undefined + await confirmation + .getByRole('button', { name: 'Enable', exact: true }) + .click({ noWaitAfter: true }) + expect(await permission.result).toMatchObject({ ok: false }) + expect( + await window.evaluate(async () => + ( + globalThis as typeof globalThis & { simDesktop: SimDesktopApi } + ).simDesktop.settings.getPreferences() + ) + ).toMatchObject({ fullFileAccess: false }) + }) + await test.step('Full file access is opt-in, survives restart, and stops granting access when disabled', async () => { + if (!app) throw new Error('Desktop app is not running') + const fullFolder = join(root, 'Full access') + mkdirSync(fullFolder) + writeFileSync(join(fullFolder, 'file.txt'), 'full access contents') + calls.fullAccess = { + toolName: 'read_local_file', + args: { path: join(fullFolder, 'file.txt') }, + } + await expect( + window.getByRole('switch', { name: 'Full file access', exact: true }) + ).not.toBeChecked() + const permission = await requestPermission({ operation: 'read', toolCallId: 'fullAccess' }) + const confirmationShown = app.waitForEvent('window') + await permission.prompt + .getByRole('button', { name: 'Full file access', exact: true }) + .click({ noWaitAfter: true }) + const confirmation = await confirmationShown + await expect(confirmation.getByRole('button', { name: 'Cancel', exact: true })).toBeFocused() + const returnedPrompt = app.waitForEvent('window') + await confirmation + .getByRole('button', { name: 'Cancel', exact: true }) + .click({ noWaitAfter: true }) + const folderPrompt = await returnedPrompt + expect( + await window.evaluate(async () => + ( + globalThis as typeof globalThis & { simDesktop: SimDesktopApi } + ).simDesktop.settings.getPreferences() + ) + ).toMatchObject({ fullFileAccess: false }) + const acceptedConfirmation = app.waitForEvent('window') + await folderPrompt + .getByRole('button', { name: 'Full file access', exact: true }) + .click({ noWaitAfter: true }) + const allowAll = await acceptedConfirmation + await allowAll.screenshot({ + path: test.info().outputPath('full-file-access-confirmation.png'), + }) + await allowAll + .getByRole('button', { name: 'Enable', exact: true }) + .click({ noWaitAfter: true }) + expect(await permission.result).toMatchObject({ + ok: true, + data: { text: 'full access contents' }, + }) + await expect( + window.getByRole('switch', { name: 'Full file access', exact: true }) + ).toBeChecked() + expect(await invoke({ operation: 'read', toolCallId: 'fullAccess' })).toMatchObject({ + ok: true, + data: { text: 'full access contents' }, + }) + calls.fullImport = { + toolName: 'import_local_files', + args: { path: fullFolder, targetWorkspaceId: 'target-workspace' }, + } + const manifest = await invoke({ operation: 'manifest', toolCallId: 'fullImport' }) + if (!manifest.ok || manifest.data.kind !== 'manifest') + throw new Error('Missing full-access manifest') + const imported = manifest.data.entries.find((entry) => entry.relativePath === 'file.txt') + if (!imported) throw new Error('Missing full-access file') + const chunk = await invoke({ + operation: 'chunk', + toolCallId: 'fullImport', + relativePath: imported.relativePath, + revision: imported.revision, + offset: 0, + }) + if (!chunk.ok || chunk.data.kind !== 'chunk') throw new Error('Missing full-access contents') + expect(Object.values(chunk.data.bytes)).toEqual([...Buffer.from('full access contents')]) + await expect( + window.getByRole('switch', { name: 'Full file access', exact: true }) + ).toBeChecked() + await expect( + window.getByRole('switch', { name: 'Full file access', exact: true }) + ).toBeEnabled() + await window.screenshot({ + path: test.info().outputPath('desktop-settings-full-access.png'), + animations: 'disabled', + }) + await window.evaluate(() => document.documentElement.classList.add('dark')) + await window.screenshot({ + path: test.info().outputPath('desktop-settings-full-access-dark.png'), + animations: 'disabled', + }) + await window.evaluate(() => document.documentElement.classList.remove('dark')) + expect(await invoke({ operation: 'read', toolCallId: 'notAuthorized' })).toMatchObject({ + ok: false, + }) + await app?.close() + app = await launch() + window = await app.firstWindow() + await expect(window.getByRole('heading', { name: 'Local files', exact: true })).toHaveText( + 'Local files' + ) + await expect + .poll(() => + window.evaluate(async () => + ( + globalThis as typeof globalThis & { simDesktop: SimDesktopApi } + ).simDesktop.settings.getPreferences() + ) + ) + .toMatchObject({ fullFileAccess: true }) + expect(await invoke({ operation: 'read', toolCallId: 'fullAccess' })).toMatchObject({ + ok: true, + data: { text: 'full access contents' }, + }) + await window.getByRole('switch', { name: 'Full file access', exact: true }).click() + await expect( + window.getByRole('switch', { name: 'Full file access', exact: true }) + ).not.toBeChecked() + const revoked = await requestPermission({ operation: 'read', toolCallId: 'fullAccess' }) + await revoked.prompt + .getByRole('button', { name: "Don't allow", exact: true }) + .click({ noWaitAfter: true }) + expect(await revoked.result).toMatchObject({ ok: false }) + }) + await test.step('a folder grant works in another chat but does not permit symlink escapes', async () => { + expect(await invoke({ operation: 'read', toolCallId: 'otherChat' })).toMatchObject({ + ok: true, + data: { text: 'native file contents' }, + }) + writeFileSync(join(source, 'empty', 'new.txt'), 'new file in a subfolder') + calls.nested = { + toolName: 'read_local_file', + args: { path: join(source, 'empty', 'new.txt') }, + chatId: 'another-chat', + } + expect(await invoke({ operation: 'read', toolCallId: 'nested' })).toMatchObject({ + ok: true, + data: { text: 'new file in a subfolder' }, + }) + rmSync(join(source, 'empty', 'new.txt')) + symlinkSync(join(outside, 'private.txt'), join(source, 'linked.txt')) + calls.escape = { toolName: 'read_local_file', args: { path: join(source, 'linked.txt') } } + const escapedRead = await requestPermission({ operation: 'read', toolCallId: 'escape' }) + await expect(escapedRead.prompt.getByRole('dialog')).toContainText( + JSON.stringify(realpathSync(outside)) + ) + await escapedRead.prompt + .getByRole('button', { name: "Don't allow", exact: true }) + .click({ noWaitAfter: true }) + expect(await escapedRead.result).toMatchObject({ ok: false }) + rmSync(join(source, 'linked.txt')) + }) + await test.step('cancelled calls and changed arguments cannot acquire a grant', async () => { + for (const changed of [false, true]) { + calls.stale = { toolName: 'read_local_file', args: { path: join(outside, 'private.txt') } } + const stale = await requestPermission({ operation: 'read', toolCallId: 'stale' }) + if (changed) calls.stale.args.path = join(source, 'report.txt') + else calls.stale = undefined + await stale.prompt + .getByRole('button', { name: 'Allow folder', exact: true }) + .click({ noWaitAfter: true }) + expect(await stale.result).toMatchObject({ ok: false }) + } + }) + await test.step('native traversal rejects redirected ancestors and replaced grant identities', async () => { + const parent = join(realpathSync(source), 'native-parent') + mkdirSync(parent) + writeFileSync(join(parent, 'inside.txt'), 'inside') + const linked = join(realpathSync(source), 'native-link') + symlinkSync(outside, linked) + try { + const result = await app?.evaluate( + async ({ app }, paths) => { + const { createRequire } = process.getBuiltinModule('node:module') + const { stat } = process.getBuiltinModule('node:fs/promises') + const fs = process.getBuiltinModule('node:fs') + const native = createRequire(`${app.getAppPath()}/package.json`)( + './dist/native/directory.node' + ) as { + openApproved( + root: string, + relative: string, + dev: bigint, + ino: bigint, + directory: boolean + ): Promise + } + const root = await stat(paths.source, { bigint: true }) + const denied = async (path: string, ino = root.ino) => { + try { + const fd = await native.openApproved(paths.source, path, root.dev, ino, false) + fs.closeSync(fd) + return false + } catch { + return true + } + } + const valid = await native.openApproved( + paths.source, + 'native-parent/inside.txt', + root.dev, + root.ino, + false + ) + const text = fs.readFileSync(valid, 'utf8') + fs.closeSync(valid) + return { + text, + ancestor: await denied('native-link/private.txt'), + traversal: await denied('../Reports-other/private.txt'), + replaced: await denied('native-parent/inside.txt', root.ino + 1n), + overflow: await denied('native-parent/inside.txt', root.ino + (1n << 64n)), + } + }, + { source: realpathSync(source) } + ) + expect(result).toEqual({ + text: 'inside', + ancestor: true, + traversal: true, + replaced: true, + overflow: true, + }) + } finally { + rmSync(linked) + rmSync(parent, { recursive: true }) + } + }) + await test.step('a symlink replacement cannot redirect an inspected file', async () => { + const file = realpathSync(join(source, 'report.txt')) + const backup = join(source, 'original-report.txt') + const other = join(source, 'other.txt') + writeFileSync(other, 'different file contents') + await app?.evaluate( + (_electron, paths) => { + const fs = process.getBuiltinModule( + 'node:fs/promises' + ) as typeof import('node:fs/promises') + const original = fs.stat + fs.stat = (async (...args: Parameters) => { + const result = await original(...args) + if (args[0] === paths.file) { + fs.stat = original + await fs.rename(paths.file, paths.backup) + await fs.symlink(paths.other, paths.file) + } + return result + }) as typeof fs.stat + }, + { file, backup, other } + ) + try { + expect(await invoke({ operation: 'read', toolCallId: 'text' })).toMatchObject({ ok: false }) + } finally { + rmSync(file) + renameSync(backup, file) + rmSync(other) + } + }) + await test.step('swapping an ancestor cannot redirect directory enumeration', async () => { + const parent = join(realpathSync(source), 'parent') + const child = join(parent, 'child') + const backup = join(realpathSync(source), 'parent-backup') + const otherParent = join(outside, 'parent') + mkdirSync(child, { recursive: true }) + mkdirSync(join(otherParent, 'child'), { recursive: true }) + writeFileSync(join(child, 'allowed.txt'), 'allowed') + writeFileSync(join(otherParent, 'child', 'private.txt'), 'outside') + calls.ancestorRace = { toolName: 'read_local_file', args: { path: child } } + await app?.evaluate( + (_electron, paths) => { + const fs = process.getBuiltinModule( + 'node:fs/promises' + ) as typeof import('node:fs/promises') + const originalStat = fs.lstat + const originalRealpath = fs.realpath + let swapped = false + const restore = async () => { + fs.lstat = originalStat + fs.realpath = originalRealpath + if (swapped) { + await fs.rm(paths.parent) + await fs.rename(paths.backup, paths.parent) + swapped = false + } + } + ;( + globalThis as typeof globalThis & { restoreLocalFileRace?: () => Promise } + ).restoreLocalFileRace = restore + fs.lstat = (async (...args: Parameters) => { + const result = await originalStat(...args) + if (args[0] === paths.child) { + fs.lstat = originalStat + await fs.rename(paths.parent, paths.backup) + await fs.symlink(paths.otherParent, paths.parent) + swapped = true + } + return result + }) as typeof fs.lstat + fs.realpath = (async (...args: Parameters) => { + if (swapped && args[0] === paths.child) await restore() + return originalRealpath(...args) + }) as typeof fs.realpath + }, + { parent, child, backup, otherParent } + ) + try { + expect(await invoke({ operation: 'read', toolCallId: 'ancestorRace' })).toMatchObject({ + ok: true, + data: { entries: [{ name: 'allowed.txt', kind: 'file' }] }, + }) + } finally { + await app?.evaluate(async () => { + const runtime = globalThis as typeof globalThis & { + restoreLocalFileRace?: () => Promise + } + await runtime.restoreLocalFileRace?.() + runtime.restoreLocalFileRace = undefined + }) + rmSync(parent, { recursive: true, force: true }) + rmSync(otherParent, { recursive: true, force: true }) + } + }) + await test.step('a replaced directory cannot return a listing for its old contents', async () => { + const directory = join(realpathSync(source), 'replace-during-read') + const backup = join(realpathSync(source), 'previous-directory') + mkdirSync(directory) + writeFileSync(join(directory, 'old.txt'), 'old contents') + calls.directoryReplaced = { toolName: 'read_local_file', args: { path: directory } } + await app?.evaluate( + (_electron, paths) => { + const fs = process.getBuiltinModule( + 'node:fs/promises' + ) as typeof import('node:fs/promises') + const original = fs.lstat + fs.lstat = (async (...args: Parameters) => { + if (args[0] === paths.directory) { + fs.lstat = original + await fs.rename(paths.directory, paths.backup) + await fs.mkdir(paths.directory) + await fs.writeFile(`${paths.directory}/new.txt`, 'new contents') + } + return original(...args) + }) as typeof fs.lstat + }, + { directory, backup } + ) + try { + expect(await invoke({ operation: 'read', toolCallId: 'directoryReplaced' })).toMatchObject({ + ok: false, + }) + } finally { + rmSync(directory, { recursive: true, force: true }) + rmSync(backup, { recursive: true, force: true }) + } + }) + await test.step('cancelling a directory read prevents its contents from returning', async () => { + const path = realpathSync(join(source, 'empty')) + calls.directoryCancel = { toolName: 'read_local_file', args: { path } } + await app?.evaluate((_electron, path) => { + const fs = process.getBuiltinModule('node:fs/promises') as typeof import('node:fs/promises') + const original = fs.lstat + fs.lstat = (async (...args: Parameters) => { + const handle = await original(...args) + if (args[0] === path) { + fs.lstat = original + await new Promise((resolve) => { + ;( + globalThis as typeof globalThis & { releaseLocalFileRead?: () => void } + ).releaseLocalFileRead = resolve + }) + } + return handle + }) as typeof fs.lstat + }, path) + const reading = invoke({ operation: 'read', toolCallId: 'directoryCancel' }) + void reading.catch(() => {}) + try { + await expect + .poll(() => + app?.evaluate( + () => + typeof (globalThis as typeof globalThis & { releaseLocalFileRead?: () => void }) + .releaseLocalFileRead === 'function' + ) + ) + .toBe(true) + await invoke({ operation: 'cancel', toolCallId: 'directoryCancel' }) + } finally { + await app?.evaluate(() => { + const runtime = globalThis as typeof globalThis & { releaseLocalFileRead?: () => void } + runtime.releaseLocalFileRead?.() + runtime.releaseLocalFileRead = undefined + }) + } + expect(await reading).toMatchObject({ ok: false }) + }) + await test.step('cancelling in the renderer closes consent without remembering access', async () => { + calls.cancelled = { + toolName: 'read_local_file', + args: { path: join(outside, 'private.txt') }, + } + const cancelled = await requestPermission({ operation: 'read', toolCallId: 'cancelled' }) + const closed = cancelled.prompt.waitForEvent('close', { timeout: 5000 }) + await window.evaluate(async () => { + const api = (globalThis as typeof globalThis & { simDesktop: SimDesktopApi }).simDesktop + await api.localFiles?.({ operation: 'cancel', toolCallId: 'cancelled' }) + }) + await closed + expect(await cancelled.result).toMatchObject({ ok: false }) + const again = await requestPermission({ operation: 'read', toolCallId: 'cancelled' }) + await again.prompt + .getByRole('button', { name: "Don't allow", exact: true }) + .click({ noWaitAfter: true }) + expect(await again.result).toMatchObject({ ok: false }) + }) + await test.step('consent escapes direction controls in folder names', async () => { + const folder = join(root, 'Bidi\u061c\u200e\u200f') + mkdirSync(folder) + calls.bidi = { toolName: 'read_local_file', args: { path: folder } } + const bidi = await requestPermission({ operation: 'read', toolCallId: 'bidi' }) + await expect(bidi.prompt.getByRole('dialog')).toContainText('Bidi\\u061c\\u200e\\u200f') + await bidi.prompt + .getByRole('button', { name: "Don't allow", exact: true }) + .click({ noWaitAfter: true }) + expect(await bidi.result).toMatchObject({ ok: false }) + }) + await test.step('an unanswered prompt does not block approved folders', async () => { + calls.blocker = { toolName: 'read_local_file', args: { path: join(outside, 'private.txt') } } + const blocker = await requestPermission({ operation: 'read', toolCallId: 'blocker' }) + expect(await invoke({ operation: 'read', toolCallId: 'text' })).toMatchObject({ ok: true }) + await blocker.prompt + .getByRole('button', { name: "Don't allow", exact: true }) + .click({ noWaitAfter: true }) + expect(await blocker.result).toMatchObject({ ok: false }) + }) + await test.step('cancelled calls cannot reuse an approved folder', async () => { + calls.expired = { toolName: 'read_local_file', args: { path: join(source, 'report.txt') } } + expireAfterAuthorization.add('expired') + expect(await invoke({ operation: 'read', toolCallId: 'expired' })).toMatchObject({ + ok: false, + }) + }) + await test.step('replacing the proposed folder during consent does not expose its new target', async () => { + const proposed = join(root, 'Proposed') + mkdirSync(proposed) + calls.retargeted = { toolName: 'read_local_file', args: { path: proposed } } + const retargeted = await requestPermission({ operation: 'read', toolCallId: 'retargeted' }) + renameSync(proposed, join(root, 'Original')) + mkdirSync(proposed) + writeFileSync(join(proposed, 'unapproved.txt'), 'replacement folder contents') + await retargeted.prompt + .getByRole('button', { name: 'Allow folder', exact: true }) + .click({ noWaitAfter: true }) + expect(await retargeted.result).toMatchObject({ ok: false }) + }) const result = await invoke({ operation: 'manifest', toolCallId: 'import' }) if (!result.ok || result.data.kind !== 'manifest') throw new Error(JSON.stringify(result)) expect(result.data.targetWorkspaceId).toBe('target-workspace') @@ -141,6 +876,142 @@ test('native file tools read and import through the installed preload without Si ok: false, code: 'ALREADY_STARTED', }) + await test.step('an approved folder permits imports without repeated destination prompts', async () => { + calls.otherImport = { + toolName: 'import_local_files', + args: { path: source, targetWorkspaceId: 'other-workspace' }, + } + expect(await invoke({ operation: 'manifest', toolCallId: 'otherImport' })).toMatchObject({ + ok: true, + }) + }) + await test.step('folder permissions survive restarting the desktop app', async () => { + await app?.close() + app = await launch() + window = await app.firstWindow() + await expect(window.getByRole('heading', { name: 'Local files', exact: true })).toHaveText( + 'Local files' + ) + expect(await invoke({ operation: 'read', toolCallId: 'text' })).toMatchObject({ ok: true }) + }) + await test.step('forgetting a folder revokes native reads and survives restart', async () => { + await window.getByRole('button', { name: 'Forget folders', exact: true }).click() + await expect(window.getByRole('button', { name: 'Forgotten', exact: true })).toBeVisible() + await app?.close() + app = await launch() + window = await app.firstWindow() + await expect(window.getByRole('heading', { name: 'Local files', exact: true })).toHaveText( + 'Local files' + ) + const revoked = await requestPermission({ operation: 'read', toolCallId: 'text' }) + await revoked.prompt + .getByRole('button', { name: 'Allow folder', exact: true }) + .click({ noWaitAfter: true }) + expect(await revoked.result).toMatchObject({ ok: true }) + }) + + await test.step('a remembered grant does not follow a replaced folder after restart', async () => { + await app?.close() + renameSync(source, join(root, 'Original-reports')) + mkdirSync(source) + writeFileSync(join(source, 'report.txt'), 'replacement contents') + app = await launch() + window = await app.firstWindow() + await expect(window.getByRole('heading', { name: 'Local files', exact: true })).toHaveText( + 'Local files' + ) + const replaced = await requestPermission({ operation: 'read', toolCallId: 'text' }) + await replaced.prompt + .getByRole('button', { name: "Don't allow", exact: true }) + .click({ noWaitAfter: true }) + expect(await replaced.result).toMatchObject({ ok: false }) + rmSync(source, { recursive: true }) + renameSync(join(root, 'Original-reports'), source) + const restored = await requestPermission({ operation: 'read', toolCallId: 'text' }) + await restored.prompt + .getByRole('button', { name: 'Allow folder', exact: true }) + .click({ noWaitAfter: true }) + expect(await restored.result).toMatchObject({ ok: true }) + }) + + await test.step('sign-out revokes remembered grants and Full file access before the next account session', async () => { + await window.getByRole('switch', { name: 'Full file access', exact: true }).click() + await expect( + window.getByRole('switch', { name: 'Full file access', exact: true }) + ).toBeChecked() + await app?.evaluate(({ Menu }) => { + const item = Menu.getApplicationMenu() + ?.items.flatMap((entry) => entry.submenu?.items ?? []) + .find((entry) => entry.label === 'Sign Out') + if (!item) throw new Error('Sign Out menu item missing') + item.click() + }) + await expect(window).toHaveURL(`http://127.0.0.1:${address.port}/login`) + signedIn = true + await window.goto(`http://127.0.0.1:${address.port}/`) + await expect + .poll(() => + window.evaluate(async () => { + const api = (globalThis as typeof globalThis & { simDesktop: SimDesktopApi }).simDesktop + return (await api.localFilesystem?.({ operation: 'list_mounts' }))?.ok + }) + ) + .toBe(true) + await expect + .poll(() => + window.evaluate(async () => + ( + globalThis as typeof globalThis & { simDesktop: SimDesktopApi } + ).simDesktop.settings.getPreferences() + ) + ) + .toMatchObject({ fullFileAccess: false }) + const revoked = await requestPermission({ operation: 'read', toolCallId: 'text' }) + await revoked.prompt + .getByRole('button', { name: "Don't allow", exact: true }) + .click({ noWaitAfter: true }) + expect(await revoked.result).toMatchObject({ ok: false }) + }) + await test.step('a failed settings write reports the error and leaves Full file access disabled', async () => { + const settingsPath = join(root, 'profile', 'settings.json') + const saved = JSON.parse(readFileSync(settingsPath, 'utf8')) + for (const previous of [false, true]) { + await app?.close() + rmSync(settingsPath, { recursive: true, force: true }) + writeFileSync(settingsPath, JSON.stringify({ ...saved, fullFileAccess: previous })) + app = await launch() + window = await app.firstWindow() + const toggle = window.getByRole('switch', { name: 'Full file access', exact: true }) + await expect(toggle).toBeChecked({ checked: previous }) + renameSync(settingsPath, `${settingsPath}.backup`) + mkdirSync(settingsPath) + try { + await toggle.click() + await expect( + window.getByText( + 'Could not save file access settings. Your previous setting may return after restarting Sim.', + { exact: true } + ) + ).toBeVisible() + await expect(toggle).not.toBeChecked() + expect( + await window.evaluate(async () => + ( + globalThis as typeof globalThis & { simDesktop: SimDesktopApi } + ).simDesktop.settings.getPreferences() + ) + ).toMatchObject({ fullFileAccess: false }) + expect(await invoke({ operation: 'read', toolCallId: 'text' })).toMatchObject({ + ok: false, + }) + } finally { + await app.close() + app = undefined + rmSync(settingsPath, { recursive: true, force: true }) + renameSync(`${settingsPath}.backup`, settingsPath) + } + } + }) } finally { await app?.close() server?.close() diff --git a/apps/desktop/e2e/smoke.spec.ts b/apps/desktop/e2e/smoke.spec.ts index 08ace958c77..7867c67d108 100644 --- a/apps/desktop/e2e/smoke.spec.ts +++ b/apps/desktop/e2e/smoke.spec.ts @@ -138,6 +138,7 @@ test.describe('desktop shell smoke', () => { test('OAuth popups share the session without inheriting the privileged preload', async () => { app = await launchApp(origin) const window = await app.firstWindow() + await window.waitForURL(`${origin}/home`, { waitUntil: 'load' }) await window.evaluate(() => { document.cookie = 'sim-e2e-session=shared; Path=/; SameSite=Lax' }) @@ -319,7 +320,7 @@ test.describe('desktop shell smoke', () => { await window.locator('#server').click() const picker = await pickerPromise - expect(picker.url()).toBe('sim-shell://pages/server.html') + await expect(picker).toHaveURL('sim-shell://pages/server.html') await expect(picker.getByRole('dialog', { name: 'Sim server', exact: true })).toBeVisible() await expect(picker.getByLabel('Server URL')).toHaveValue('http://127.0.0.1:1') await expect(picker.getByLabel('Server URL')).toBeFocused() diff --git a/apps/desktop/native/directory.cc b/apps/desktop/native/directory.cc new file mode 100644 index 00000000000..3cf0d536ebb --- /dev/null +++ b/apps/desktop/native/directory.cc @@ -0,0 +1,328 @@ +#include + +#include +#include +#include +#include + +#include +#include +#include +#include +#include +#include +#include + +struct Entry { + std::string name; + const char* kind; +}; + +struct DirectoryRead { + napi_async_work work = nullptr; + napi_deferred deferred = nullptr; + int descriptor = -1; + size_t limit = 0; + bool truncated = false; + std::string error; + std::vector entries; +}; + +static const char* EntryKind(DIR* directory, const dirent* entry) { + switch (entry->d_type) { + case DT_REG: return "file"; + case DT_DIR: return "directory"; + case DT_LNK: return "symlink"; + case DT_UNKNOWN: { + struct stat metadata; + if (fstatat(dirfd(directory), entry->d_name, &metadata, AT_SYMLINK_NOFOLLOW) == 0) { + if (S_ISREG(metadata.st_mode)) return "file"; + if (S_ISDIR(metadata.st_mode)) return "directory"; + if (S_ISLNK(metadata.st_mode)) return "symlink"; + } + return "other"; + } + default: return "other"; + } +} + +static void ReadEntries(napi_env, void* data) { + auto* read = static_cast(data); + DIR* directory = fdopendir(read->descriptor); + if (!directory) { + close(read->descriptor); + read->descriptor = -1; + read->error = "Could not enumerate the approved directory."; + return; + } + read->descriptor = -1; + while (true) { + errno = 0; + const dirent* entry = readdir(directory); + if (!entry) { + if (errno != 0) read->error = "Could not finish reading the approved directory."; + break; + } + if (strcmp(entry->d_name, ".") == 0 || strcmp(entry->d_name, "..") == 0) continue; + if (read->entries.size() == read->limit) { + read->truncated = true; + break; + } + read->entries.push_back({entry->d_name, EntryKind(directory, entry)}); + } + closedir(directory); +} + +static napi_value String(napi_env env, const char* value) { + napi_value result; + napi_create_string_utf8(env, value, NAPI_AUTO_LENGTH, &result); + return result; +} + +static void Complete(napi_env env, napi_status status, void* data) { + auto* read = static_cast(data); + if (read->descriptor >= 0) close(read->descriptor); + if (status != napi_ok || !read->error.empty()) { + napi_value error; + napi_create_error(env, nullptr, + String(env, read->error.empty() ? "Directory read cancelled." : read->error.c_str()), + &error); + napi_reject_deferred(env, read->deferred, error); + } else { + napi_value result; + napi_value entries; + napi_value truncated; + napi_create_object(env, &result); + napi_create_array_with_length(env, read->entries.size(), &entries); + for (size_t index = 0; index < read->entries.size(); index++) { + napi_value entry; + napi_create_object(env, &entry); + napi_set_named_property(env, entry, "name", String(env, read->entries[index].name.c_str())); + napi_set_named_property(env, entry, "kind", String(env, read->entries[index].kind)); + napi_set_element(env, entries, index, entry); + } + napi_get_boolean(env, read->truncated, &truncated); + napi_set_named_property(env, result, "entries", entries); + napi_set_named_property(env, result, "truncated", truncated); + napi_resolve_deferred(env, read->deferred, result); + } + napi_delete_async_work(env, read->work); + delete read; +} + +/** The descriptor is duplicated before scheduling; no pathname is reopened by the worker. */ +static napi_value ReadDirectory(napi_env env, napi_callback_info info) { + size_t count = 2; + napi_value arguments[2]; + double descriptor = -1; + double limit = 0; + if (napi_get_cb_info(env, info, &count, arguments, nullptr, nullptr) != napi_ok || count != 2 || + napi_get_value_double(env, arguments[0], &descriptor) != napi_ok || + napi_get_value_double(env, arguments[1], &limit) != napi_ok || + !std::isfinite(descriptor) || descriptor < 0 || descriptor > INT_MAX || + descriptor != std::floor(descriptor) || limit < 1 || limit > 1000 || limit != std::floor(limit)) { + napi_throw_type_error(env, nullptr, "Expected a directory descriptor and an entry limit from 1 to 1000."); + return nullptr; + } + auto* read = new DirectoryRead(); + read->limit = static_cast(limit); + read->descriptor = fcntl(static_cast(descriptor), F_DUPFD_CLOEXEC, 0); + if (read->descriptor < 0) { + delete read; + napi_throw_error(env, nullptr, "Could not retain the approved directory descriptor."); + return nullptr; + } + napi_value promise; + if (napi_create_promise(env, &read->deferred, &promise) != napi_ok || + napi_create_async_work(env, nullptr, String(env, "ReadApprovedDirectory"), ReadEntries, + Complete, read, &read->work) != napi_ok || + napi_queue_async_work(env, read->work) != napi_ok) { + close(read->descriptor); + if (read->work) napi_delete_async_work(env, read->work); + delete read; + napi_throw_error(env, nullptr, "Could not schedule the directory read."); + return nullptr; + } + return promise; +} + +struct ApprovedOpen { + napi_async_work work = nullptr; + napi_deferred deferred = nullptr; + std::string root; + std::string relative; + uint64_t dev = 0; + uint64_t ino = 0; + bool directory = false; + int descriptor = -1; + std::string error; +}; + +static void OpenApprovedPath(napi_env, void* data) { + auto* request = static_cast(data); + int descriptor = open(request->root.c_str(), O_RDONLY | O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC); + struct stat metadata; + if (descriptor < 0 || fstat(descriptor, &metadata) != 0 || + static_cast(metadata.st_dev) != request->dev || + static_cast(metadata.st_ino) != request->ino) { + if (descriptor >= 0) close(descriptor); + request->error = "The approved folder changed. Request access again."; + return; + } + size_t start = 0; + while (start < request->relative.size()) { + const size_t end = request->relative.find('/', start); + const bool last = end == std::string::npos; + const std::string component = request->relative.substr(start, last ? end : end - start); + if (component.empty() || component == "." || component == "..") { + close(descriptor); + request->error = "The path must stay within the approved folder."; + return; + } + const int next = openat(descriptor, component.c_str(), + O_RDONLY | O_NOFOLLOW | O_CLOEXEC | O_NONBLOCK | + ((!last || request->directory) ? O_DIRECTORY : 0)); + close(descriptor); + if (next < 0) { + request->error = "Could not open the path within the approved folder."; + return; + } + descriptor = next; + if (last) break; + start = end + 1; + } + if (fstat(descriptor, &metadata) != 0 || + (request->directory ? !S_ISDIR(metadata.st_mode) : !S_ISREG(metadata.st_mode))) { + close(descriptor); + request->error = "The approved path is not a regular file or directory."; + return; + } + request->descriptor = descriptor; +} + +static void CompleteOpen(napi_env env, napi_status status, void* data) { + auto* request = static_cast(data); + if (status != napi_ok || !request->error.empty()) { + if (request->descriptor >= 0) close(request->descriptor); + napi_value error; + napi_create_error(env, nullptr, String(env, request->error.empty() + ? "File open cancelled." : request->error.c_str()), &error); + napi_reject_deferred(env, request->deferred, error); + } else { + napi_value descriptor; + napi_create_int32(env, request->descriptor, &descriptor); + napi_resolve_deferred(env, request->deferred, descriptor); + } + napi_delete_async_work(env, request->work); + delete request; +} + +static bool ReadPath(napi_env env, napi_value value, std::string& path) { + size_t length = 0; + if (napi_get_value_string_utf8(env, value, nullptr, 0, &length) != napi_ok || length > 4096) + return false; + std::vector buffer(length + 1); + if (napi_get_value_string_utf8(env, value, buffer.data(), buffer.size(), &length) != napi_ok) + return false; + path.assign(buffer.data(), length); + return path.find('\0') == std::string::npos; +} + +/** Opens each component relative to the verified grant descriptor; no ancestor is followed. */ +static napi_value OpenApproved(napi_env env, napi_callback_info info) { + size_t count = 5; + napi_value arguments[5]; + auto* request = new ApprovedOpen(); + bool devLossless = false; + bool inoLossless = false; + if (napi_get_cb_info(env, info, &count, arguments, nullptr, nullptr) != napi_ok || count != 5 || + !ReadPath(env, arguments[0], request->root) || request->root.empty() || request->root[0] != '/' || + !ReadPath(env, arguments[1], request->relative) || + (!request->relative.empty() && (request->relative.front() == '/' || request->relative.back() == '/')) || + napi_get_value_bigint_uint64(env, arguments[2], &request->dev, &devLossless) != napi_ok || + napi_get_value_bigint_uint64(env, arguments[3], &request->ino, &inoLossless) != napi_ok || + !devLossless || !inoLossless || + napi_get_value_bool(env, arguments[4], &request->directory) != napi_ok) { + delete request; + napi_throw_type_error(env, nullptr, "Expected a granted root, relative path, identity, and path kind."); + return nullptr; + } + napi_value promise; + if (napi_create_promise(env, &request->deferred, &promise) != napi_ok || + napi_create_async_work(env, nullptr, String(env, "OpenApprovedPath"), OpenApprovedPath, + CompleteOpen, request, &request->work) != napi_ok || + napi_queue_async_work(env, request->work) != napi_ok) { + if (request->work) napi_delete_async_work(env, request->work); + delete request; + napi_throw_error(env, nullptr, "Could not schedule the approved file open."); + return nullptr; + } + return promise; +} + +struct DescriptorClose { + napi_async_work work = nullptr; + napi_deferred deferred = nullptr; + int descriptor = -1; + bool failed = false; +}; + +static void CloseDescriptor(napi_env, void* data) { + auto* request = static_cast(data); + request->failed = close(request->descriptor) != 0; + request->descriptor = -1; +} + +static void CompleteClose(napi_env env, napi_status status, void* data) { + auto* request = static_cast(data); + if (request->descriptor >= 0) CloseDescriptor(env, request); + if (status != napi_ok || request->failed) { + napi_value error; + napi_create_error(env, nullptr, String(env, "Could not close the approved file."), &error); + napi_reject_deferred(env, request->deferred, error); + } else { + napi_value result; + napi_get_undefined(env, &result); + napi_resolve_deferred(env, request->deferred, result); + } + napi_delete_async_work(env, request->work); + delete request; +} + +/** Native opens retain native ownership through close, including inside Node workers. */ +static napi_value CloseFile(napi_env env, napi_callback_info info) { + size_t count = 1; + napi_value argument; + double descriptor = -1; + if (napi_get_cb_info(env, info, &count, &argument, nullptr, nullptr) != napi_ok || count != 1 || + napi_get_value_double(env, argument, &descriptor) != napi_ok || + !std::isfinite(descriptor) || descriptor < 0 || descriptor > INT_MAX || + descriptor != std::floor(descriptor)) { + napi_throw_type_error(env, nullptr, "Expected an approved file descriptor."); + return nullptr; + } + auto* request = new DescriptorClose(); + request->descriptor = static_cast(descriptor); + napi_value promise; + if (napi_create_promise(env, &request->deferred, &promise) != napi_ok || + napi_create_async_work(env, nullptr, String(env, "CloseApprovedFile"), CloseDescriptor, + CompleteClose, request, &request->work) != napi_ok || + napi_queue_async_work(env, request->work) != napi_ok) { + close(request->descriptor); + if (request->work) napi_delete_async_work(env, request->work); + delete request; + napi_throw_error(env, nullptr, "Could not schedule the approved file close."); + return nullptr; + } + return promise; +} + +NAPI_MODULE_INIT() { + napi_property_descriptor properties[] = { + {"readDirectory", nullptr, ReadDirectory, nullptr, nullptr, nullptr, napi_default, nullptr}, + {"openApproved", nullptr, OpenApproved, nullptr, nullptr, nullptr, napi_default, nullptr}, + {"closeFile", nullptr, CloseFile, nullptr, nullptr, nullptr, napi_default, nullptr}, + }; + napi_define_properties(env, exports, 3, properties); + return exports; +} diff --git a/apps/desktop/package.json b/apps/desktop/package.json index 3a6dbd01c5e..f52bcfe37de 100644 --- a/apps/desktop/package.json +++ b/apps/desktop/package.json @@ -25,8 +25,8 @@ "lint:check": "biome check .", "format": "biome format --write .", "format:check": "biome format .", - "test": "vitest run", - "test:watch": "vitest", + "test": "bun run scripts/build-native.ts && vitest run", + "test:watch": "bun run scripts/build-native.ts && vitest", "test:e2e": "playwright test" }, "dependencies": { diff --git a/apps/desktop/scripts/build-native.ts b/apps/desktop/scripts/build-native.ts new file mode 100644 index 00000000000..8980683f31f --- /dev/null +++ b/apps/desktop/scripts/build-native.ts @@ -0,0 +1,62 @@ +import { execFileSync } from 'node:child_process' +import { existsSync, mkdirSync, statSync } from 'node:fs' +import { dirname, join } from 'node:path' +import { createLogger } from '@sim/logger' + +const logger = createLogger('DesktopNativeBuild') +if (process.platform !== 'darwin' && process.platform !== 'linux') { + throw new Error('Native desktop modules require macOS or Linux.') +} +const nodeExecutable = execFileSync('node', ['-p', 'process.execPath'], { encoding: 'utf8' }).trim() +const includeDirectory = join(dirname(nodeExecutable), '..', 'include', 'node') +if (!existsSync(join(includeDirectory, 'node_api.h'))) { + throw new Error(`Could not find Node-API headers in ${includeDirectory}`) +} +const outputDirectory = 'dist/native' +mkdirSync(outputDirectory, { recursive: true }) +const modules = [ + { name: 'directory', source: 'native/directory.cc', appKit: false }, + ...(process.platform === 'darwin' + ? [{ name: 'help-search', source: 'native/help-search.mm', appKit: true }] + : []), +] +for (const module of modules) { + const output = join(outputDirectory, `${module.name}.node`) + if ( + existsSync(output) && + statSync(output).mtimeMs >= + Math.max(statSync(module.source).mtimeMs, statSync(import.meta.filename).mtimeMs) + ) + continue + const macOS = process.platform === 'darwin' + execFileSync( + macOS ? 'xcrun' : 'c++', + [ + ...(macOS ? ['clang++'] : []), + '-std=c++17', + '-DNAPI_VERSION=8', + ...(macOS + ? [ + '-bundle', + '-undefined', + 'dynamic_lookup', + '-mmacosx-version-min=12.0', + '-arch', + 'arm64', + '-arch', + 'x86_64', + ] + : ['-shared', '-fPIC']), + '-I', + includeDirectory, + ...(module.appKit + ? ['-fobjc-arc', '-fblocks', '-framework', 'AppKit', '-framework', 'Foundation'] + : []), + '-o', + output, + module.source, + ], + { stdio: 'inherit' } + ) + logger.info('Compiled native desktop module', { module: module.name }) +} diff --git a/apps/desktop/scripts/build.ts b/apps/desktop/scripts/build.ts index b15dc2fb354..59527b6c616 100644 --- a/apps/desktop/scripts/build.ts +++ b/apps/desktop/scripts/build.ts @@ -1,6 +1,6 @@ import { execFileSync } from 'node:child_process' -import { cpSync, existsSync, mkdirSync, readFileSync, rmSync } from 'node:fs' -import { dirname, join, resolve } from 'node:path' +import { cpSync, readFileSync, rmSync } from 'node:fs' +import { dirname, resolve } from 'node:path' import { type BuildOptions, build } from 'esbuild' import postcss from 'postcss' import loadPostcssConfig from 'postcss-load-config' @@ -34,52 +34,6 @@ rmSync(generatedIcon, { force: true, recursive: true }) cpSync(appIcon, generatedIcon, { recursive: true }) console.log(`• Selecting desktop icon: ${appIcon}`) -function compileNativeHelpSearch(): void { - const outputDirectory = 'dist/native' - rmSync(outputDirectory, { force: true, recursive: true }) - if (process.platform !== 'darwin') return - - const nodeExecutable = execFileSync('node', ['-p', 'process.execPath'], { - encoding: 'utf8', - }).trim() - const nodeIncludeDirectory = join(dirname(nodeExecutable), '..', 'include', 'node') - const nodeApiHeader = join(nodeIncludeDirectory, 'node_api.h') - if (!existsSync(nodeApiHeader)) { - throw new Error(`Could not find Node-API headers at ${nodeApiHeader}`) - } - - mkdirSync(outputDirectory, { recursive: true }) - execFileSync( - 'xcrun', - [ - 'clang++', - '-std=c++17', - '-DNAPI_VERSION=8', - '-fobjc-arc', - '-fblocks', - '-bundle', - '-undefined', - 'dynamic_lookup', - '-mmacosx-version-min=12.0', - '-arch', - 'arm64', - '-arch', - 'x86_64', - '-I', - nodeIncludeDirectory, - '-framework', - 'AppKit', - '-framework', - 'Foundation', - '-o', - join(outputDirectory, 'help-search.node'), - 'native/help-search.mm', - ], - { stdio: 'inherit' } - ) - console.log('• Compiled native macOS documentation Help search') -} - const common = { bundle: true, platform: 'node' as const, @@ -138,7 +92,7 @@ const renderer: BuildOptions = { } async function run(): Promise { - compileNativeHelpSearch() + execFileSync(process.execPath, ['run', 'scripts/build-native.ts'], { stdio: 'inherit' }) if (watch) { const { context } = await import('esbuild') const rendererCtx = await context(renderer) diff --git a/apps/desktop/src/main/browser-agent/cdp.test.ts b/apps/desktop/src/main/browser-agent/cdp.test.ts index 821f9b956d4..a62595d7ee8 100644 --- a/apps/desktop/src/main/browser-agent/cdp.test.ts +++ b/apps/desktop/src/main/browser-agent/cdp.test.ts @@ -61,11 +61,22 @@ function createOopifFrameFixture() { } } +/** Callbacks for a page with no user to ask, so the shell answers every dialog. */ +const shellAnswersDialogs = { + claimUserDialog: () => false, + onDialogClosed: () => {}, + claimUserLeave: () => false, +} + describe('browser-agent CDP instrumentation', () => { it('leaves file chooser dialogs native so users can upload files', async () => { const contents = new WebContentsView().webContents - await ensureInstrumented(contents, { onDialog: vi.fn(), dialogResponse: () => null }) + await ensureInstrumented(contents, { + onDialog: vi.fn(), + dialogResponse: () => null, + ...shellAnswersDialogs, + }) expect(contents.debugger.sendCommand).toHaveBeenCalledWith('Page.enable', undefined) expect(contents.debugger.sendCommand).not.toHaveBeenCalledWith( @@ -86,10 +97,18 @@ describe('browser-agent CDP instrumentation', () => { }) await expect( - ensureInstrumented(contents, { onDialog: vi.fn(), dialogResponse: () => null }) + ensureInstrumented(contents, { + onDialog: vi.fn(), + dialogResponse: () => null, + ...shellAnswersDialogs, + }) ).rejects.toThrow('setup acknowledgement lost') await expect( - ensureInstrumented(contents, { onDialog: vi.fn(), dialogResponse: () => null }) + ensureInstrumented(contents, { + onDialog: vi.fn(), + dialogResponse: () => null, + ...shellAnswersDialogs, + }) ).resolves.toBeUndefined() expect(autoAttachAttempts).toBe(2) @@ -98,7 +117,11 @@ describe('browser-agent CDP instrumentation', () => { it('dismisses an OOPIF dialog on the flattened child session', async () => { const contents = new WebContentsView().webContents const onDialog = vi.fn() - await ensureInstrumented(contents, { onDialog, dialogResponse: () => null }) + await ensureInstrumented(contents, { + onDialog, + dialogResponse: () => null, + ...shellAnswersDialogs, + }) const listener = vi .mocked(contents.debugger.on) .mock.calls.find(([event]) => event === 'message')?.[1] as @@ -132,7 +155,7 @@ describe('browser-agent CDP instrumentation', () => { const contents = new WebContentsView().webContents const onDialog = vi.fn() const dialogResponse = vi.fn(() => ({ accept: true })) - await ensureInstrumented(contents, { onDialog, dialogResponse }) + await ensureInstrumented(contents, { onDialog, dialogResponse, ...shellAnswersDialogs }) const listener = vi .mocked(contents.debugger.on) .mock.calls.find(([event]) => event === 'message')?.[1] as @@ -275,7 +298,11 @@ describe('browser-agent CDP instrumentation', () => { async (treeKind) => { const contents = new WebContentsView().webContents const { child, frameTree } = createOopifFrameFixture() - await ensureInstrumented(contents, { onDialog: vi.fn(), dialogResponse: () => null }) + await ensureInstrumented(contents, { + onDialog: vi.fn(), + dialogResponse: () => null, + ...shellAnswersDialogs, + }) const listener = vi .mocked(contents.debugger.on) .mock.calls.find(([event]) => event === 'message')?.[1] as @@ -378,7 +405,11 @@ describe('browser-agent CDP instrumentation', () => { it('falls back to the root target when OOPIF isolated-world creation fails', async () => { const contents = new WebContentsView().webContents const { child, frameTree } = createOopifFrameFixture() - await ensureInstrumented(contents, { onDialog: vi.fn(), dialogResponse: () => null }) + await ensureInstrumented(contents, { + onDialog: vi.fn(), + dialogResponse: () => null, + ...shellAnswersDialogs, + }) const listener = vi .mocked(contents.debugger.on) .mock.calls.find(([event]) => event === 'message')?.[1] as @@ -458,7 +489,11 @@ describe('browser-agent file input handles', () => { async function fileInputFixture(childSession = false) { const contents = new WebContentsView().webContents const { child, frameTree } = createOopifFrameFixture() - await ensureInstrumented(contents, { onDialog: vi.fn(), dialogResponse: () => null }) + await ensureInstrumented(contents, { + onDialog: vi.fn(), + dialogResponse: () => null, + ...shellAnswersDialogs, + }) if (childSession) { const onMessage = vi.mocked(contents.debugger.on).mock.calls[0]?.[1] as | ((event: unknown, method: string, params: unknown, sessionId?: string) => void) diff --git a/apps/desktop/src/main/browser-agent/cdp.ts b/apps/desktop/src/main/browser-agent/cdp.ts index 6bf9f9fbc2d..f7c77d69e74 100644 --- a/apps/desktop/src/main/browser-agent/cdp.ts +++ b/apps/desktop/src/main/browser-agent/cdp.ts @@ -13,6 +13,7 @@ import { createLogger } from '@sim/logger' import { getErrorMessage } from '@sim/utils/errors' import { interruptibleSleep } from '@sim/utils/helpers' import { isRecordLike } from '@sim/utils/object' +import { truncateAtCodePoint } from '@sim/utils/string' import type { NativeImage, WebContents, WebFrameMain } from 'electron' const logger = createLogger('BrowserAgentCdp') @@ -38,8 +39,14 @@ export interface DialogResponse { export interface CdpCallbacks { /** A JS dialog was handled; the driver surfaces it to the model. */ onDialog: (dialog: PageDialog) => void - /** The running action's requested answer; dialogs are dismissed when it has none. */ + /** The running action's requested answer; defaults to declining the dialog. */ dialogResponse: () => DialogResponse | null + /** Leaves a visible user-owned alert or confirm to Electron's native dialog. */ + claimUserDialog: () => boolean + /** The dialog ended through a user answer, a CDP answer, or page teardown. */ + onDialogClosed: () => void + /** True when the user, not the shell, decides this beforeunload. */ + claimUserLeave: () => boolean } /** Per-tab callbacks, so a background tab's events reach ITS driver, not the @@ -189,37 +196,55 @@ function handleDebuggerEvent( return } const callbacks = callbacksByContents.get(contents) + if (method === 'Page.javascriptDialogClosed') { + callbacks?.onDialogClosed() + return + } if (method === 'Page.javascriptDialogOpening') { const type = String(params.type ?? 'dialog') - const message = String(params.message ?? '').slice(0, 500) - // Dialogs never stay open: beforeunload is accepted (navigation proceeds), - // and alert/confirm follow the running action's requested answer, defaulting - // to dismissal so an unexpected dialog can never block the page. - const accept = type === 'beforeunload' || callbacks?.dialogResponse()?.accept === true - const answer = { accept } - void (async () => { - let handled = false - try { - await send(contents, 'Page.handleJavaScriptDialog', answer, parentSessionId) - handled = true - } catch { - // Some Chromium builds surface an OOPIF's tab-modal dialog on its - // flattened session but accept the answer only on the root target. - if (parentSessionId) { - try { - await send(contents, 'Page.handleJavaScriptDialog', answer) - handled = true - } catch {} - } - } + const rawMessage = String(params.message ?? '') + const message = truncateAtCodePoint(rawMessage, 500, '') + const requested = callbacks?.dialogResponse() ?? null + if (!requested && (type === 'alert' || type === 'confirm') && callbacks?.claimUserDialog()) { + return + } + if (type === 'beforeunload' && callbacks?.claimUserLeave()) { + // The user's Leave replays the navigation; this unload stays cancelled. + void answerDialog(contents, { accept: false }, parentSessionId) + return + } + // CDP unblocks JavaScript, but Electron may retain the native dialog until navigation. + const accept = type === 'beforeunload' || requested?.accept === true + void answerDialog(contents, { accept }, parentSessionId).then((handled) => { if (handled) logger.info('Handled page dialog', { type, accept }) else logger.warn('Could not handle page dialog', { type }) callbacks?.onDialog({ type, message, handled, accepted: handled && accept }) - })() + }) return } } +async function answerDialog( + contents: WebContents, + answer: { accept: boolean }, + parentSessionId: string | undefined +): Promise { + try { + await send(contents, 'Page.handleJavaScriptDialog', answer, parentSessionId) + return true + } catch { + // Some Chromium builds surface an OOPIF's tab-modal dialog on its + // flattened session but accept the answer only on the root target. + if (!parentSessionId) return false + try { + await send(contents, 'Page.handleJavaScriptDialog', answer) + return true + } catch { + return false + } + } +} + interface ProtocolFrame { id: string parentId?: string diff --git a/apps/desktop/src/main/browser-agent/driver.ts b/apps/desktop/src/main/browser-agent/driver.ts index 8e6922a6a7b..a2f98b763ce 100644 --- a/apps/desktop/src/main/browser-agent/driver.ts +++ b/apps/desktop/src/main/browser-agent/driver.ts @@ -554,6 +554,7 @@ function recordNotice(notice: string): void { function pageStateFor(contents: WebContents, tabId: string): BrowserPageState { const issue = session.pageIssueForContents(contents) const mediaPermissionRequest = session.mediaPermissionRequestForContents(contents) + const dialog = session.pageDialogForContents(contents) return { scopeId: session.getBrowserScopeId(), tabId, @@ -564,6 +565,7 @@ function pageStateFor(contents: WebContents, tabId: string): BrowserPageState { canGoForward: session.canGoForward(contents), ...(issue ? { issue } : {}), ...(mediaPermissionRequest ? { mediaPermissionRequest } : {}), + ...(dialog ? { dialog } : {}), } } @@ -615,6 +617,10 @@ function instrumentTab(contents: WebContents): void { const requested = driverScopeState().dialogResponse return requested?.contents === contents ? requested.response : null }), + claimUserDialog: () => + session.withBrowserScope(scopeId, () => session.claimUserDialog(contents)), + onDialogClosed: inScope(() => session.notePageDialogClosed(contents)), + claimUserLeave: () => session.withBrowserScope(scopeId, () => session.claimUserLeave(contents)), } void (async () => { let lastError: unknown @@ -5221,6 +5227,14 @@ export async function executeTool( ) { throw new ToolError('This browser action was cancelled before it started.') } + session.withBrowserScope(resolvedScopeId, () => { + const automation = session.automationTab() + if (automation && session.hasPendingPageDialog(automation.view.webContents)) { + throw new ToolError( + 'The user is answering a dialog on this page. Wait for their answer before using it.' + ) + } + }) state.activeToolCallId = toolCallId ?? null const executionController = new AbortController() let cancelActiveExecution: () => void = () => {} @@ -5456,6 +5470,16 @@ export async function handlePanelAction( } return } + if (action.action === 'enable-page-dialogs') { + session.enablePageDialogs() + return + } + if (action.action === 'respond-dialog') { + if (typeof action.requestId === 'string' && typeof action.allowed === 'boolean') { + session.respondToPageDialog(action.requestId, action.allowed) + } + return + } if (action.action === 'respond-site-permission') { /** Older renderers can still send a response to the retired task-navigation prompt. */ return @@ -5471,8 +5495,11 @@ export async function handlePanelAction( action.url, { agentOwned: false } ) - session.prepareExplicitNavigation(contents) - void contents.loadURL(action.url).catch(() => {}) + const url = action.url + session.navigateForUser(contents, () => { + session.prepareExplicitNavigation(contents) + void contents.loadURL(url).catch(() => {}) + }) session.focusPageForUser(contents) } return @@ -5495,13 +5522,13 @@ export async function handlePanelAction( const contents = tab.view.webContents switch (action.action) { case 'reload': - session.reloadPage(contents) + session.navigateForUser(contents, () => session.reloadPage(contents)) return case 'back': - session.goBack(contents) + session.navigateForUser(contents, () => session.goBack(contents)) return case 'forward': - session.goForward(contents) + session.navigateForUser(contents, () => session.goForward(contents)) return case 'print': contents.print({ printBackground: true }) diff --git a/apps/desktop/src/main/browser-agent/panel.ts b/apps/desktop/src/main/browser-agent/panel.ts index 304e7e22ade..0a3d9a5174c 100644 --- a/apps/desktop/src/main/browser-agent/panel.ts +++ b/apps/desktop/src/main/browser-agent/panel.ts @@ -20,7 +20,6 @@ import { getErrorMessage } from '@sim/utils/errors' import type { BrowserWindow, WebContentsView } from 'electron' import { zoomPercentOf } from '@/main/browser-agent/context-menu' import type { AgentTab } from '@/main/browser-agent/session' -import { reassertTabThrottling } from '@/main/browser-agent/session' const logger = createLogger('BrowserAgentPanel') @@ -49,6 +48,8 @@ export interface PanelHost { onGeometryChanged?: () => void /** Runs after each layout that leaves the active view attached and visible. */ onViewShown?: (view: WebContentsView) => void + /** Restores a revealed view's own chat policy after its initial paint. */ + restoreTabThrottling?: (view: WebContentsView) => void } let host: PanelHost = { @@ -547,7 +548,7 @@ export function layout(): void { contents.setBackgroundThrottling(false) contents.invalidate() setTimeout(() => { - if (!contents.isDestroyed()) reassertTabThrottling() + if (!contents.isDestroyed()) host.restoreTabThrottling?.(active.view) }, 1_000) } } diff --git a/apps/desktop/src/main/browser-agent/session.ts b/apps/desktop/src/main/browser-agent/session.ts index e02e79bc899..752fca5bec9 100644 --- a/apps/desktop/src/main/browser-agent/session.ts +++ b/apps/desktop/src/main/browser-agent/session.ts @@ -9,6 +9,7 @@ import type { BrowserMediaDevice, BrowserMediaPermissionRequest, BrowserOmniboxFocusMode, + BrowserPageDialog, BrowserPageIssue, BrowserTabState, BrowserTabsState, @@ -121,6 +122,14 @@ export interface AgentTab { openerTabId?: string /** A user action asked for this page to take focus once it is on screen. */ pendingUserFocus?: boolean + /** Electron owns the native alert or confirm until CDP reports it closed. */ + nativePageDialogOpen?: boolean + /** A leave-site decision that waits on the user, and how to answer it. */ + pageDialog?: { request: BrowserPageDialog; respond: (accept: boolean) => void } + /** Replays the user's browser-chrome navigation if the page asks before unloading. */ + pendingLeave?: () => unknown + /** The user chose Leave; the replayed navigation must not ask again. */ + allowNextUnload?: boolean } interface PendingMediaPermission { @@ -287,6 +296,8 @@ interface BrowserScopeState { * is the only evidence the Browser is the shortcut target while they show. */ browserChromeFocused: boolean + /** The renderer shows page dialogs, so the user can be asked instead of the shell answering. */ + pageDialogsEnabled: boolean automationActive: boolean automationNeedsAttention: boolean /** @@ -320,6 +331,7 @@ function createBrowserScopeState(): BrowserScopeState { focusedBrowserTabId: null, focusedBrowserClearTimer: null, browserChromeFocused: false, + pageDialogsEnabled: false, automationActive: false, automationNeedsAttention: false, findingTabId: null, @@ -1071,6 +1083,10 @@ export function initSession( const scopeId = browserScopeIdForView(view) if (scopeId) withBrowserScope(scopeId, () => applyPendingUserFocus(view)) }, + restoreTabThrottling: (view) => { + const scopeId = browserScopeIdForView(view) + if (scopeId) withBrowserScope(scopeId, applyAutomationTabPolicy) + }, onViewDetached: (view) => { if (!view) return const scopeId = browserScopeIdForView(view) @@ -1870,6 +1886,130 @@ function applyPendingUserFocus(view: WebContentsView): void { view.webContents.focus() } +/** Lets this scope's renderer show page dialogs from now on. */ +export function enablePageDialogs(): void { + currentScope.pageDialogsEnabled = true +} + +/** + * Whether a dialog on this tab is the user's to answer: the renderer can show + * it, the page is on screen, and it is the user's page rather than the agent's. + * Everything else keeps the shell answering, so the user is never asked about + * work they did not start and a hidden page is never left blocked. + */ +function userOwnsPageDialogs(tab: AgentTab): boolean { + const contents = tab.view.webContents + if (!currentScope.pageDialogsEnabled || contents.isDestroyed()) return false + if (tab.id !== currentScope.activeTabId || !isPanelVisible()) return false + if (getBrowserScopeId() !== getActiveBrowserScopeId()) return false + if (isDispatchingAgentInput(contents)) return false + if (automationTab()?.id !== tab.id) return true + return automationTabClaimedByUser() && !currentScope.automationActive +} + +function holdPageDialogForUser(tab: AgentTab, respond: (accept: boolean) => void): void { + if (tab.pageDialog) answerPageDialog(tab, false) + tab.pageDialog = { request: { requestId: generateId(), kind: 'beforeunload' }, respond } + events?.onPageStateChanged(tab.view.webContents) +} + +function answerPageDialog(tab: AgentTab, accept: boolean): void { + const dialog = tab.pageDialog + if (!dialog) return + tab.pageDialog = undefined + if (!tab.view.webContents.isDestroyed()) events?.onPageStateChanged(tab.view.webContents) + dialog.respond(accept) +} + +/** Lets Electron display a single native alert or confirm for the user. */ +export function claimUserDialog(contents: WebContents): boolean { + const tab = tabForContents(contents) + if (!tab || !tab.view.getVisible() || !userOwnsPageDialogs(tab)) return false + tab.nativePageDialogOpen = true + return true +} + +function invalidatePageDialog(tab: AgentTab): void { + const hadPageDialog = Boolean(tab.pageDialog) + tab.pendingLeave = undefined + tab.allowNextUnload = false + tab.pageDialog = undefined + if (hadPageDialog && !tab.view.webContents.isDestroyed()) { + events?.onPageStateChanged(tab.view.webContents) + } +} + +/** Clears native dialog ownership after the user answers or the page closes it. */ +export function notePageDialogClosed(contents: WebContents): void { + const tab = tabForContents(contents) + if (!tab?.nativePageDialogOpen) return + tab.nativePageDialogOpen = false + focusAfterPageDialog(tab) +} + +/** Prevents automation from answering or replacing a decision the user owns. */ +export function hasPendingPageDialog(contents: WebContents): boolean { + const tab = tabForContents(contents) + return Boolean(tab?.nativePageDialogOpen || tab?.pageDialog) +} + +function focusAfterPageDialog(tab: AgentTab): void { + const contents = tab.view.webContents + if ( + !contents.isDestroyed() && + tab.id === currentScope.activeTabId && + getBrowserScopeId() === getActiveBrowserScopeId() && + isPanelVisible() + ) { + focusPageForUser(contents) + } +} + +/** The user's answer to the exact dialog the renderer showed. */ +export function respondToPageDialog(requestId: string, accept: boolean): void { + const tab = tabs.find((entry) => entry.pageDialog?.request.requestId === requestId) + if (!tab) return + focusAfterPageDialog(tab) + answerPageDialog(tab, accept) +} + +/** The dialog on this page awaiting the user's answer, if any. */ +export function pageDialogForContents(contents: WebContents): BrowserPageDialog | undefined { + return tabForContents(contents)?.pageDialog?.request +} + +/** + * Whether the user decides this unload: true once a prompt holds the user's + * browser-chrome navigation, which is then cancelled until they choose Leave. + */ +export function claimUserLeave(contents: WebContents): boolean { + const tab = tabForContents(contents) + if (!tab || tab.allowNextUnload) return false + if (tab.pageDialog?.request.kind === 'beforeunload') return true + const leave = tab.pendingLeave + tab.pendingLeave = undefined + if (!leave || !userOwnsPageDialogs(tab)) return false + holdPageDialogForUser(tab, (accept) => { + if (!accept) return + tab.allowNextUnload = true + leave() + }) + return true +} + +/** + * Runs a navigation the user started from browser chrome. If the page asks + * before unloading, the navigation is held and replayed once the user agrees. + * `navigate` returns false when there was nothing to traverse. + */ +export function navigateForUser(contents: WebContents, navigate: () => unknown): void { + const tab = tabForContents(contents) + if (tab && currentScope.pageDialogsEnabled) tab.pendingLeave = navigate + // Nothing to traverse (Back with no history): no unload will ask, so a later + // page-initiated navigation must not replay this one. + if (navigate() === false && tab) tab.pendingLeave = undefined +} + function focusRendererOmnibox(mode: BrowserOmniboxFocusMode): void { if (getBrowserScopeId() !== getActiveBrowserScopeId()) return const win = panelWindow() @@ -2437,16 +2577,24 @@ function initializeTabView( event.preventDefault() }) - // Pages may hold navigation hostage with beforeunload dialogs nobody can - // see; always let the unload proceed. - contents.on('will-prevent-unload', (event) => { - event.preventDefault() - }) + // A beforeunload is decided twice, by whichever answer lands first: here + // (preventDefault lets the unload proceed) and by the CDP dialog. Both ask + // claimUserLeave, so agent work and page-initiated navigations proceed, and a + // navigation the user started from browser chrome is cancelled and replayed + // only once the user chooses to leave. + contents.on( + 'will-prevent-unload', + bindToBrowserScope(scopeId, (event) => { + if (!claimUserLeave(contents)) event.preventDefault() + }) + ) contents.on( 'render-process-gone', bindToBrowserScope(scopeId, (_event, details) => { const tab = tabs.find((entry) => entry.view === view) if (!tab) return + tab.nativePageDialogOpen = false + invalidatePageDialog(tab) if (tab.recoveringUnresponsive) { tab.recoveringUnresponsive = false contents.reload() @@ -2521,11 +2669,11 @@ function initializeTabView( return } if (shortcut === 'reload') { - reloadPage(contents) + navigateForUser(contents, () => reloadPage(contents)) return } if (shortcut === 'hard-reload') { - hardReloadPage(contents) + navigateForUser(contents, () => hardReloadPage(contents)) return } @@ -2577,6 +2725,15 @@ function initializeTabView( persistBrowserSession() }) ) + contents.on( + 'did-start-navigation', + bindToBrowserScope(scopeId, (details) => { + if (!details.isMainFrame) return + const tab = tabForContents(contents) + if (!tab) return + invalidatePageDialog(tab) + }) + ) contents.on( 'did-navigate-in-page', bindToBrowserScope(scopeId, (_event, _url, isMainFrame) => { @@ -2655,15 +2812,6 @@ export function setAutomationNeedsAttention(needsAttention: boolean): void { events?.onTabsChanged() } -/** - * Re-applies the tab throttling policy after a caller temporarily suspended it - * (the panel's reveal pulse). Exempts the automation-active tab exactly as the - * internal policy does. - */ -export function reassertTabThrottling(): void { - applyAutomationTabPolicy() -} - /** * Unthrottles the automation tab while automation is active, throttles every other tab, and * keeps the automation tab composited while no panel shows it. Call after anything that changes @@ -3353,8 +3501,11 @@ export function switchTab(tabId: string, { claim = true }: { claim?: boolean } = revokeTabMediaPermissions(previousActiveTab, false) previousActiveTab.pendingUserFocus = false } - currentScope.activeTabId = tab.id + // The claim describes the page on screen, so a mirrored switch to another + // page leaves that page unclaimed rather than inheriting the last one's. if (claim) currentScope.visibleTabUserSelected = true + else if (currentScope.activeTabId !== tab.id) currentScope.visibleTabUserSelected = false + currentScope.activeTabId = tab.id promotePendingTabRestore(tab) // Visible selection does not move the automation exemption; the user may // inspect another page while a tool continues in its background tab. @@ -3551,18 +3702,26 @@ export function handleFocusedShortcut( case 'focus-omnibox': focusRendererOmnibox('select') return true - case 'reload-or-clear': - reloadPage(shortcutTab.view.webContents) + case 'reload-or-clear': { + const contents = shortcutTab.view.webContents + navigateForUser(contents, () => reloadPage(contents)) return true - case 'hard-reload': - hardReloadPage(shortcutTab.view.webContents) + } + case 'hard-reload': { + const contents = shortcutTab.view.webContents + navigateForUser(contents, () => hardReloadPage(contents)) return true - case 'back': - goBack(shortcutTab.view.webContents) + } + case 'back': { + const contents = shortcutTab.view.webContents + navigateForUser(contents, () => goBack(contents)) return true - case 'forward': - goForward(shortcutTab.view.webContents) + } + case 'forward': { + const contents = shortcutTab.view.webContents + navigateForUser(contents, () => goForward(contents)) return true + } } const zoomAction = zoomActionForShortcut(shortcut) diff --git a/apps/desktop/src/main/config.ts b/apps/desktop/src/main/config.ts index d4b5497bb5c..68e5a87cad5 100644 --- a/apps/desktop/src/main/config.ts +++ b/apps/desktop/src/main/config.ts @@ -104,6 +104,7 @@ export interface DesktopSettings { /** Whether omnibox typing may request live Google search completions. */ browserSearchSuggestionsEnabled?: boolean terminalEnabled?: boolean + fullFileAccess?: boolean /** Keep the machine awake while a chat is running desktop work in the background. */ preventSleepWhileRunning?: boolean /** Device-wide browser page appearance; `app` follows Sim. */ @@ -238,6 +239,7 @@ const DEFAULT_SETTINGS: DesktopSettings = { browserEnabled: true, browserSearchSuggestionsEnabled: true, terminalEnabled: true, + fullFileAccess: false, preventSleepWhileRunning: true, } diff --git a/apps/desktop/src/main/desktop-executor/runner.test.ts b/apps/desktop/src/main/desktop-executor/runner.test.ts index f10ac5a3748..e3df0c0bcf9 100644 --- a/apps/desktop/src/main/desktop-executor/runner.test.ts +++ b/apps/desktop/src/main/desktop-executor/runner.test.ts @@ -1,8 +1,10 @@ -import { mkdir, mkdtemp, rm, writeFile } from 'node:fs/promises' +import { mkdir, mkdtemp, realpath, rm, stat, writeFile } from 'node:fs/promises' import { tmpdir } from 'node:os' -import { join } from 'node:path' +import { dirname, join, relative } from 'node:path' +import { fileURLToPath } from 'node:url' import type { TerminalToolResponse } from '@sim/terminal-protocol' -import { afterEach, describe, expect, it, vi } from 'vitest' +import { app } from 'electron' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { DeviceRequestError } from '@/main/desktop-executor/client' import type { ClaimedDesktopCall, @@ -10,6 +12,13 @@ import type { } from '@/main/desktop-executor/protocol' import { createDesktopToolRunner, type DesktopToolRunnerDeps } from '@/main/desktop-executor/runner' import { executeLocalFileRequest } from '@/main/local-files' +import { openNativeFile } from '@/main/native-directory' + +vi.mock('electron', () => import('@/test/electron-mock')) + +beforeEach(() => { + vi.mocked(app.getAppPath).mockReturnValue(fileURLToPath(new URL('../../..', import.meta.url))) +}) function terminalCall(toolCallId: string, operation: string): ClaimedDesktopCall { return { @@ -35,7 +44,23 @@ function runner(overrides: Partial = {}) { terminal: { executeTool: vi.fn(), cancelTool: vi.fn(async () => true) }, localFiles: { request: (call, request) => - executeLocalFileRequest(request, { toolName: call.toolName, args: call.args }), + executeLocalFileRequest( + request, + { toolName: call.toolName, args: call.args }, + { + path: String(call.args.path), + resolve: realpath, + open: async (path, directory = false) => { + const root = await realpath(dirname(String(call.args.path))) + return openNativeFile( + root, + relative(root, path), + await stat(root, { bigint: true }), + directory + ) + }, + } + ), }, imports: { importEntry: vi.fn() }, localFilesystem: { handle: vi.fn(), vfsRoot: () => 'user-local/x--1' }, diff --git a/apps/desktop/src/main/desktop-executor/runner.ts b/apps/desktop/src/main/desktop-executor/runner.ts index 1bb3663d765..8f1f789e365 100644 --- a/apps/desktop/src/main/desktop-executor/runner.ts +++ b/apps/desktop/src/main/desktop-executor/runner.ts @@ -106,7 +106,8 @@ export interface DesktopToolRunnerDeps { localFiles: { request( call: ClaimedDesktopCall, - request: DesktopLocalFileRequest + request: DesktopLocalFileRequest, + signal: AbortSignal ): Promise } imports: { @@ -289,7 +290,8 @@ export function createDesktopToolRunner(deps: DesktopToolRunnerDeps): DesktopToo const folders: DesktopLocalFileImportResult['folders'] = [] const targetWorkspaceId = typeof call.args.targetWorkspaceId === 'string' ? call.args.targetWorkspaceId : '' - const read = (request: DesktopLocalFileRequest) => deps.localFiles.request(call, request) + const read = (request: DesktopLocalFileRequest) => + deps.localFiles.request(call, request, signal) try { const response = await read({ operation: 'manifest', toolCallId: call.toolCallId }) if (!response.ok) throw new Error(response.error) @@ -337,10 +339,14 @@ export function createDesktopToolRunner(deps: DesktopToolRunnerDeps): DesktopToo if (!deps.accountDataAvailable()) return localAccessUnavailable() if (call.toolName === 'read_local_file') { return localFileReadCompletion( - await deps.localFiles.request(call, { - operation: 'read', - toolCallId: call.toolCallId, - }) + await deps.localFiles.request( + call, + { + operation: 'read', + toolCallId: call.toolCallId, + }, + signal + ) ) } if (call.toolName === 'import_local_files') return await runImport(call, signal) diff --git a/apps/desktop/src/main/desktop-executor/service.ts b/apps/desktop/src/main/desktop-executor/service.ts index 95b856be5e3..b51d5afcde2 100644 --- a/apps/desktop/src/main/desktop-executor/service.ts +++ b/apps/desktop/src/main/desktop-executor/service.ts @@ -35,6 +35,7 @@ import { } from '@/main/desktop-executor/executor' import { createExecutorJournal } from '@/main/desktop-executor/journal' import { + type ClaimedDesktopCall, DESKTOP_EXECUTOR_PROTOCOL_VERSION, type DesktopExecutorTiming, type DesktopImportEntryRequest, @@ -79,6 +80,8 @@ export interface DesktopExecutorService { * journal, whenever it is asked; empty when the journal cannot be read. */ pendingResults(): Promise> + /** Confirms that Sim still authorizes this device to execute the claimed call. */ + revalidateCall(call: ClaimedDesktopCall): Promise /** Stores one entry of a claimed import, as this device's registered session. */ importEntry( request: DesktopImportEntryRequest, @@ -487,6 +490,12 @@ export function createDesktopExecutorService( pendingResults() { return pendingResultsSnapshot() }, + async revalidateCall(call) { + const current = client + if (!current || !deps.accountDataAvailable()) return false + await current.renewLease(call.toolCallId, call.executionToken) + return client === current && deps.accountDataAvailable() + }, importEntry(request, signal) { if (!client) throw new Error('The Sim desktop app is not signed in to Sim.') return client.importEntry(request, signal) diff --git a/apps/desktop/src/main/desktop-settings.ts b/apps/desktop/src/main/desktop-settings.ts index 37c88b27056..a237202a12f 100644 --- a/apps/desktop/src/main/desktop-settings.ts +++ b/apps/desktop/src/main/desktop-settings.ts @@ -37,6 +37,7 @@ export interface DesktopSettingsService { getPreferences(): DesktopPreferences setPreference(key: DesktopPreferenceKey, value: boolean): DesktopPreferences setBrowserSearchSuggestionsEnabled(enabled: boolean): DesktopPreferences + setFullFileAccess(enabled: boolean): DesktopPreferences setPreventSleepWhileRunning(enabled: boolean): DesktopPreferences setAppearancePreference( key: DesktopAppearanceSettingKey, @@ -52,6 +53,7 @@ export interface DesktopSettingsService { interface DesktopSettingsServiceDeps { config: ConfigStore + onFullFileAccessChanged?: (preferences: DesktopPreferences) => void getMainWindow: () => BrowserWindow | null openMainWindowAt: (route?: string) => void setAutoDownloadUpdates: (enabled: boolean) => void @@ -96,6 +98,7 @@ function readPreferences( browserEnabled: config.get('browserEnabled') ?? true, browserSearchSuggestionsEnabled: config.get('browserSearchSuggestionsEnabled') ?? true, terminalEnabled: config.get('terminalEnabled') ?? true, + fullFileAccess: config.get('fullFileAccess') === true, preventSleepWhileRunning: config.get('preventSleepWhileRunning') ?? true, browserTheme: isDesktopAppearanceTheme(browserTheme) ? browserTheme : 'app', browserDefaultZoom: isDesktopZoomPercent(browserDefaultZoom) ? browserDefaultZoom : 100, @@ -179,6 +182,19 @@ export function createDesktopSettingsService( deps.config.flush() return read() }, + setFullFileAccess(enabled) { + deps.config.set('fullFileAccess', enabled) + if (!deps.config.flush()) { + deps.config.set('fullFileAccess', false) + deps.onFullFileAccessChanged?.(read()) + throw new Error( + 'Could not save file access settings. Your previous setting may return after restarting Sim.' + ) + } + const preferences = read() + deps.onFullFileAccessChanged?.(preferences) + return preferences + }, setPreventSleepWhileRunning(enabled) { deps.config.set('preventSleepWhileRunning', enabled) deps.config.flush() diff --git a/apps/desktop/src/main/index.ts b/apps/desktop/src/main/index.ts index 63edf362b29..71b2f67449e 100644 --- a/apps/desktop/src/main/index.ts +++ b/apps/desktop/src/main/index.ts @@ -15,10 +15,12 @@ import { } from 'electron' import { beginAccountDataTeardown, + captureAccountDataGeneration, completeDeploymentScopedTeardown, getAccountDataTeardownKind, getAccountDataTeardownOrigin, initializeAccountDataRecovery, + isAccountDataGenerationCurrent, isAccountDataTeardownRequired, prepareAccountDataTeardownForQuit, retryAccountDataTeardown, @@ -74,6 +76,7 @@ import { } from '@/main/help-search' import { registerIpcHandlers } from '@/main/ipc' import { attachLoadHealth, type LoadHealthHandle } from '@/main/load-health' +import { LocalFilePermissions } from '@/main/local-file-permissions' import { executeLocalFileRequest } from '@/main/local-files' import { LocalFilesystemService, mountVfsRoot } from '@/main/local-filesystem' import { createEncryptedLocalFilesystemGrantStore } from '@/main/local-filesystem-grant-store' @@ -167,6 +170,19 @@ function main(): void { join(userDataPath, 'local-filesystem-grants.json') ), }) + const localFilePermissions = new LocalFilePermissions( + localFilesystem, + () => config.get('fullFileAccess') === true, + () => { + desktopSettings.setFullFileAccess(true) + } + ) + const clearLocalFileAccess = async () => { + config.set('fullFileAccess', false) + const saved = config.flush() + await localFilesystem.forgetAll() + if (!saved) throw new Error('Full file access could not be disabled') + } const scopeEvents = new ScopedEventRouter() const terminal = new TerminalRegistry( { @@ -349,7 +365,7 @@ function main(): void { }, }, { label: 'task resource state', clear: clearDesktopChatSessions }, - { label: 'local filesystem grants', clear: () => localFilesystem.forgetAll() }, + { label: 'local filesystem grants', clear: clearLocalFileAccess }, ] const outcomes = await Promise.allSettled( stores.map(({ clear }) => Promise.resolve().then(clear)) @@ -533,6 +549,9 @@ function main(): void { const desktopSettings = createDesktopSettingsService({ config, + onFullFileAccessChanged: (preferences) => { + broadcast('desktop:settings:full-file-access-changed', preferences) + }, getMainWindow, openMainWindowAt: (route) => void openMainWindowAt(route), setAutoDownloadUpdates: (enabled) => updater?.setAutoDownload(enabled), @@ -627,8 +646,27 @@ function main(): void { }, terminal, localFiles: { - request: (call, request) => - executeLocalFileRequest(request, { toolName: call.toolName, args: call.args }), + request: async (call, request, signal) => { + const generation = captureAccountDataGeneration() + const origin = appOrigin() + const authorization = { toolName: call.toolName, args: call.args } + try { + const access = await localFilePermissions.authorize(authorization, { + parent: async () => getMainWindow(), + origin, + generation, + signal, + isCurrent: () => + isAccountDataGenerationCurrent(generation) && + accountDataAvailable() && + appOrigin() === origin, + revalidate: () => desktopExecutor.revalidateCall(call), + }) + return await executeLocalFileRequest(request, authorization, access) + } catch (error) { + return { ok: false, error: getErrorMessage(error) } + } + }, }, imports: { importEntry: (request, signal) => desktopExecutor.importEntry(request, signal), @@ -654,7 +692,7 @@ function main(): void { // that would have cleared fine still holding the outgoing deployment's // access. Each failure is named so the picker can say what survived. const stores = [ - { label: 'local file access', clear: () => localFilesystem.forgetAll() }, + { label: 'local file access', clear: clearLocalFileAccess }, { label: 'built-in browser sessions', clear: () => clearAgentBrowserProfile({ settingsPersistence: 'server-repair' }), @@ -772,7 +810,7 @@ function main(): void { } const stores = [ { label: 'built-in browser sessions', clear: () => clearAgentBrowserProfile() }, - { label: 'local filesystem grants', clear: () => localFilesystem.forgetAll() }, + { label: 'local filesystem grants', clear: clearLocalFileAccess }, { label: 'browser site history', clear: () => { @@ -891,6 +929,7 @@ function main(): void { if (win) loadHealthByWindow.get(win)?.retry() }, localFilesystem, + localFilePermissions, terminal, settings: desktopSettings, getWindowState: (sender) => ({ @@ -980,6 +1019,9 @@ function main(): void { allowHttpLocalhost, openSettings, openServerSettings: () => serverWindow.open(), + openFolderAccess: (parent) => { + if (accountDataAvailable()) localFilesystem.showAccessMenu(parent) + }, newWindow: () => void createAndLoadAppWindow(), newChat: () => void openMainWindowAt(newChatRoute(config.get('lastRoute'))), handleFocusedResourceShortcut: (win, shortcut) => diff --git a/apps/desktop/src/main/ipc.test.ts b/apps/desktop/src/main/ipc.test.ts index 0c037b5a5b0..d38cea1ec4d 100644 --- a/apps/desktop/src/main/ipc.test.ts +++ b/apps/desktop/src/main/ipc.test.ts @@ -134,6 +134,7 @@ import { import { getSearchSuggestions } from '@/main/browser-search/suggestions' import { trackInputActivity } from '@/main/input-activity' import { type IpcDeps, registerIpcHandlers } from '@/main/ipc' +import { LocalFilePermissions } from '@/main/local-file-permissions' import { LocalFilesystemService } from '@/main/local-filesystem' import { isLocalPageUrl } from '@/main/local-pages' import { TerminalRegistry } from '@/main/terminal/registry' @@ -287,6 +288,9 @@ describe('registerIpcHandlers', () => { mockCoordinator.showChooser.mockClear() mockCoordinator.listFillOptions.mockClear() mockCoordinator.fillCredential.mockClear() + const localFilesystem = new LocalFilesystemService({ + chooseDirectory: vi.fn(async () => null), + }) deps = { appOrigin: () => APP, getExecutorDevice: () => null, @@ -297,9 +301,8 @@ describe('registerIpcHandlers', () => { beginOAuthConnect: vi.fn(async () => true), prepareSourceConnect: vi.fn(() => 's'.repeat(32)), cancelSourceConnect: vi.fn(() => true), - localFilesystem: new LocalFilesystemService({ - chooseDirectory: vi.fn(async () => null), - }), + localFilesystem, + localFilePermissions: new LocalFilePermissions(localFilesystem), terminal: new TerminalRegistry(), scopeEvents: { activateBrowser: vi.fn(), @@ -311,6 +314,7 @@ describe('registerIpcHandlers', () => { getPreferences: vi.fn(() => DEFAULT_DESKTOP_PREFERENCES), setPreference: vi.fn(), setBrowserSearchSuggestionsEnabled: vi.fn(), + setFullFileAccess: vi.fn(), setPreventSleepWhileRunning: vi.fn(), setAppearancePreference: vi.fn(), setBrowserDefaultZoom: vi.fn(), @@ -462,71 +466,6 @@ describe('registerIpcHandlers', () => { }) }) - it('reads a native file through canonical IPC arguments without folder grants or user activation', async () => { - const { invoke } = collectHandlers() - const handler = invoke.get('desktop:local-files') - const path = fileURLToPath(import.meta.url) - const fetchAuthorization = vi.fn(async () => - Response.json({ chatId: 'chat-1', toolName: 'read_local_file', args: { path, limit: 64 } }) - ) - const authorizedEvent = { - senderFrame: { url: `${APP}/o/org/home` }, - sender: { session: { fetch: fetchAuthorization } }, - } - const mounts = vi.spyOn(deps.localFilesystem, 'handle') - expect( - await handler?.(authorizedEvent, { - operation: 'read', - toolCallId: 'tool-native', - path: '/not/the/canonical/path', - }) - ).toMatchObject({ - ok: true, - data: { kind: 'read', path, text: readFileSync(path, 'utf8').slice(0, 64) }, - }) - expect(mounts).not.toHaveBeenCalled() - expect(fetchAuthorization).toHaveBeenCalledWith( - `${APP}/api/desktop/tool/authorize`, - expect.objectContaining({ body: JSON.stringify({ toolCallId: 'tool-native', claim: true }) }) - ) - expect( - await handler?.(evilEvent, { operation: 'read', toolCallId: 'tool-native' }) - ).toMatchObject({ ok: false }) - }) - - it('claims native imports at IPC before traversal and rejects a replay', async () => { - const { invoke } = collectHandlers() - const handler = invoke.get('desktop:local-files') - const fetchAuthorization = vi - .fn() - .mockResolvedValueOnce( - Response.json({ - chatId: 'chat-1', - toolName: 'import_local_files', - args: { path: fileURLToPath(import.meta.url), targetWorkspaceId: 'workspace' }, - }) - ) - .mockResolvedValueOnce(Response.json({ error: 'already started' }, { status: 409 })) - const event = { - senderFrame: { url: `${APP}/o/org/home` }, - sender: { session: { fetch: fetchAuthorization } }, - } - const request = { operation: 'manifest', toolCallId: 'tool-import' } - expect(await handler?.(event, request)).toMatchObject({ - ok: true, - data: { - kind: 'manifest', - targetWorkspaceId: 'workspace', - entries: [{ relativePath: '', kind: 'file' }], - }, - }) - expect(fetchAuthorization).toHaveBeenCalledWith( - `${APP}/api/desktop/tool/authorize`, - expect.objectContaining({ body: JSON.stringify({ toolCallId: 'tool-import', claim: true }) }) - ) - expect(await handler?.(event, request)).toMatchObject({ ok: false, code: 'ALREADY_STARTED' }) - }) - it('requires server authorization for every privileged filesystem tool request', async () => { const { invoke } = collectHandlers() const handler = invoke.get('desktop:local-filesystem') diff --git a/apps/desktop/src/main/ipc.ts b/apps/desktop/src/main/ipc.ts index 41d9b409650..6a1068b2336 100644 --- a/apps/desktop/src/main/ipc.ts +++ b/apps/desktop/src/main/ipc.ts @@ -1,3 +1,4 @@ +import { isDeepStrictEqual } from 'node:util' import { BROWSER_TOOL_AUTHORIZATION_TIMEOUT_MS, type BrowserPanelAction, @@ -32,6 +33,10 @@ import { isRecordLike, toRecord } from '@sim/utils/object' import { PASTE_LIMITS, utf8ByteLength } from '@sim/utils/paste' import type { BrowserWindow, IpcMainEvent, IpcMainInvokeEvent, WebContents } from 'electron' import { clipboard, ipcMain, shell } from 'electron' +import { + captureAccountDataGeneration, + isAccountDataGenerationCurrent, +} from '@/main/account-data-generation' import { type BrowserToolQueueBoundary, cancelActiveTool, @@ -83,8 +88,9 @@ import { isSafeInternalPath } from '@/main/config' import type { DesktopSettingsService } from '@/main/desktop-settings' import { isDesktopPreferenceKey } from '@/main/desktop-settings' import { hasRecentDeliberateInput, hasRecentDiscreteInput } from '@/main/input-activity' +import type { LocalFilePermissions } from '@/main/local-file-permissions' import { executeLocalFileRequest } from '@/main/local-files' -import type { LocalFilesystemService } from '@/main/local-filesystem' +import type { LocalFileAccess, LocalFilesystemService } from '@/main/local-filesystem' import { isAppOrigin, openExternalSafe } from '@/main/navigation' import type { ScopedEventRouter } from '@/main/scoped-event-router' import type { TerminalRegistry } from '@/main/terminal/registry' @@ -341,6 +347,7 @@ export interface IpcDeps { isLocalPageUrl: (url: string) => boolean retryLoad: (sender: WebContents) => void localFilesystem: LocalFilesystemService + localFilePermissions: LocalFilePermissions terminal: TerminalRegistry scopeEvents: Pick< ScopedEventRouter, @@ -607,6 +614,8 @@ async function authorizeLocalFilesystemTool( * unvalidated args they must parse themselves. */ export function registerIpcHandlers(deps: IpcDeps): void { + const activeLocalFiles = new Map>() + let activeLocalFileCount = 0 const browserScopeBySender = new WeakMap() const terminalScopeBySender = new WeakMap() const browserPendingScopesBySender = new WeakMap>() @@ -766,8 +775,12 @@ export function registerIpcHandlers(deps: IpcDeps): void { requiresAccountData: true, passSender: true, denied: { ok: false, error: 'Local file tools are unavailable from this page.' }, - handler: (_sender, request, authorization) => - executeLocalFileRequest(request, authorization as DesktopToolAuthorization), + handler: (_sender, request, authorization, access) => + executeLocalFileRequest( + request, + authorization as DesktopToolAuthorization, + access as LocalFileAccess + ), }, 'desktop:local-filesystem': { kind: 'invoke', @@ -811,6 +824,17 @@ export function registerIpcHandlers(deps: IpcDeps): void { ? deps.settings.setBrowserSearchSuggestionsEnabled(enabled) : deps.settings.getPreferences(), }, + 'desktop:settings:set-full-file-access': { + kind: 'invoke', + gate: 'app-origin', + needsUserActivation: true, + requiresAccountData: true, + denied: null, + handler: (enabled) => + typeof enabled === 'boolean' + ? deps.settings.setFullFileAccess(enabled) + : deps.settings.getPreferences(), + }, 'desktop:settings:set-prevent-sleep': { kind: 'invoke', gate: 'app-origin', @@ -2133,30 +2157,81 @@ export function registerIpcHandlers(deps: IpcDeps): void { } } if (channel === 'desktop:local-files') { + const generation = captureAccountDataGeneration() + const origin = deps.appOrigin() const request = args[0] - if (!isRecordLike(request)) return { ok: false, error: 'Invalid local file request.' } - let failureStatus: number | undefined - const authorization = await fetchDesktopToolAuthorization( - event, - deps, - request.toolCallId, - request.operation === 'manifest' || request.operation === 'read', - (status) => { - failureStatus = status - } - ) - if (failureStatus === 409) + if (!isRecordLike(request) || !isDesktopToolCallId(request.toolCallId)) + return { ok: false, error: 'Invalid local file request.' } + const key = JSON.stringify([event.sender.id, request.toolCallId]) + if (request.operation === 'cancel') { + for (const pending of activeLocalFiles.get(key) ?? []) pending.abort() + return { ok: false, error: 'Local file operation cancelled.' } + } + if (activeLocalFileCount >= 128) return { ok: false, - code: 'ALREADY_STARTED', - error: 'This import is already running or was already started.', + error: 'Too many local file operations are running. Try again later.', } - if ( - !authorization || - !['read_local_file', 'import_local_files'].includes(authorization.toolName) - ) - return { ok: false, error: 'This is not an authorized pending local file tool call.' } - handlerArgs = [request, authorization] + const controller = new AbortController() + const controllers = activeLocalFiles.get(key) ?? new Set() + controllers.add(controller) + activeLocalFiles.set(key, controllers) + activeLocalFileCount++ + try { + let failureStatus: number | undefined + const authorization = await fetchDesktopToolAuthorization( + event, + deps, + request.toolCallId, + request.operation === 'manifest' || request.operation === 'read', + (status) => { + failureStatus = status + } + ) + if (failureStatus === 409) + return { + ok: false, + code: 'ALREADY_STARTED', + error: 'This import is already running or was already started.', + } + if ( + !authorization || + !['read_local_file', 'import_local_files'].includes(authorization.toolName) + ) + return { ok: false, error: 'This is not an authorized pending local file tool call.' } + if ( + authorization.toolName === 'read_local_file' + ? request.operation !== 'read' + : request.operation !== 'manifest' && request.operation !== 'chunk' + ) + return { ok: false, error: 'The operation does not match the pending tool call.' } + const parent = deps.getWindowForContents(event.sender) + if (!parent) + return { ok: false, error: 'A desktop window is required to approve file access.' } + const access = await deps.localFilePermissions.authorize(authorization, { + parent: async () => parent, + origin, + generation, + signal: controller.signal, + isCurrent: () => + isAccountDataGenerationCurrent(generation) && + deps.accountDataAvailable() && + deps.appOrigin() === origin && + isAppOriginSender(event, origin), + revalidate: async () => + isDeepStrictEqual( + authorization, + await fetchDesktopToolAuthorization(event, deps, request.toolCallId) + ), + }) + return await spec.handler(event.sender, request, authorization, access) + } catch (error) { + return { ok: false, error: getErrorMessage(error) } + } finally { + controllers.delete(controller) + if (controllers.size === 0) activeLocalFiles.delete(key) + activeLocalFileCount-- + } } if (spec.passSender) { handlerArgs = [event.sender, ...handlerArgs] diff --git a/apps/desktop/src/main/local-file-permissions.ts b/apps/desktop/src/main/local-file-permissions.ts new file mode 100644 index 00000000000..9504f591327 --- /dev/null +++ b/apps/desktop/src/main/local-file-permissions.ts @@ -0,0 +1,262 @@ +import { lstat, realpath, stat } from 'node:fs/promises' +import { homedir } from 'node:os' +import { dirname, isAbsolute, join, relative, resolve } from 'node:path' +import { isDesktopScopeId } from '@sim/desktop-bridge' +import type { BrowserWindow } from 'electron' +import { showShellDialog } from '@/main/dialogs' +import type { LocalFileAuthorization } from '@/main/local-files' +import type { LocalFileAccess, LocalFilesystemService } from '@/main/local-filesystem' +import { openNativeFile } from '@/main/native-directory' + +const MAX_PENDING_REQUESTS = 32 + +interface LocalFilePermissionContext { + parent: () => Promise + origin: string + generation: number + signal: AbortSignal + isCurrent: () => boolean + revalidate: () => Promise +} + +interface PendingFolderDecision { + contexts: Set + controller: AbortController + decision: Promise +} + +function nativePath(value: unknown): string { + if (typeof value !== 'string' || value.length > 4096 || value.includes('\0')) + throw new Error('A native absolute path or ~/ path is required.') + const path = + value === '~' ? homedir() : value.startsWith('~/') ? join(homedir(), value.slice(2)) : value + if (!isAbsolute(path)) throw new Error('Use an absolute path or ~/ path.') + return resolve(path) +} + +function assertCurrent(context: LocalFilePermissionContext): void { + context.signal.throwIfAborted() + if (!context.isCurrent()) + throw new Error('This local file request expired. Ask again in the current chat.') +} + +async function revalidate(context: LocalFilePermissionContext): Promise { + assertCurrent(context) + if (!(await context.revalidate())) + throw new Error('This local file tool call is no longer pending or its arguments changed.') + assertCurrent(context) +} + +/** Shares each pending folder decision while remembered access remains concurrent. */ +export class LocalFilePermissions { + private queue: Promise = Promise.resolve() + private readonly pending = new Map() + + constructor( + private readonly filesystem: LocalFilesystemService, + private readonly fullFileAccess: () => boolean = () => false, + private readonly enableFullFileAccess?: () => void + ) {} + + async authorize( + authorization: LocalFileAuthorization, + context: LocalFilePermissionContext + ): Promise { + assertCurrent(context) + if ( + authorization.toolName === 'import_local_files' && + (!isDesktopScopeId(authorization.args.targetWorkspaceId) || + (authorization.args.folderId !== undefined && + !isDesktopScopeId(authorization.args.folderId))) + ) + throw new Error('A valid destination workspace and folder are required for imports.') + const path = await realpath(nativePath(authorization.args.path)) + if (this.fullFileAccess()) { + return this.unrestrictedAccess(path, context) + } + const existing = await this.filesystem.nativeAccess(path) + if (existing) return this.authorizedAccess(existing, context) + const info = await stat(path) + if (!info.isFile() && !info.isDirectory()) + throw new Error('The path is not a regular file or directory.') + const folder = info.isDirectory() ? path : dirname(path) + const key = JSON.stringify([context.generation, context.origin, folder]) + let pending = this.pending.get(key) + if (pending?.controller.signal.aborted) pending = undefined + if (!pending) { + if (this.pending.size >= MAX_PENDING_REQUESTS) + throw new Error('Too many local file requests are waiting for permission. Try again later.') + const contexts = new Set() + const controller = new AbortController() + const decision = this.queue.then(() => + this.requestFolder(folder, contexts, controller.signal) + ) + this.queue = decision.then( + () => undefined, + () => undefined + ) + const request = { contexts, controller, decision } + pending = request + this.pending.set(key, request) + void this.queue.then(() => { + if (this.pending.get(key) === request) this.pending.delete(key) + }) + } + pending.contexts.add(context) + await this.waitForDecision(pending, context) + if (this.fullFileAccess()) return this.unrestrictedAccess(path, context) + const access = await this.filesystem.nativeAccess(path) + if (!access) throw new Error('The approved folder is no longer available.') + return this.authorizedAccess(access, context) + } + + private async unrestrictedAccess( + path: string, + context: LocalFilePermissionContext + ): Promise { + const info = await stat(path) + const folder = info.isDirectory() ? path : dirname(path) + const identity = await stat(folder, { bigint: true }) + return this.authorizedAccess( + { + path, + resolve: realpath, + open: (requested, directory = false) => + openNativeFile(folder, relative(folder, requested), identity, directory), + }, + { ...context, isCurrent: () => context.isCurrent() && this.fullFileAccess() } + ) + } + + private waitForDecision( + pending: PendingFolderDecision, + context: LocalFilePermissionContext + ): Promise { + return new Promise((resolve, reject) => { + const leave = () => { + context.signal.removeEventListener('abort', cancel) + pending.contexts.delete(context) + if (pending.contexts.size === 0) pending.controller.abort() + } + const cancel = () => { + leave() + reject(context.signal.reason) + } + context.signal.addEventListener('abort', cancel, { once: true }) + pending.decision.then( + () => { + leave() + resolve() + }, + (error) => { + leave() + reject(error) + } + ) + if (context.signal.aborted) cancel() + }) + } + + private async currentContext( + contexts: Set, + signal: AbortSignal + ): Promise { + signal.throwIfAborted() + for (const context of contexts) { + try { + await revalidate(context) + if (contexts.has(context)) return context + } catch { + signal.throwIfAborted() + } + } + throw new Error('The local file requests are no longer pending. Ask again in the current chat.') + } + + private async requestFolder( + folder: string, + contexts: Set, + signal: AbortSignal + ): Promise { + const context = await this.currentContext(contexts, signal) + if (this.fullFileAccess() || (await this.filesystem.nativeAccess(folder))) return + const root = await lstat(folder, { bigint: true }) + if (!root.isDirectory()) throw new Error('The folder is no longer available.') + const displayedPath = JSON.stringify(folder).replace( + /\p{Bidi_Control}/gu, + (character) => `\\u${character.charCodeAt(0).toString(16).padStart(4, '0')}` + ) + signal.throwIfAborted() + const parent = await context.parent() + await this.currentContext(contexts, signal) + const options = { + signal, + title: 'Allow access to this folder?', + message: displayedPath, + detail: `Sim can read and import files from this folder and its subfolders across chats on ${context.origin}.\n\nManage access in File → Folder Access.`, + buttons: [ + 'Allow folder', + "Don't allow", + ...(this.enableFullFileAccess ? ['Full file access'] : []), + ], + defaultId: 1, + cancelId: 1, + } + while (true) { + await this.currentContext(contexts, signal) + const result = await (parent ? showShellDialog(parent, options) : showShellDialog(options)) + signal.throwIfAborted() + if (result.response === 2 && this.enableFullFileAccess) { + const confirmation = { + signal, + title: 'Enable full file access?', + message: `Sim can read and import files from any folder on this computer across chats on ${context.origin}.`, + detail: 'Turn this off in Settings → Desktop → Full file access.', + buttons: ['Enable', 'Cancel'], + defaultId: 1, + cancelId: 1, + } + const answer = await (parent + ? showShellDialog(parent, confirmation) + : showShellDialog(confirmation)) + await this.currentContext(contexts, signal) + if (answer.response !== 0) continue + this.enableFullFileAccess() + return + } + if (result.response !== 0) throw new Error('The user did not allow this local file access.') + const current = await this.currentContext(contexts, signal) + await this.filesystem.grantDirectory({ path: folder }, current.generation, root) + return + } + } + + private async authorizedAccess( + access: LocalFileAccess, + context: LocalFilePermissionContext + ): Promise { + await revalidate(context) + const resolveApproved = async (path: string): Promise => { + assertCurrent(context) + const resolved = await access.resolve(path) + assertCurrent(context) + return resolved + } + await resolveApproved(access.path) + return { + path: access.path, + resolve: resolveApproved, + open: async (path, directory) => { + assertCurrent(context) + const file = await access.open(path, directory) + try { + assertCurrent(context) + return file + } catch (error) { + await file.close() + throw error + } + }, + } + } +} diff --git a/apps/desktop/src/main/local-files.test.ts b/apps/desktop/src/main/local-files.test.ts index 1e01abebfd7..fd7fe391d4d 100644 --- a/apps/desktop/src/main/local-files.test.ts +++ b/apps/desktop/src/main/local-files.test.ts @@ -1,12 +1,32 @@ -import { mkdir, mkdtemp, rm, symlink, truncate, writeFile } from 'node:fs/promises' +import { mkdir, mkdtemp, realpath, rm, stat, symlink, truncate, writeFile } from 'node:fs/promises' import { tmpdir } from 'node:os' -import { join } from 'node:path' -import { afterEach, beforeEach, expect, it } from 'vitest' -import { executeLocalFileRequest } from '@/main/local-files' +import { join, relative } from 'node:path' +import { fileURLToPath } from 'node:url' +import { app } from 'electron' +import { afterEach, beforeEach, expect, it, vi } from 'vitest' + +vi.mock('electron', () => import('@/test/electron-mock')) + +import { + executeLocalFileRequest as executeApprovedLocalFileRequest, + type LocalFileAuthorization, +} from '@/main/local-files' +import { openNativeFile } from '@/main/native-directory' + +/** Parser and import invariants run with explicit fixture access; consent is covered through Electron. */ +function executeLocalFileRequest(request: unknown, authorization: LocalFileAuthorization) { + return executeApprovedLocalFileRequest(request, authorization, { + path: String(authorization.args.path), + resolve: realpath, + open: async (path, directory = false) => + openNativeFile(root, relative(root, path), await stat(root, { bigint: true }), directory), + }) +} let root: string beforeEach(async () => { - root = await mkdtemp(join(tmpdir(), 'sim-native-files-')) + vi.mocked(app.getAppPath).mockReturnValue(fileURLToPath(new URL('../..', import.meta.url))) + root = await realpath(await mkdtemp(join(tmpdir(), 'sim-native-files-'))) }) afterEach(async () => { await rm(root, { recursive: true, force: true }) diff --git a/apps/desktop/src/main/local-files.ts b/apps/desktop/src/main/local-files.ts index a4b9cd9a42c..840dd6c10f5 100644 --- a/apps/desktop/src/main/local-files.ts +++ b/apps/desktop/src/main/local-files.ts @@ -1,5 +1,4 @@ -import { open, readdir, realpath, stat } from 'node:fs/promises' -import { homedir } from 'node:os' +import { lstat, stat } from 'node:fs/promises' import { basename, isAbsolute, join, relative, resolve, sep } from 'node:path' import type { DesktopLocalFileEntry, @@ -10,7 +9,10 @@ import type { import { MAX_DESKTOP_IMPORT_FILE_BYTES } from '@sim/desktop-bridge' import { getErrorMessage } from '@sim/utils/errors' import { isRecordLike } from '@sim/utils/object' +import { compareStrings } from '@sim/utils/string' import { PDFDocument } from 'pdf-lib' +import type { LocalFileAccess } from '@/main/local-filesystem' +import { readNativeDirectory } from '@/main/native-directory' const CHUNK_BYTES = 8 * 1024 * 1024 const MAX_ENTRIES = 1000 @@ -21,16 +23,6 @@ export interface LocalFileAuthorization { args: Record } -/** Resolve normal native paths; macOS, not Sim folder grants, owns filesystem access. */ -function nativePath(value: unknown): string { - if (typeof value !== 'string' || value.length > 4096 || value.includes('\0')) - throw new Error('A native absolute path or ~/ path is required.') - const path = - value === '~' ? homedir() : value.startsWith('~/') ? join(homedir(), value.slice(2)) : value - if (!isAbsolute(path)) throw new Error('Use an absolute path or ~/ path.') - return resolve(path) -} - function revision(info: Awaited>): string { return `${info.dev}:${info.ino}:${info.size}:${info.mtimeMs}` } @@ -48,33 +40,42 @@ function assertImportPath(root: string, candidate: string): void { throw new Error('The file is outside this import source.') } -async function inspect(path: string, args: Record): Promise { +async function readApprovedDirectory(path: string, access: LocalFileAccess) { + const handle = await access.open(path, true) + try { + const listing = await readNativeDirectory(handle.fd, MAX_ENTRIES) + const canonical = await access.resolve(path) + const current = await lstat(canonical) + const opened = await handle.stat() + if (canonical !== path || current.dev !== opened.dev || current.ino !== opened.ino) + throw new Error('The local directory changed while it was being read. Try again.') + listing.entries.sort((left, right) => compareStrings(left.name, right.name)) + return listing + } finally { + await handle.close() + } +} + +async function inspect( + path: string, + args: Record, + access: LocalFileAccess +): Promise { const info = await stat(path) if (info.isDirectory()) { - const entries = await readdir(path, { withFileTypes: true }) + const { entries, truncated } = await readApprovedDirectory(path, access) return { kind: 'read', path, representation: 'directory', - truncated: entries.length > MAX_ENTRIES, - entries: entries - .sort((a, b) => (a.name < b.name ? -1 : a.name > b.name ? 1 : 0)) - .slice(0, MAX_ENTRIES) - .map((entry) => ({ - name: entry.name, - kind: entry.isFile() - ? 'file' - : entry.isDirectory() - ? 'directory' - : entry.isSymbolicLink() - ? 'symlink' - : 'other', - })), + truncated, + entries, } } if (!info.isFile()) throw new Error('The path is not a regular file or directory.') - const file = await open(path, 'r') + const file = await access.open(path) try { + const info = await file.stat() const header = Buffer.alloc(16) await file.read(header, 0, header.length, 0) const mediaType = header.subarray(0, 8).equals(Buffer.from([137, 80, 78, 71, 13, 10, 26, 10])) @@ -169,17 +170,18 @@ async function inspect(path: string, args: Record): Promise + args: Record, + access: LocalFileAccess ): Promise { if (typeof args.targetWorkspaceId !== 'string') throw new Error('A target workspace is required.') - const root = await realpath(path) + const root = await access.resolve(path) const entries: DesktopLocalFileEntry[] = [] async function walk(current: string, ancestors: ReadonlySet): Promise { if (entries.length >= MAX_ENTRIES) throw new Error( 'The directory exceeds 1,000 entries. Import smaller subdirectories separately.' ) - const canonical = await realpath(current) + const canonical = await access.resolve(current) assertImportPath(root, canonical) const info = await stat(canonical) if (!info.isDirectory() && !info.isFile()) @@ -197,7 +199,12 @@ async function manifest( }) if (info.isDirectory()) { const next = new Set([...ancestors, canonical]) - for (const name of (await readdir(current)).sort()) await walk(join(current, name), next) + const children = await readApprovedDirectory(canonical, access) + if (children.truncated) + throw new Error( + 'The directory exceeds 1,000 entries. Import smaller subdirectories separately.' + ) + for (const entry of children.entries) await walk(join(current, entry.name), next) } } await walk(path, new Set()) @@ -210,20 +217,27 @@ async function manifest( } } -/** Calls have already been authorized against the pending server record by the IPC boundary. */ +/** Requires both a pending server call and a main-process grant before returning local data. */ export async function executeLocalFileRequest( request: unknown, - authorization: LocalFileAuthorization + authorization: LocalFileAuthorization, + access: LocalFileAccess ): Promise { try { if (!isRecordLike(request)) throw new Error('Invalid local file request.') - const path = nativePath(authorization.args.path) - if (request.operation === 'read' && authorization.toolName === 'read_local_file') - return { ok: true, data: await inspect(path, authorization.args) } + const path = await access.resolve(access.path) + if (request.operation === 'read' && authorization.toolName === 'read_local_file') { + const data = await inspect(path, authorization.args, access) + await access.resolve(path) + return { ok: true, data } + } if (authorization.toolName !== 'import_local_files') throw new Error('The operation does not match the pending tool call.') - if (request.operation === 'manifest') - return { ok: true, data: await manifest(path, authorization.args) } + if (request.operation === 'manifest') { + const data = await manifest(path, authorization.args, access) + await access.resolve(path) + return { ok: true, data } + } if ( request.operation !== 'chunk' || typeof request.relativePath !== 'string' || @@ -232,11 +246,11 @@ export async function executeLocalFileRequest( throw new Error('Invalid file chunk request.') const child = resolve(path, request.relativePath) assertImportPath(path, child) - const root = await realpath(path) - const canonical = await realpath(child) + const root = await access.resolve(path) + const canonical = await access.resolve(child) assertImportPath(root, canonical) const offset = boundedInteger(request.offset, 0, Number.MAX_SAFE_INTEGER) - const file = await open(canonical, 'r') + const file = await access.open(canonical) try { const info = await file.stat() if (!info.isFile() || revision(info) !== request.revision) @@ -248,6 +262,7 @@ export async function executeLocalFileRequest( const { bytesRead } = await file.read(buffer, 0, buffer.length, offset) if (revision(await file.stat()) !== request.revision) throw new Error('The source file changed during import.') + await access.resolve(child) return { ok: true, data: { diff --git a/apps/desktop/src/main/local-filesystem-grant-store.test.ts b/apps/desktop/src/main/local-filesystem-grant-store.test.ts index 7fef1bd965d..8b9fc5e8540 100644 --- a/apps/desktop/src/main/local-filesystem-grant-store.test.ts +++ b/apps/desktop/src/main/local-filesystem-grant-store.test.ts @@ -16,7 +16,11 @@ function testEncryption(available = true) { } describe('createEncryptedLocalFilesystemGrantStore', () => { - it('encrypts grants at rest and restores them', async () => { + it.each([ + {}, + { dev: 1, ino: Number.MAX_SAFE_INTEGER }, + { dev: '1', ino: '18446744073709551615' }, + ])('encrypts grants at rest and restores their exact identity %j', async (identity) => { const directory = await mkdtemp(join(tmpdir(), 'sim-localfs-store-')) const filePath = join(directory, 'grants.json') const encryption = testEncryption() @@ -27,6 +31,7 @@ describe('createEncryptedLocalFilesystemGrantStore', () => { name: 'project', rootPath: '/Users/example/private-project', bookmark: 'security-scoped-bookmark', + ...identity, }, ] @@ -35,7 +40,6 @@ describe('createEncryptedLocalFilesystemGrantStore', () => { const raw = await readFile(filePath, 'utf8') expect(raw).not.toContain(grants[0].rootPath) expect(raw).not.toContain(grants[0].bookmark) - expect(encryption.encryptString).toHaveBeenCalledOnce() await expect(store.load()).resolves.toEqual(grants) await store.clear() diff --git a/apps/desktop/src/main/local-filesystem-grant-store.ts b/apps/desktop/src/main/local-filesystem-grant-store.ts index 67216976840..3b7dc887198 100644 --- a/apps/desktop/src/main/local-filesystem-grant-store.ts +++ b/apps/desktop/src/main/local-filesystem-grant-store.ts @@ -21,6 +21,8 @@ export interface PersistedLocalFilesystemGrant { id: string name: string rootPath: string + dev?: number | string + ino?: number | string bookmark?: string } @@ -41,6 +43,15 @@ interface EncryptedGrantEnvelope { ciphertext: string } +function isStoredIdentity(value: unknown): value is number | string { + if (typeof value === 'number') return Number.isSafeInteger(value) && value >= 0 + return ( + typeof value === 'string' && + /^(0|[1-9][0-9]{0,19})$/.test(value) && + BigInt(value) <= 18446744073709551615n + ) +} + function isPersistedGrant(value: unknown): value is PersistedLocalFilesystemGrant { if (!value || typeof value !== 'object' || Array.isArray(value)) return false const grant = value as Record @@ -55,6 +66,8 @@ function isPersistedGrant(value: unknown): value is PersistedLocalFilesystemGran grant.rootPath.length > 0 && grant.rootPath.length <= MAX_GRANT_PATH_LENGTH && !grant.rootPath.includes('\0') && + ((grant.dev === undefined && grant.ino === undefined) || + (isStoredIdentity(grant.dev) && isStoredIdentity(grant.ino))) && (grant.bookmark === undefined || (typeof grant.bookmark === 'string' && grant.bookmark.length > 0 && diff --git a/apps/desktop/src/main/local-filesystem.test.ts b/apps/desktop/src/main/local-filesystem.test.ts index f4d0b5a35a6..81a1bbc2c6e 100644 --- a/apps/desktop/src/main/local-filesystem.test.ts +++ b/apps/desktop/src/main/local-filesystem.test.ts @@ -3,6 +3,7 @@ import { mkdir, mkdtemp, open, + readFile, realpath, rename, rm, @@ -16,7 +17,12 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' vi.mock('electron', () => import('@/test/electron-mock')) vi.mock('node:fs/promises', async (importOriginal) => { const actual = await importOriginal() - return { ...actual, open: vi.fn(actual.open) } + return { + ...actual, + open: vi.fn(actual.open), + realpath: vi.fn(actual.realpath), + readFile: vi.fn(actual.readFile), + } }) import type { LocalFilesystemMount, LocalFilesystemResponse } from '@sim/desktop-bridge' @@ -379,6 +385,34 @@ describe('LocalFilesystemService', () => { } ) + it.each(['resolution', 'read'] as const)( + 'returns no VFS contents when access is revoked during %s', + async (stage) => { + const granted = await mount(service) + const revoke = () => service.handle({ operation: 'forget_mount', uri: granted.uri }) + if (stage === 'resolution') { + const original = vi.mocked(realpath).getMockImplementation() + if (!original) throw new Error('Missing real filesystem implementation') + vi.mocked(realpath).mockImplementationOnce(async (...args) => { + const result = await original(...args) + await revoke() + return result + }) + } else { + const original = vi.mocked(readFile).getMockImplementation() + if (!original) throw new Error('Missing real filesystem implementation') + vi.mocked(readFile).mockImplementationOnce(async (...args) => { + const result = await original(...args) + await revoke() + return result + }) + } + expect( + await service.handle({ operation: 'read', uri: `${granted.uri}README.md` }) + ).toMatchObject({ ok: false, code: 'MOUNT_NOT_FOUND' }) + } + ) + it('rejects lexical traversal before URL normalization can reinterpret it', async () => { const granted = await mount(service) const traversal = await service.handle({ diff --git a/apps/desktop/src/main/local-filesystem.ts b/apps/desktop/src/main/local-filesystem.ts index a83abf50881..f823c60c315 100644 --- a/apps/desktop/src/main/local-filesystem.ts +++ b/apps/desktop/src/main/local-filesystem.ts @@ -17,10 +17,12 @@ import { MAX_GREP_RESULTS, MAX_READ_LINES, } from '@sim/desktop-bridge/local-filesystem-limits' +import { getErrorMessage } from '@sim/utils/errors' import { generateId } from '@sim/utils/id' import { isRecordLike } from '@sim/utils/object' import { escapeRegExp, stripTrailingSlashes, truncate } from '@sim/utils/string' -import { app, dialog, shell } from 'electron' +import type { BrowserWindow } from 'electron' +import { app, dialog, Menu, shell } from 'electron' import micromatch from 'micromatch' import safeRegex from 'safe-regex2' import { @@ -30,10 +32,12 @@ import { runAccountDataMutation, waitForAccountDataMutations, } from '@/main/account-data-generation' +import { showShellDialog } from '@/main/dialogs' import type { LocalFilesystemGrantStore, PersistedLocalFilesystemGrant, } from '@/main/local-filesystem-grant-store' +import { openNativeFile } from '@/main/native-directory' const MAX_URI_LENGTH = 4096 const MAX_LIST_ENTRIES = 500 @@ -93,6 +97,8 @@ const GRANT_SCOPED_OPERATIONS: ReadonlySet = new Set([ interface GrantedMount extends LocalFilesystemMount { rootPath: string + dev: bigint + ino: bigint bookmark?: string stopAccessing?: () => void } @@ -346,6 +352,12 @@ async function selectDirectoryEntries( return { entries, truncated: seen > entries.length } } +export interface LocalFileAccess { + path: string + resolve: (path: string) => Promise + open: (path: string, directory?: boolean) => ReturnType +} + export class LocalFilesystemService { private readonly mounts = new Map() private readonly activeRequests = new Map() @@ -488,12 +500,12 @@ export class LocalFilesystemService { 'Local filesystem operation is not supported.' ) } + if (grant) await this.assertMountCurrent(grant) } finally { if (requestId) { this.activeRequests.delete(requestId) } } - if (grant && this.mounts.get(grant.id)?.rootPath !== grant.rootPath) throw mountNotFound() return { ok: true, data } } catch (error) { const safe = safeError(error) @@ -629,7 +641,18 @@ export class LocalFilesystemService { throw new LocalFilesystemError('CANCELLED', 'The folder request expired during sign-out.') } - const selected = typeof selection === 'string' ? { path: selection } : selection + return this.grantDirectory( + typeof selection === 'string' ? { path: selection } : selection, + generation + ) + } + + /** Records a folder selected through trusted desktop UI, with the identity shown at consent. */ + async grantDirectory( + selected: SelectedDirectory, + generation: number, + expected?: { dev: bigint; ino: bigint } + ): Promise { const stopAccessing = selected.bookmark ? this.startAccessingBookmark(selected.bookmark) : undefined @@ -644,7 +667,7 @@ export class LocalFilesystemService { try { const rootPath = await realpath(selected.path) - const rootStat = await stat(rootPath) + const rootStat = await stat(rootPath, { bigint: true }) if (!isAccountDataGenerationCurrent(generation)) { throw new LocalFilesystemError('CANCELLED', 'The folder request expired during sign-out.') } @@ -652,7 +675,24 @@ export class LocalFilesystemService { throw new LocalFilesystemError('NOT_A_DIRECTORY', 'The selected item is not a directory.') } + if ( + expected && + (rootPath !== selected.path || + rootStat.dev !== expected.dev || + rootStat.ino !== expected.ino) + ) { + throw new LocalFilesystemError( + 'ACCESS_DENIED', + 'The folder changed while awaiting permission. Request access again.' + ) + } const existing = [...this.mounts.values()].find((mount) => mount.rootPath === rootPath) + if (!existing && this.mounts.size >= 256) { + throw new LocalFilesystemError( + 'INVALID_REQUEST', + 'Forget an unused folder in File → Folder Access before adding more.' + ) + } const id = existing?.id ?? generateId() const bookmark = selected.bookmark ?? existing?.bookmark const nextStopAccessing = selected.bookmark @@ -667,6 +707,8 @@ export class LocalFilesystemService { name: basename(rootPath) || 'Local files', uri: localUri(id), rootPath, + dev: rootStat.dev, + ino: rootStat.ino, remembered: existing?.remembered ?? false, ...(bookmark ? { bookmark } : {}), ...(nextStopAccessing ? { stopAccessing: nextStopAccessing } : {}), @@ -691,6 +733,98 @@ export class LocalFilesystemService { } } + /** Resolves native tool paths through the same remembered grants as VFS reads. */ + async nativeAccess(candidate: string): Promise { + const path = await realpath(candidate) + for (const mount of this.mounts.values()) { + if (!isWithinRoot(mount.rootPath, path)) continue + try { + await this.assertMountCurrent(mount) + } catch { + continue + } + const resolveGranted = async (requested: string): Promise => { + await this.assertMountCurrent(mount) + const resolved = await realpath(requested) + if (!isWithinRoot(mount.rootPath, resolved)) { + throw new LocalFilesystemError( + 'ACCESS_DENIED', + 'This path is outside the approved folder.' + ) + } + await this.assertMountCurrent(mount) + return resolved + } + return { + path, + resolve: resolveGranted, + open: async (requested, directory = false) => { + const canonical = await resolveGranted(requested) + if (canonical !== requested) throw new Error('The local path changed. Try again.') + const file = await openNativeFile( + mount.rootPath, + relative(mount.rootPath, canonical), + mount, + directory + ) + try { + await this.assertMountCurrent(mount) + return file + } catch (error) { + await file.close() + throw error + } + }, + } + } + return null + } + + private async assertMountCurrent(mount: GrantedMount): Promise { + const root = await lstat(mount.rootPath, { bigint: true }) + const current = this.mounts.get(mount.id) + if (!current || current.rootPath !== mount.rootPath) throw mountNotFound() + if ( + current.dev !== mount.dev || + current.ino !== mount.ino || + !root.isDirectory() || + root.dev !== mount.dev || + root.ino !== mount.ino + ) { + throw new LocalFilesystemError( + 'ACCESS_DENIED', + 'Folder access was removed or the folder changed. Request access again.' + ) + } + } + + /** Native controls share the same grant store and revocation path as the desktop bridge. */ + showAccessMenu(parent: BrowserWindow): void { + const generation = captureAccountDataGeneration() + const run = (operation: () => Promise) => { + if (!isAccountDataGenerationCurrent(generation) || parent.isDestroyed()) return + void operation().catch((error) => + showShellDialog(parent, { + title: 'Folder access', + message: getErrorMessage(error), + buttons: ['OK'], + }) + ) + } + Menu.buildFromTemplate([ + { label: 'Add Folder…', click: () => run(() => this.mountDirectory()) }, + { type: 'separator' }, + ...[...this.mounts.values()].map((mount) => ({ + label: mount.rootPath, + submenu: [ + { label: 'Show Folder', click: () => run(async () => this.revealMount(mount.uri)) }, + { label: 'Forget Folder', click: () => run(() => this.forgetMount(mount.uri)) }, + ], + })), + ...(this.mounts.size === 0 ? [{ label: 'No folders allowed', enabled: false }] : []), + ]).popup({ window: parent }) + } + private listMounts(): LocalFilesystemData { return { mounts: [...this.mounts.values()].map((mount) => this.publicMount(mount)) } } @@ -709,42 +843,52 @@ export class LocalFilesystemService { const generation = captureAccountDataGeneration() const grants = await this.grantStore.load() if (!isAccountDataGenerationCurrent(generation)) return - let skipped = false + let needsPersist = false for (const grant of grants) { if (!/^[a-zA-Z0-9-]{1,128}$/.test(grant.id) || this.mounts.has(grant.id)) { - skipped = true + needsPersist = true continue } const stopAccessing = grant.bookmark ? this.startAccessingBookmark(grant.bookmark) : undefined try { const rootPath = await realpath(grant.rootPath) - const rootStat = await stat(rootPath) + const rootStat = await stat(rootPath, { bigint: true }) if (!isAccountDataGenerationCurrent(generation)) { stopAccessing?.() return } - if (!rootStat.isDirectory()) { + if ( + !rootStat.isDirectory() || + rootPath !== grant.rootPath || + (grant.dev !== undefined && + (grant.ino === undefined || + BigInt(grant.dev) !== rootStat.dev || + BigInt(grant.ino) !== rootStat.ino)) + ) { stopAccessing?.() - skipped = true + needsPersist = true continue } + if (grant.dev === undefined) needsPersist = true this.mounts.set(grant.id, { id: grant.id, name: basename(rootPath) || grant.name || 'Local files', uri: localUri(grant.id), rootPath, + dev: rootStat.dev, + ino: rootStat.ino, remembered: true, ...(grant.bookmark ? { bookmark: grant.bookmark } : {}), ...(stopAccessing ? { stopAccessing } : {}), }) } catch { stopAccessing?.() - skipped = true + needsPersist = true } } - if (skipped) { + if (needsPersist) { await runAccountDataMutation(generation, () => this.persistMounts()) } } @@ -754,6 +898,8 @@ export class LocalFilesystemService { id: mount.id, name: mount.name, rootPath: mount.rootPath, + dev: mount.dev.toString(), + ino: mount.ino.toString(), ...(mount.bookmark ? { bookmark: mount.bookmark } : {}), })) } @@ -940,6 +1086,7 @@ export class LocalFilesystemService { private async resolveUri(uri: string): Promise { const { mount, relativePath } = this.parseUri(uri) + await this.assertMountCurrent(mount) const lexicalPath = resolve(mount.rootPath, ...relativePath.split('/').filter(Boolean)) if (!isWithinRoot(mount.rootPath, lexicalPath)) { throw new LocalFilesystemError( @@ -954,6 +1101,7 @@ export class LocalFilesystemService { 'The requested path is outside the selected folder.' ) } + await this.assertMountCurrent(mount) return { mount, relativePath, lexicalPath, realPath } } diff --git a/apps/desktop/src/main/menu.test.ts b/apps/desktop/src/main/menu.test.ts index 7364b5aaa39..2eb8862dc32 100644 --- a/apps/desktop/src/main/menu.test.ts +++ b/apps/desktop/src/main/menu.test.ts @@ -20,6 +20,7 @@ function makeDeps(origin = 'https://sim.ai'): MenuDeps { allowHttpLocalhost: vi.fn(() => false), openSettings: vi.fn(), openServerSettings: vi.fn(), + openFolderAccess: vi.fn(), newWindow: vi.fn(), newChat: vi.fn(), handleFocusedResourceShortcut: vi.fn(() => false), diff --git a/apps/desktop/src/main/menu.ts b/apps/desktop/src/main/menu.ts index b2234f74d5b..b6bfb58fcf7 100644 --- a/apps/desktop/src/main/menu.ts +++ b/apps/desktop/src/main/menu.ts @@ -19,6 +19,7 @@ export interface MenuDeps { openSettings: () => void /** Opens the native server picker (see main/server-window.ts). */ openServerSettings: () => void + openFolderAccess: (parent: BrowserWindow) => void newWindow: () => void newChat: () => void /** @@ -212,6 +213,14 @@ export function buildMenuTemplate(deps: MenuDeps): MenuItemConstructorOptions[] { label: 'File', submenu: [ + { + label: 'Folder Access…', + click: (_item, focusedWindow) => { + const win = focusedWindowOrMain(focusedWindow) + if (win) deps.openFolderAccess(win) + }, + }, + { type: 'separator' }, { label: 'New Window', accelerator: 'CmdOrCtrl+Shift+N', diff --git a/apps/desktop/src/main/native-directory.ts b/apps/desktop/src/main/native-directory.ts new file mode 100644 index 00000000000..224d16b37dd --- /dev/null +++ b/apps/desktop/src/main/native-directory.ts @@ -0,0 +1,71 @@ +import { fstat, read } from 'node:fs' +import { join } from 'node:path' +import { promisify } from 'node:util' +import type { DesktopLocalFileRead } from '@sim/desktop-bridge' +import { app } from 'electron' + +interface NativeDirectoryListing { + entries: NonNullable + truncated: boolean +} + +interface NativeDirectoryBridge { + closeFile: (descriptor: number) => Promise + readDirectory: (descriptor: number, limit: number) => Promise + openApproved: ( + root: string, + relativePath: string, + dev: bigint, + ino: bigint, + directory: boolean + ) => Promise +} + +let bridge: NativeDirectoryBridge | undefined +const statDescriptor = promisify(fstat) +const readDescriptor = promisify(read) + +function nativeBridge(): NativeDirectoryBridge { + bridge ??= require( + join(app.getAppPath(), 'dist', 'native', 'directory.node') + ) as NativeDirectoryBridge + return bridge +} + +/** Owns a descriptor opened beneath the identity of a granted directory. */ +export async function openNativeFile( + root: string, + relativePath: string, + identity: { dev: bigint; ino: bigint }, + directory: boolean +) { + let descriptor = await nativeBridge().openApproved( + root, + relativePath, + identity.dev, + identity.ino, + directory + ) + return { + get fd() { + return descriptor + }, + stat: () => statDescriptor(descriptor), + read: (buffer: Buffer, offset: number, length: number, position: number) => + readDescriptor(descriptor, buffer, offset, length, position), + close: async () => { + if (descriptor < 0) return + const closing = descriptor + descriptor = -1 + await nativeBridge().closeFile(closing) + }, + } +} + +/** Enumerates the already-validated descriptor without resolving a pathname again. */ +export function readNativeDirectory( + descriptor: number, + limit: number +): Promise { + return nativeBridge().readDirectory(descriptor, limit) +} diff --git a/apps/desktop/src/main/terminal/shell-startup.test.ts b/apps/desktop/src/main/terminal/shell-startup.test.ts index f734aad5ec9..ab1d2856f43 100644 --- a/apps/desktop/src/main/terminal/shell-startup.test.ts +++ b/apps/desktop/src/main/terminal/shell-startup.test.ts @@ -76,7 +76,6 @@ afterEach(() => { pty.emit = null pty.exit = null pty.writes.length = 0 - vi.unstubAllEnvs() }) describe('a shell that is still starting', () => { diff --git a/apps/desktop/src/preload/index.ts b/apps/desktop/src/preload/index.ts index 6071925e7dd..3704ce86a74 100644 --- a/apps/desktop/src/preload/index.ts +++ b/apps/desktop/src/preload/index.ts @@ -179,6 +179,15 @@ const api: SimDesktopApi = { getPreferences: (): Promise => ipcRenderer.invoke('desktop:settings:get'), setPreference: (key: DesktopPreferenceKey, value: boolean): Promise => ipcRenderer.invoke('desktop:settings:set', key, value), + setFullFileAccess: (enabled: boolean): Promise => + ipcRenderer.invoke('desktop:settings:set-full-file-access', enabled), + onFullFileAccessChanged: ( + callback: (preferences: DesktopPreferences) => void + ): (() => void) => { + const listener = (_event: unknown, preferences: DesktopPreferences) => callback(preferences) + ipcRenderer.on('desktop:settings:full-file-access-changed', listener) + return () => ipcRenderer.removeListener('desktop:settings:full-file-access-changed', listener) + }, setPreventSleepWhileRunning: (enabled: boolean): Promise => ipcRenderer.invoke('desktop:settings:set-prevent-sleep', enabled), setBrowserSearchSuggestionsEnabled: (enabled: boolean): Promise => diff --git a/apps/desktop/src/renderer/credential-picker/index.tsx b/apps/desktop/src/renderer/credential-picker/index.tsx index 622daf6f053..b492059c07b 100644 --- a/apps/desktop/src/renderer/credential-picker/index.tsx +++ b/apps/desktop/src/renderer/credential-picker/index.tsx @@ -57,7 +57,7 @@ function CredentialPicker({ configuration, api }: CredentialPickerProps) { align='start' sideOffset={0} avoidCollisions={false} - className='w-[320px] max-w-none' + className='w-[320px] max-w-none max-md:max-w-none!' aria-label='Saved passwords' onOpenAutoFocus={(event) => { event.preventDefault() diff --git a/apps/docs/app/global.css b/apps/docs/app/global.css index f98df319fc2..a6503c5a5a6 100644 --- a/apps/docs/app/global.css +++ b/apps/docs/app/global.css @@ -1641,7 +1641,7 @@ main article tbody tr:last-child td { Vertical margin is deliberately not set here: prose fences want `my-4` (the component supplies it) and API samples sit flush inside their panel with `my-0`. - @see packages/emcn/src/components/chip/chip-chrome.ts — `chipFieldSurfaceClass`, the constant + @see packages/emcn/src/components/chip/chrome.ts — `chipFieldSurfaceClass`, the constant this mirrors. Keep the two in step by hand. */ figure.shiki, div:has(> [role="tablist"]):has(> div > figure.shiki) { diff --git a/apps/docs/components/icons.tsx b/apps/docs/components/icons.tsx index 70e0ecd565c..a39190f4b78 100644 --- a/apps/docs/components/icons.tsx +++ b/apps/docs/components/icons.tsx @@ -1905,32 +1905,6 @@ export function AirtableIcon(props: SVGProps) { ) } -export function AirweaveIcon(props: SVGProps) { - return ( - - - - - - ) -} - export function AlgoliaIcon(props: SVGProps) { return ( @@ -3561,25 +3535,157 @@ export function MicrosoftIcon(props: SVGProps) { } export function IntuneIcon(props: SVGProps) { + const id = useId() return ( - - - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + ) } export function RampIcon(props: SVGProps) { return ( - - - + + ) } @@ -6269,6 +6375,37 @@ export function CbInsightsIcon(props: SVGProps) { ) } +export function CheckrIcon(props: SVGProps) { + return ( + + + + + + + + + ) +} + export function CalendlyIcon(props: SVGProps) { return ( @@ -9524,7 +9661,7 @@ export function NewRelicIcon(props: SVGProps) { ) } -export function NetSuiteIcon(props: SVGProps) { +export function OracleIcon(props: SVGProps) { return ( ) { ) } +export function NetSuiteIcon(props: SVGProps) { + return +} + export function WizaIcon(props: SVGProps) { return ( diff --git a/apps/docs/components/ui/faq.tsx b/apps/docs/components/ui/faq.tsx index 561fe4541e3..67003a0381d 100644 --- a/apps/docs/components/ui/faq.tsx +++ b/apps/docs/components/ui/faq.tsx @@ -2,7 +2,6 @@ import { useId, useState } from 'react' import { ChevronRight } from '@sim/emcn/icons' -import Script from 'next/script' import { serializeJsonLd } from '@/lib/json-ld' import { cn } from '@/lib/utils' @@ -91,7 +90,7 @@ export function FAQ({ items, title = 'Common Questions' }: FAQProps) { return (
-