Skip to content

Commit 2cebe68

Browse files
committed
fix(tasks)!: contain the server filesystem tasks by default
BREAKING CHANGE: `registerCommonTasks()` in the node and electron builds no longer registers `FileGrepTask`, `FileLoaderTask` or `FileSedTask`. Call the new `registerFileSystemTasks()` to restore them. Separately, a local path now has to resolve inside `config.roots`, which defaults to `[process.cwd()]` rather than to "anywhere"; pass explicit `roots`, or `allowAnyRoot: true`, to read outside the working directory. The classes are exported unchanged — only ambient registry availability and the containment default moved. Two states existed and neither contained anything. `resolveLocalFilePath` skipped containment whenever `roots` was `undefined`, on the stated grounds that "the enforced control is the `filesystem:read` entitlement". It is not: `TaskGraphRunner` consults an enforcer only under `enforceEntitlements`, and `ENTITLEMENT_ENFORCER` has no default factory, so the flag without a registered enforcer throws. Neither is a default. An embedder that called `registerCommonTasks()` and ran a graph therefore had a task in the ambient registry that read any path the process could open, and `assertResolvedPathDeclared` did not narrow it — it recomputes the path from the same input through the same resolver, so with no roots both sides agree and every path passes. `FileLoaderTask.server` was worse than the other two and is why the fix cannot stop at `roots`: it declared no entitlement at all, honoured no roots, and reached the filesystem through `url.slice(7)`, which neither percent-decodes nor rejects a `file://` host. It now runs the same resolver and carries the same `configSchema()` / static + instance `entitlements()` pair its siblings do, with `metadata.url` still the caller's path so the output shape is unchanged. The registration split is the half that survives untrusted input. A serialized node is built as `{...item.config, id, defaults}`, so a graph that names `FileGrepTask` supplies its own config and can state `roots: ["/"]` itself — no default this package picks constrains it. The only control left is the type not resolving. The cwd default is the defence for the other case, a trusting embedder that authored the graph itself. Root resolution is also order-independent now. `realpathSync(root)` ran inside the `some()` predicate, which short-circuits: `[good, missing]` returned true without ever resolving the broken root while `[missing, good]` threw, on identical input. Every root is resolved before any containment verdict, so a misconfigured one fails always rather than sometimes — the deterministic direction and the safer one. The ~35 existing server tests that constructed these tasks with no `roots` now state `roots: [testDir]`. That churn is the point: each one says which directory it means to read.
1 parent 9f251ca commit 2cebe68

16 files changed

Lines changed: 850 additions & 90 deletions

