Skip to content

Commit 0208548

Browse files
committed
fix!: rename ActivityDefaultOptions, order the duration guard, state schedule update concurrency
Review remediation on the v8 audit branch. - `ActivityDefaultOptions` -> `ContractActivityOptions`. The property was renamed to `activityOptions` in the earlier sweep but the type name was missed; the new name also keeps the contract-level, portable subset distinct from Temporal's own `ActivityOptions`, which is what the worker-side `activityOptionsByName` overrides take. - `assertDuration` checks the cheap length cap before running the regex. The pattern is linear so this was never exploitable, just the wrong order. - `TypedScheduleHandle.update`'s JSDoc claimed Temporal retries `updateFn` on a server-side conflict. It does not: in SDK 1.20.3 `ScheduleHandle.update` describes once, calls `updateFn` once, and `_updateSchedule` sends no conflict token, so `UpdateSchedule` is unconditional. Concurrent updates are last-writer-wins in the raw SDK too. Documented plainly, including the one thing the wrapper does change (an extra `describe`, slightly widening the window) and the one guarantee it keeps (validated options == persisted options).
1 parent f17e415 commit 0208548

4 files changed

Lines changed: 20 additions & 11 deletions

File tree

packages/client/src/schedule.ts

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -132,12 +132,18 @@ export type TypedScheduleHandle = {
132132
* has no schema to check it against); prefer delete + `create` for
133133
* contract-level changes.
134134
*
135-
* Implementation note: validation is asynchronous (schemas may be), so
136-
* the description is fetched by this wrapper and the *already-computed*
137-
* options are handed to Temporal's `ScheduleHandle.update`. `updateFn` is
138-
* therefore invoked exactly once per call — on a server-side conflict the
139-
* same computed options are retried, rather than `updateFn` being re-run
140-
* against a fresh description.
135+
* Concurrency: **last writer wins.** Temporal's `UpdateSchedule` RPC is
136+
* unconditional — the TypeScript SDK sends no conflict token and does not
137+
* re-run `updateFn` on a conflict — so a concurrent modification landing
138+
* between the read and the write is overwritten. That is true of the raw
139+
* SDK too; this wrapper does not weaken it, but it does widen the window
140+
* slightly: validation is asynchronous (schemas may be), so the wrapper
141+
* fetches the description itself and hands the *already-computed* options
142+
* to `ScheduleHandle.update`, which describes again internally. `updateFn`
143+
* is invoked exactly once per call, and the options that are validated are
144+
* exactly the options that are persisted.
145+
*
146+
* If two writers can race on one schedule, serialize them yourself.
141147
*/
142148
update: (
143149
updateFn: (

packages/contract/src/builder.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -570,7 +570,9 @@ function assertDuration(context: string, key: string, value: unknown): void {
570570
if (typeof value !== "string") {
571571
fail(`${context}: ${key} must be an ms-formatted string or a number of milliseconds`);
572572
}
573-
if (!MS_DURATION_PATTERN.test(value) || value.length > 100) {
573+
// Cheap length cap first: the regex is linear, but there is no reason to
574+
// run it over an arbitrarily long string when the cap rejects it anyway.
575+
if (value.length > 100 || !MS_DURATION_PATTERN.test(value)) {
574576
fail(
575577
`${context}: ${key} has invalid duration "${value}" — expected an ms-formatted string (a number followed by an optional unit ms/s/m/h/d/w/y or its long form, e.g. "30s", "5 minutes", "1.5h") or a number of milliseconds`,
576578
);

packages/contract/src/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ export type {
2828
InferErrorData,
2929
InferErrorDataInput,
3030
// Contract-level activity option defaults
31-
ActivityDefaultOptions,
31+
ContractActivityOptions,
3232
ActivityRetryPolicy,
3333
DurationValue,
3434
// Search attributes

packages/contract/src/types.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,8 @@ export type ActivityRetryPolicy = {
6464
};
6565

6666
/**
67-
* Contract-level default `ActivityOptions` for a single activity.
67+
* Contract-level default activity options for a single activity — the
68+
* portable subset of Temporal's `ActivityOptions`.
6869
*
6970
* Declared on `defineActivity` so operational behavior (timeouts, retry
7071
* policy) ships with the contract as a single source of truth shared by
@@ -80,7 +81,7 @@ export type ActivityRetryPolicy = {
8081
* deliberately excluded — those belong to the worker's
8182
* `activityOptionsByName`, not the portable contract.
8283
*/
83-
export type ActivityDefaultOptions = {
84+
export type ContractActivityOptions = {
8485
readonly startToCloseTimeout?: DurationValue;
8586
readonly scheduleToCloseTimeout?: DurationValue;
8687
readonly scheduleToStartTimeout?: DurationValue;
@@ -99,7 +100,7 @@ export type ActivityDefinition<
99100
readonly input: TInput;
100101
readonly output: TOutput;
101102
readonly errors?: TErrors;
102-
readonly activityOptions?: ActivityDefaultOptions;
103+
readonly activityOptions?: ContractActivityOptions;
103104
};
104105

105106
/**

0 commit comments

Comments
 (0)