Skip to content

Schema: fix getPropertySignatures crash on Struct with optionalWith({ default })#6086

Merged
gcanti merged 1 commit intoEffect-TS:mainfrom
taylorOntologize:fix/schema-ast-getPropertyKeyIndexedAccess-transformation
Feb 24, 2026
Merged

Schema: fix getPropertySignatures crash on Struct with optionalWith({ default })#6086
gcanti merged 1 commit intoEffect-TS:mainfrom
taylorOntologize:fix/schema-ast-getPropertyKeyIndexedAccess-transformation

Conversation

@taylorOntologize
Copy link
Contributor

@taylorOntologize taylorOntologize commented Feb 24, 2026

Summary

  • SchemaAST.getPropertyKeyIndexedAccess now handles Transformation AST nodes by delegating to ast.to, matching the existing behavior of getPropertyKeys
  • Fixes a crash where getPropertySignatures throws "Unsupported schema (Transformation)" on any Schema.Struct containing Schema.optionalWith with { default }, { as: "Option" }, { nullable: true }, or similar options
  • This also fixes @effect/ai's Tool.getJsonSchema() and LanguageModel.streamText, which call getPropertySignatures when building tool definitions

Closes #6085

Root cause

getPropertySignatures falls through its switch to a fallback path that calls getPropertyKeys(ast) then getPropertyKeyIndexedAccess(ast, name). getPropertyKeys handles Transformation (delegates to ast.to, added in #2343), but getPropertyKeyIndexedAccess did not — so the first half succeeds and the second half crashes.

At the time of #2343, this asymmetry was safe: getPropertyKeys gained Transformation support for pick/omit, which handle Transformations directly and never reach the fallback. The gap became observable when @effect/ai's Tool.getJsonSchema started calling getPropertySignatures on tool parameter schemas that use optionalWith({ default }).

Fix

One line — add case "Transformation": return getPropertyKeyIndexedAccess(ast.to, name), mirroring getPropertyKeys.

Tests

Two new test cases in getPropertySignatures.test.ts:

  • Transformation (Struct with optionalWith default) — the { default } variant
  • Transformation (Struct with optionalWith as Option) — the { as: "Option" } variant

Full suite passes (6152 tests, 0 failures), pnpm check clean.

Related

getIndexSignatures has the same missing Transformation case. It doesn't crash (returns [] silently), but it causes Schema.omit to silently drop index signatures on Transformation structs. That's a separate bug with a different surface area.

Playground repro

https://effect.website/play/#edb480bc7434

@changeset-bot
Copy link

changeset-bot bot commented Feb 24, 2026

🦋 Changeset detected

Latest commit: ff58c8e

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 36 packages
Name Type
effect Patch
@effect/cli Patch
@effect/cluster Patch
@effect/experimental Patch
@effect/opentelemetry Patch
@effect/platform-browser Patch
@effect/platform-bun Patch
@effect/platform-node-shared Patch
@effect/platform-node Patch
@effect/platform Patch
@effect/printer-ansi Patch
@effect/printer Patch
@effect/rpc Patch
@effect/sql-clickhouse Patch
@effect/sql-d1 Patch
@effect/sql-drizzle Patch
@effect/sql-kysely Patch
@effect/sql-libsql Patch
@effect/sql-mssql Patch
@effect/sql-mysql2 Patch
@effect/sql-pg Patch
@effect/sql-sqlite-bun Patch
@effect/sql-sqlite-do Patch
@effect/sql-sqlite-node Patch
@effect/sql-sqlite-react-native Patch
@effect/sql-sqlite-wasm Patch
@effect/sql Patch
@effect/typeclass Patch
@effect/vitest Patch
@effect/workflow Patch
@effect/ai Patch
@effect/ai-amazon-bedrock Patch
@effect/ai-anthropic Patch
@effect/ai-google Patch
@effect/ai-openai Patch
@effect/ai-openrouter Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@gcanti gcanti merged commit 4d97a61 into Effect-TS:main Feb 24, 2026
11 checks passed
@github-project-automation github-project-automation bot moved this from Discussion Ongoing to Done in PR Backlog Feb 24, 2026
@github-actions github-actions bot mentioned this pull request Feb 24, 2026
MrNaif2018 pushed a commit to bitcart/bitcart-frontend that referenced this pull request Mar 21, 2026
This PR contains the following updates:

| Package | Change | [Age](https://docs.renovatebot.com/merge-confidence/) | [Confidence](https://docs.renovatebot.com/merge-confidence/) |
|---|---|---|---|
| [effect](https://effect.website) ([source](https://github.com/Effect-TS/effect/tree/HEAD/packages/effect)) | [`3.19.19` → `3.20.0`](https://renovatebot.com/diffs/npm/effect/3.19.19/3.20.0) | ![age](https://developer.mend.io/api/mc/badges/age/npm/effect/3.20.0?slim=true) | ![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/effect/3.19.19/3.20.0?slim=true) |

---

### Effect `AsyncLocalStorage` context lost/contaminated inside Effect fibers under concurrent load with RPC
[CVE-2026-32887](https://nvd.nist.gov/vuln/detail/CVE-2026-32887) / [GHSA-38f7-945m-qr2g](GHSA-38f7-945m-qr2g)

<details>
<summary>More information</summary>

#### Details
##### Versions

- `effect`: 3.19.15
- `@effect/rpc`: 0.72.1
- `@effect/platform`: 0.94.2
- Node.js: v22.20.0
- Vercel runtime with Fluid compute
- Next.js: 16 (App Router)
- `@clerk/nextjs`: 6.x

##### Root cause

Effect's `MixedScheduler` batches fiber continuations and drains them inside a **single** microtask or timer callback. The `AsyncLocalStorage` context active during that callback belongs to whichever request first triggered the scheduler's drain cycle — **not** the request that owns the fiber being resumed.

##### Detailed mechanism

##### 1. Scheduler batching (`effect/src/Scheduler.ts`, `MixedScheduler`)

```typescript
// MixedScheduler.starve() — called once when first task is scheduled
private starve(depth = 0) {
  if (depth >= this.maxNextTickBeforeTimer) {
    setTimeout(() => this.starveInternal(0), 0)       // timer queue
  } else {
    Promise.resolve(void 0).then(() => this.starveInternal(depth + 1)) // microtask queue
  }
}

// MixedScheduler.starveInternal() — drains ALL accumulated tasks in one call
private starveInternal(depth: number) {
  const tasks = this.tasks.buckets
  this.tasks.buckets = []
  for (const [_, toRun] of tasks) {
    for (let i = 0; i < toRun.length; i++) {
      toRun[i]()  // ← Every fiber continuation runs in the SAME ALS context
    }
  }
  // ...
}
```

`scheduleTask` only calls `starve()` when `running` is `false`. Subsequent tasks accumulate in `this.tasks` until `starveInternal` drains them all. The `Promise.then()` (or `setTimeout`) callback inherits the ALS context from whichever call site created it — i.e., whichever request's fiber first set `running = true`.

**Result:** Under concurrent load, fiber continuations from Request A and Request B execute inside the same `starveInternal` call, sharing a single ALS context. If Request A triggered `starve()`, then Request B's fiber reads Request A's ALS context.

##### 2. `toWebHandlerRuntime` does not propagate ALS (`@effect/platform/src/HttpApp.ts:211-240`)

```typescript
export const toWebHandlerRuntime = <R>(runtime: Runtime.Runtime<R>) => {
  const httpRuntime: Types.Mutable<Runtime.Runtime<R>> = Runtime.make(runtime)
  const run = Runtime.runFork(httpRuntime)
  return <E>(self: Default<E, R | Scope.Scope>, middleware?) => {
    return (request: Request, context?): Promise<Response> =>
      new Promise((resolve) => {
        // Per-request Effect context is correctly set via contextMap:
        const contextMap = new Map<string, any>(runtime.context.unsafeMap)
        const httpServerRequest = ServerRequest.fromWeb(request)
        contextMap.set(ServerRequest.HttpServerRequest.key, httpServerRequest)
        httpRuntime.context = Context.unsafeMake(contextMap)

        // But the fiber is forked without any ALS propagation:
        const fiber = run(httpApp as any)  // ← ALS context is NOT captured or restored
      })
  }
}
```

Effect's own `Context` (containing `HttpServerRequest`) is correctly set per-request. But the **Node.js ALS context** — which frameworks like Next.js, Clerk, and OpenTelemetry rely on — is not captured at fork time or restored when the fiber's continuations execute.

##### 3. The dangerous pattern this enables

```typescript
// RPC handler — runs inside an Effect fiber
const handler = Effect.gen(function*() {
  // This calls auth() from @&#8203;clerk/nextjs/server, which reads from ALS
  const { userId } = yield* Effect.tryPromise({
    try: async () => auth(),  // ← may read WRONG user's session
    catch: () => new UnauthorizedError({ message: "Auth failed" })
  })
  return yield* repository.getUser(userId)
})
```

The `async () => auth()` thunk executes when the fiber continuation is scheduled by `MixedScheduler`. At that point, the ALS context belongs to an arbitrary concurrent request.

##### Reproduction scenario

```
Timeline (two concurrent requests to the same toWebHandler endpoint):

T0: Request A arrives → POST handler → webHandler(requestA)
    → Promise executor runs synchronously
    → httpRuntime.context set to A's context
    → fiber A forked, runs first ops synchronously
    → fiber A yields (e.g., at Effect.tryPromise boundary)
    → scheduler.scheduleTask(fiberA_continuation)
    → running=false → starve() called → Promise.resolve().then(drain)
       ↑ ALS context captured = Request A's context

T1: Request B arrives → POST handler → webHandler(requestB)
    → Promise executor runs synchronously
    → httpRuntime.context set to B's context
    → fiber B forked, runs first ops synchronously
    → fiber B yields
    → scheduler.scheduleTask(fiberB_continuation)
    → running=true → task queued, no new starve()

T2: Microtask fires → starveInternal() runs
    → Drains fiberA_continuation → auth() reads ALS → gets A's context ✓
    → Drains fiberB_continuation → auth() reads ALS → gets A's context ✗ ← WRONG USER
```

##### Minimal reproduction

```typescript
import { AsyncLocalStorage } from "node:async_hooks"
import { Effect, Layer } from "effect"
import { RpcServer, RpcSerialization, Rpc, RpcGroup } from "@&#8203;effect/rpc"
import { HttpServer } from "@&#8203;effect/platform"
import * as S from "effect/Schema"

// Simulate a framework's ALS (like Next.js / Clerk)
const requestStore = new AsyncLocalStorage<{ userId: string }>()

class GetUser extends Rpc.make("GetUser", {
  success: S.Struct({ userId: S.String, alsUserId: S.String }),
  failure: S.Never,
  payload: {}
}) {}

const MyRpc = RpcGroup.make("MyRpc").add(GetUser)

const MyRpcLive = MyRpc.toLayer(
  RpcGroup.toHandlers(MyRpc, {
    GetUser: () =>
      Effect.gen(function*() {
        // Simulate calling an ALS-dependent API inside an Effect fiber
        const alsResult = yield* Effect.tryPromise({
          try: async () => {
            const store = requestStore.getStore()
            return store?.userId ?? "NONE"
          },
          catch: () => { throw new Error("impossible") }
        })
        return { userId: "from-effect-context", alsUserId: alsResult }
      })
  })
)

const RpcLayer = MyRpcLive.pipe(
  Layer.provideMerge(RpcSerialization.layerJson),
  Layer.provideMerge(HttpServer.layerContext)
)

const { handler } = RpcServer.toWebHandler(MyRpc, { layer: RpcLayer })

// Simulate two concurrent requests with different ALS contexts
async function main() {
  const results = await Promise.all([
    requestStore.run({ userId: "user-A" }, () => handler(makeRpcRequest("GetUser"))),
    requestStore.run({ userId: "user-B" }, () => handler(makeRpcRequest("GetUser"))),
  ])

  // Parse responses and check if alsUserId matches the expected user
  // Under the bug: both responses may show "user-A" (or one shows the other's)
  for (const res of results) {
    console.log(await res.json())
  }
}
```

##### Impact

| Symptom | Severity |
|---------|----------|
| `auth()` returns wrong user's session | **Critical** — authentication bypass |
| `cookies()` / `headers()` from Next.js read wrong request | **High** — data leakage |
| OpenTelemetry trace context crosses requests | **Medium** — incorrect traces |
| Works locally, fails in production | Hard to diagnose — only manifests under concurrent load |

##### Workaround

Capture ALS-dependent values **before** entering the Effect runtime and pass them via Effect's own context system:

```typescript
// In the route handler — OUTSIDE the Effect fiber (ALS is correct here)
export const POST = async (request: Request) => {
  const { userId } = await auth()  // ← Safe: still in Next.js ALS context

  // Inject into request headers or use the `context` parameter
  const headers = new Headers(request.headers)
  headers.set("x-clerk-auth-user-id", userId ?? "")
  const enrichedRequest = new Request(request.url, {
    method: request.method,
    headers,
    body: request.body,
    duplex: "half" as any,
  })

  return webHandler(enrichedRequest)
}

// In Effect handlers — read from HttpServerRequest headers instead of calling auth()
const getAuthenticatedUserId = Effect.gen(function*() {
  const req = yield* HttpServerRequest.HttpServerRequest
  const userId = req.headers["x-clerk-auth-user-id"]
  if (!userId) return yield* Effect.fail(new UnauthorizedError({ message: "Auth required" }))
  return userId
})
```

##### Suggested fix (for Effect maintainers)

##### Option A: Propagate ALS context through the scheduler

Capture the `AsyncLocalStorage` snapshot when a fiber continuation is scheduled, and restore it when the continuation executes:

```typescript
// In MixedScheduler or the fiber runtime
import { AsyncLocalStorage } from "node:async_hooks"

scheduleTask(task: Task, priority: number) {
  // Capture current ALS context
  const snapshot = AsyncLocalStorage.snapshot()
  this.tasks.scheduleTask(() => snapshot(task), priority)
  // ...
}
```

`AsyncLocalStorage.snapshot()` (Node.js 20.5+) returns a function that, when called, restores the ALS context from the point of capture. This ensures each fiber continuation runs with its originating request's ALS context.

**Trade-off:** Adds one closure allocation per scheduled task. Could be opt-in via a `FiberRef` or scheduler option.

##### Option B: Capture ALS at `runFork` and restore per fiber step

When `Runtime.runFork` is called, capture the ALS snapshot and associate it with the fiber. Before each fiber step (in the fiber runtime's `evaluateEffect` loop), restore the snapshot.

**Trade-off:** More invasive but provides correct ALS propagation for the fiber's entire lifetime, including across `flatMap` chains and `Effect.tryPromise` thunks.

##### Option C: Document the limitation and provide a `context` injection API

If ALS propagation is intentionally not supported, document this prominently and provide a first-class API for `toWebHandler` to accept per-request context. The existing `context?: Context.Context<never>` parameter on the handler function partially addresses this, but it requires callers to know about the issue and manually extract values before entering Effect.

##### Related

- Node.js `AsyncLocalStorage` docs: https://nodejs.org/api/async_context.html
- `AsyncLocalStorage.snapshot()`: https://nodejs.org/api/async_context.html#static-method-asynclocalstoragesnapshot
- Next.js uses ALS for `cookies()`, `headers()`, `auth()` in App Router
- Similar issue pattern in other fiber-based runtimes (e.g., ZIO has `FiberRef` propagation for this)

##### POC replica of my setup

```
// Create web handler from Effect RPC
// sharedMemoMap ensures all RPC routes share the same connection pool
const { handler: webHandler, dispose } = RpcServer.toWebHandler(DemoRpc, {
  layer: RpcLayer,
  memoMap: sharedMemoMap,
});

/**
 * POST /api/rpc/demo
 */
export const POST = async (request: Request) => {
  return webHandler(request);
};

registerDispose(dispose);
```

##### Used util functions

```

/**
 * Creates a dispose registry that collects dispose callbacks and runs them
 * when `runAll` is invoked. Handles both sync and async dispose functions,
 * catching errors to prevent one failing dispose from breaking others.
 *
 * @&#8203;internal Exported for testing — use `registerDispose` in application code.
 */
export const makeDisposeRegistry = () => {
  const disposeFns: Array<() => void | Promise<void>> = []

  const runAll = () => {
    for (const fn of disposeFns) {
      try {
        const result = fn()
        if (result && typeof result.then === "function") {
          result.then(undefined, (err: unknown) => console.error("Dispose error:", err))
        }
      } catch (err) {
        console.error("Dispose error:", err)
      }
    }
  }

  const register = (dispose: () => void | Promise<void>) => {
    disposeFns.push(dispose)
  }

  return { register, runAll }
}

export const registerDispose: (dispose: () => void | Promise<void>) => void = globalValue(
  Symbol.for("@&#8203;global/RegisterDispose"),
  () => {
    const registry = makeDisposeRegistry()

    if (typeof process !== "undefined") {
      process.once("beforeExit", registry.runAll)
    }

    return registry.register
  }
)
```

##### The actual effect that was run within the RPC context that the bug was found

```
export const getAuthenticatedUserId: Effect.Effect<string, UnauthorizedError> =
  Effect.gen(function*() {
    const authResult = yield* Effect.tryPromise({
      try: async () => auth(),
      catch: () =>
        new UnauthorizedError({
          message: "Failed to get auth session"
        })
    })

    if (!authResult.userId) {
      return yield* Effect.fail(
        new UnauthorizedError({
          message: "Authentication required"
        })
      )
    }

    return authResult.userId
  })
 ```

#### Severity
- CVSS Score: 7.4 / 10 (High)
- Vector String: `CVSS:3.1/AV:N/AC:H/PR:N/UI:N/S:U/C:H/I:H/A:N`

#### References
- [https://github.com/Effect-TS/effect/security/advisories/GHSA-38f7-945m-qr2g](https://github.com/Effect-TS/effect/security/advisories/GHSA-38f7-945m-qr2g)
- [https://github.com/Effect-TS/effect](https://github.com/Effect-TS/effect)

This data is provided by [OSV](https://osv.dev/vulnerability/GHSA-38f7-945m-qr2g) and the [GitHub Advisory Database](https://github.com/github/advisory-database) ([CC-BY 4.0](https://github.com/github/advisory-database/blob/main/LICENSE.md)).
</details>

---

### Release Notes

<details>
<summary>Effect-TS/effect (effect)</summary>

### [`v3.20.0`](https://github.com/Effect-TS/effect/blob/HEAD/packages/effect/CHANGELOG.md#3200)

[Compare Source](https://github.com/Effect-TS/effect/compare/effect@3.19.19...effect@3.20.0)

##### Minor Changes

- [#&#8203;6124](Effect-TS/effect#6124) [`8798a84`](Effect-TS/effect@8798a84) Thanks [@&#8203;mikearnaldi](https://github.com/mikearnaldi)! - Fix scheduler task draining to isolate `AsyncLocalStorage` across fibers.

##### Patch Changes

- [#&#8203;6107](Effect-TS/effect#6107) [`fc82e81`](Effect-TS/effect@fc82e81) Thanks [@&#8203;gcanti](https://github.com/gcanti)! - Backport `Types.VoidIfEmpty` to 3.x

- [#&#8203;6088](Effect-TS/effect#6088) [`82996bc`](Effect-TS/effect@82996bc) Thanks [@&#8203;taylorOntologize](https://github.com/taylorOntologize)! - Schema: fix `Schema.omit` producing wrong result on Struct with `optionalWith({ default })` and index signatures

  `getIndexSignatures` now handles `Transformation` AST nodes by delegating to `ast.to`, matching the existing behavior of `getPropertyKeys` and `getPropertyKeyIndexedAccess`. Previously, `Schema.omit` on a struct combining `Schema.optionalWith` (with `{ default }`, `{ as: "Option" }`, etc.) and `Schema.Record` would silently take the wrong code path, returning a Transformation with property signatures instead of a TypeLiteral with index signatures.

- [#&#8203;6086](Effect-TS/effect#6086) [`4d97a61`](Effect-TS/effect@4d97a61) Thanks [@&#8203;taylorOntologize](https://github.com/taylorOntologize)! - Schema: fix `getPropertySignatures` crash on Struct with `optionalWith({ default })` and other Transformation-producing variants

  `SchemaAST.getPropertyKeyIndexedAccess` now handles `Transformation` AST nodes by delegating to `ast.to`, matching the existing behavior of `getPropertyKeys`. Previously, calling `getPropertySignatures` on a `Schema.Struct` containing `Schema.optionalWith` with `{ default }`, `{ as: "Option" }`, `{ nullable: true }`, or similar options would throw `"Unsupported schema (Transformation)"`.

- [#&#8203;6097](Effect-TS/effect#6097) [`f6b0960`](Effect-TS/effect@f6b0960) Thanks [@&#8203;gcanti](https://github.com/gcanti)! - Fix TupleWithRest post-rest validation to check each tail index sequentially.

</details>

---

### Configuration

📅 **Schedule**: Branch creation - "" in timezone UTC, Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0My42NC4yIiwidXBkYXRlZEluVmVyIjoiNDMuNjQuMiIsInRhcmdldEJyYW5jaCI6Im1hc3RlciIsImxhYmVscyI6WyJzZWN1cml0eSJdfQ==-->

Reviewed-on: https://git.bitcart.ai/bitcart/bitcart-frontend/pulls/163
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

SchemaAST.getPropertySignatures crashes on Struct with Schema.optionalWith({ default })

2 participants