packages/tasks/README.md

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ A package of task types for common operations, workflow management, and data pro
1515
- [LambdaTask](#lambdatask)
1616
- [JsonTask](#jsontask)
1717
- [ArrayTask](#arraytask)
18+
- [Filesystem Tasks (server builds)](#filesystem-tasks-server-builds)
1819
- [Workflow Integration](#workflow-integration)
1920
- [Error Handling](#error-handling)
2021
- [Configuration](#configuration)
@@ -563,6 +564,58 @@ const result = await task.run();
563564
- Combination generation for multiple array inputs
564565
- Seamless single-value and array handling
565566

567+
## Filesystem Tasks (server builds)
568+
569+
The Node and Electron builds ship three tasks that read the local filesystem —
570+
`FileGrepTask`, `FileLoaderTask` and `FileSedTask`.
571+
572+
**They are not registered by default.** `registerCommonTasks()` deliberately
573+
leaves them out of `TaskRegistry`, because the registry is what a _deserialized_
574+
graph resolves a task type through, and a serialized node supplies its own
575+
`config` — so a graph naming `FileGrepTask` also names its own `roots`, and no
576+
default this package picks can constrain it. The only control that survives
577+
untrusted graph JSON is the task not being resolvable at all.
578+
579+
A host that intends to expose them opts in:
580+
581+
```typescript
582+
import { registerCommonTasks, registerFileSystemTasks } from "@workglow/tasks";
583+
584+
registerCommonTasks();
585+
// Only where every graph reaching the registry is trusted:
586+
registerFileSystemTasks();
587+
```
588+
589+
The classes are exported either way, so constructing one directly needs no
590+
registration:
591+
592+
```typescript
593+
import { FileGrepTask } from "@workglow/tasks";
594+
595+
const result = await new FileGrepTask({
596+
roots: ["/srv/data"], // defaults to [process.cwd()]
597+
defaults: { url: "/srv/data/app.log", pattern: "ERROR" },
598+
}).run();
599+
```
600+
601+
### Root containment
602+
603+
Each task resolves and `realpath`s a local path before opening it, then
604+
requires the result to sit inside one of `config.roots`. Omitting `roots` means
605+
`[process.cwd()]`**not** "anywhere on the host". The `filesystem:read`
606+
entitlement is not a substitute: it is only consulted when the embedder both
607+
runs the graph with `enforceEntitlements` and registers an
608+
`ENTITLEMENT_ENFORCER`, and neither is the default.
609+
610+
Set `allowAnyRoot: true` to read any path the process can open. It has to be
611+
said explicitly; leaving `roots` unset never implies it.
612+
613+
```typescript
614+
new FileGrepTask({ allowAnyRoot: true, defaults: { url, pattern } });
615+
```
616+
617+
An unresolvable entry in `roots` fails the call wherever it sits in the array.
618+
566619
## Workflow Integration
567620

568621
All tasks can be used standalone or integrated into workflows:

packages/tasks/src/electron.ts

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,37 @@ import { FileGrepTask } from "./task/FileGrepTask.server";
2222
import { FileLoaderTask } from "./task/FileLoaderTask.server";
2323
import { FileSedTask } from "./task/FileSedTask.server";
2424

25+
/**
26+
* Registers the tasks that are safe to have in the ambient registry on a
27+
* server.
28+
*
29+
* The three filesystem tasks are NOT among them, and their absence is the
30+
* point. `TaskRegistry` is what a deserialized graph resolves a task type
31+
* through, and a serialized node carries its own `config` — so a graph that
32+
* names `FileGrepTask` also states its own `roots`, and no default this
33+
* package picks can constrain it. The only control that survives untrusted
34+
* JSON is the task not being resolvable at all.
35+
*
36+
* A host that intends to expose them opts in with
37+
* {@link registerFileSystemTasks}. The classes are exported either way, so
38+
* constructing one directly is unchanged.
39+
*/
2540
export const registerCommonTasks = () => {
26-
const tasks = registerCommonTasksFn();
41+
return registerCommonTasksFn();
42+
};
43+
44+
/**
45+
* Adds the filesystem tasks to the ambient registry, making them resolvable by
46+
* type name — including from graph JSON the host did not author.
47+
*
48+
* Call it only where every graph that can reach the registry is trusted. Each
49+
* task still contains reads to its `config.roots`, which defaults to
50+
* `process.cwd()`, but a serialized node supplies its own config: registration
51+
* is the boundary, containment is the backstop behind it.
52+
*/
53+
export const registerFileSystemTasks = () => {
2754
TaskRegistry.registerTask(FileGrepTask);
2855
TaskRegistry.registerTask(FileLoaderTask);
2956
TaskRegistry.registerTask(FileSedTask);
30-
return [...tasks, FileGrepTask, FileLoaderTask, FileSedTask];
57+
return [FileGrepTask, FileLoaderTask, FileSedTask];
3158
};

packages/tasks/src/node.ts

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,37 @@ import { FileGrepTask } from "./task/FileGrepTask.server";
2424
import { FileLoaderTask } from "./task/FileLoaderTask.server";
2525
import { FileSedTask } from "./task/FileSedTask.server";
2626

27+
/**
28+
* Registers the tasks that are safe to have in the ambient registry on a
29+
* server.
30+
*
31+
* The three filesystem tasks are NOT among them, and their absence is the
32+
* point. `TaskRegistry` is what a deserialized graph resolves a task type
33+
* through, and a serialized node carries its own `config` — so a graph that
34+
* names `FileGrepTask` also states its own `roots`, and no default this
35+
* package picks can constrain it. The only control that survives untrusted
36+
* JSON is the task not being resolvable at all.
37+
*
38+
* A host that intends to expose them opts in with
39+
* {@link registerFileSystemTasks}. The classes are exported either way, so
40+
* constructing one directly is unchanged.
41+
*/
2742
export const registerCommonTasks = () => {
28-
const tasks = registerCommonTasksFn();
43+
return registerCommonTasksFn();
44+
};
45+
46+
/**
47+
* Adds the filesystem tasks to the ambient registry, making them resolvable by
48+
* type name — including from graph JSON the host did not author.
49+
*
50+
* Call it only where every graph that can reach the registry is trusted. Each
51+
* task still contains reads to its `config.roots`, which defaults to
52+
* `process.cwd()`, but a serialized node supplies its own config: registration
53+
* is the boundary, containment is the backstop behind it.
54+
*/
55+
export const registerFileSystemTasks = () => {
2956
TaskRegistry.registerTask(FileGrepTask);
3057
TaskRegistry.registerTask(FileLoaderTask);
3158
TaskRegistry.registerTask(FileSedTask);
32-
return [...tasks, FileGrepTask, FileLoaderTask, FileSedTask];
59+
return [FileGrepTask, FileLoaderTask, FileSedTask];
3360
};

packages/tasks/src/task/FileGrepTask.server.ts

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,14 @@ const fileGrepTaskConfigSchema = {
4444
type: "array",
4545
items: { type: "string" },
4646
title: "Roots",
47-
description: "Directories a local path must resolve inside (after symlink resolution)",
47+
description:
48+
"Directories a local path must resolve inside (after symlink resolution). Defaults to the process working directory",
49+
"x-ui-hidden": true,
50+
},
51+
allowAnyRoot: {
52+
type: "boolean",
53+
title: "Allow Any Root",
54+
description: "Skip root containment entirely and read any path the process can open",
4855
"x-ui-hidden": true,
4956
},
5057
},
@@ -61,8 +68,10 @@ const fileGrepTaskConfigSchema = {
6168
* so a file with no line terminator cannot exhaust memory.
6269
*
6370
* A local path is resolved and realpath'd before it is opened, and constrained
64-
* to `config.roots` when the embedder sets them. Reading requires the
65-
* `filesystem:read` entitlement, declared scoped to the resolved path.
71+
* to `config.roots` — which defaults to the process working directory, so a
72+
* task with no stated root reads from there and nowhere else. Set
73+
* `config.allowAnyRoot` to opt out. Reading requires the `filesystem:read`
74+
* entitlement, declared scoped to the resolved path.
6675
*
6776
* Only available in Node.js and Bun environments. For cross-platform grep
6877
* (including browser), use FileGrepTask with an http(s) URL.
@@ -112,7 +121,12 @@ export class FileGrepTask extends BaseFileGrepTask<FileGrepTaskConfig> {
112121
{
113122
id: Entitlements.FILESYSTEM_READ,
114123
reason: "Reads a local file from disk",
115-
resources: [resolveLocalFilePath(url, { roots: this.config.roots })],
124+
resources: [
125+
resolveLocalFilePath(url, {
126+
roots: this.config.roots,
127+
allowAnyRoot: this.config.allowAnyRoot,
128+
}),
129+
],
116130
},
117131
],
118132
};
@@ -189,7 +203,10 @@ export class FileGrepTask extends BaseFileGrepTask<FileGrepTaskConfig> {
189203
}
190204
await context.updateProgress(0, "Opening file");
191205

192-
const file = resolveLocalFilePath(url, { roots: this.config.roots });
206+
const file = resolveLocalFilePath(url, {
207+
roots: this.config.roots,
208+
allowAnyRoot: this.config.allowAnyRoot,
209+
});
193210
this.assertResolvedPathDeclared(file);
194211

195212
if (context.signal.aborted) {

packages/tasks/src/task/FileGrepTask.ts

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -694,13 +694,21 @@ export class FileGrepTask<Config extends TaskConfig = TaskConfig> extends Task<
694694
export interface FileGrepTaskConfig extends TaskConfig {
695695
/**
696696
* Directories a local path must resolve inside, checked after symlinks are
697-
* resolved. Omitted means unrestricted: the enforced control is the
698-
* `filesystem:read` entitlement, and this is the embedder's extra fence.
697+
* resolved. Omitted means `[process.cwd()]`, NOT unrestricted — the
698+
* `filesystem:read` entitlement is only consulted when the embedder runs
699+
* with `enforceEntitlements` and a registered enforcer, so it cannot stand
700+
* in as the default fence. State {@link allowAnyRoot} to opt out.
699701
*
700702
* Honored only by the server build — the cross-platform class reaches
701703
* http(s) through `FetchUrlTask` and touches no filesystem.
702704
*/
703705
readonly roots?: readonly string[] | undefined;
706+
/**
707+
* Read any path the process can open, skipping containment entirely. Only
708+
* the literal `true` does so, and it is never implied by leaving `roots`
709+
* unset.
710+
*/
711+
readonly allowAnyRoot?: boolean | undefined;
704712
}
705713

706714
export const fileGrep = (input: FileGrepTaskInput, config?: FileGrepTaskConfig) => {

0 commit comments

Comments
 (0)