Skip to content

Commit 4644d6a

Browse files
committed
address obvious symlink issue with fs sandbox; also make tests hopefully work on github
1 parent 732ee72 commit 4644d6a

File tree

10 files changed

+200
-51
lines changed

10 files changed

+200
-51
lines changed

src/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
"version-check": "pip3 install typing_extensions mypy || pip3 install --break-system-packages typing_extensions mypy && ./workspaces.py version-check && mypy scripts/check_npm_packages.py",
1919
"test-parallel": "unset DEBUG && pnpm run version-check && cd packages && pnpm run -r --parallel test",
2020
"test": "unset DEBUG && pnpm run depcheck && pnpm run version-check && ./workspaces.py test",
21-
"test-github-ci": "unset DEBUG && pnpm run depcheck && pnpm run version-check && ./workspaces.py test --exclude=jupyter --retries=1",
21+
"test-github-ci": "unset DEBUG && pnpm run depcheck && pnpm run version-check && ./workspaces.py test --test-github-ci --exclude=jupyter --retries=1",
2222
"depcheck": "cd packages && pnpm run -r --parallel depcheck",
2323
"prettier-all": "cd packages/",
2424
"local-ci": "./scripts/ci.sh",

src/packages/file-server/btrfs/filesystem.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@ export class Filesystem {
6969
args: ["quota", "enable", "--simple", this.opts.mount],
7070
});
7171
await this.initBup();
72+
await this.sync();
7273
};
7374

7475
sync = async () => {

src/packages/file-server/btrfs/subvolume-bup.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ export class SubvolumeBup {
4646
`createBackup: creating ${BUP_SNAPSHOT} to get a consistent backup`,
4747
);
4848
await this.subvolume.snapshots.create(BUP_SNAPSHOT);
49-
const target = this.subvolume.fs.safeAbsPath(
49+
const target = await this.subvolume.fs.safeAbsPath(
5050
this.subvolume.snapshots.path(BUP_SNAPSHOT),
5151
);
5252

@@ -133,9 +133,9 @@ export class SubvolumeBup {
133133
return v;
134134
}
135135

136-
path = this.subvolume.fs
137-
.safeAbsPath(path)
138-
.slice(this.subvolume.path.length);
136+
path = (await this.subvolume.fs.safeAbsPath(path)).slice(
137+
this.subvolume.path.length,
138+
);
139139
const { stdout } = await sudo({
140140
command: "bup",
141141
args: [

src/packages/file-server/btrfs/subvolume.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -86,8 +86,8 @@ export class Subvolume {
8686
target: string;
8787
timeout?: number;
8888
}): Promise<{ stdout: string; stderr: string; exit_code: number }> => {
89-
let srcPath = this.fs.safeAbsPath(src);
90-
let targetPath = this.fs.safeAbsPath(target);
89+
let srcPath = await this.fs.safeAbsPath(src);
90+
let targetPath = await this.fs.safeAbsPath(target);
9191
if (src.endsWith("/")) {
9292
srcPath += "/";
9393
}

src/packages/file-server/btrfs/test/filesystem-stress.test.ts

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,6 @@ describe("stress operations with subvolumes", () => {
4848
});
4949

5050
it("clone the first group in serial", async () => {
51-
await fs.sync(); // needed on github actions
5251
const t = Date.now();
5352
for (let i = 0; i < count1; i++) {
5453
await fs.subvolumes.clone(`${i}`, `clone-of-${i}`);
@@ -59,7 +58,6 @@ describe("stress operations with subvolumes", () => {
5958
});
6059

6160
it("clone the second group in parallel", async () => {
62-
await fs.sync(); // needed on github actions
6361
const t = Date.now();
6462
const v: any[] = [];
6563
for (let i = 0; i < count2; i++) {
@@ -94,7 +92,6 @@ describe("stress operations with subvolumes", () => {
9492
});
9593

9694
it("everything should be gone except the clones", async () => {
97-
await fs.sync();
9895
const v = await fs.subvolumes.list();
9996
expect(v.length).toBe(count1 + count2);
10097
});

src/packages/file-server/btrfs/test/subvolume.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,7 @@ describe("the filesystem operations", () => {
143143
await vol.fs.writeFile("w.txt", "hi");
144144
const ac = new AbortController();
145145
const { signal } = ac;
146-
const watcher = vol.fs.watch("w.txt", { signal });
146+
const watcher = await vol.fs.watch("w.txt", { signal });
147147
vol.fs.appendFile("w.txt", " there");
148148
// @ts-ignore
149149
const { value, done } = await watcher.next();

src/packages/file-server/conat/test/local-path.test.ts

Lines changed: 85 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,25 +1,28 @@
11
import { localPathFileserver } from "../local-path";
2-
import { mkdtemp, rm } from "node:fs/promises";
2+
import { mkdtemp, readFile, rm, symlink } from "node:fs/promises";
33
import { tmpdir } from "node:os";
44
import { join } from "path";
55
import { fsClient } from "@cocalc/conat/files/fs";
66
import { randomId } from "@cocalc/conat/names";
77
import { before, after, client } from "@cocalc/backend/conat/test/setup";
8+
import { uuid } from "@cocalc/util/misc";
89

910
let tempDir;
11+
let tempDir2;
1012
beforeAll(async () => {
1113
await before();
1214
tempDir = await mkdtemp(join(tmpdir(), "cocalc-local-path"));
15+
tempDir2 = await mkdtemp(join(tmpdir(), "cocalc-local-path-2"));
1316
});
1417

15-
describe("use the simple fileserver", () => {
18+
describe("use all the standard api functions of fs", () => {
1619
const service = `fs-${randomId()}`;
1720
let server;
1821
it("creates the simple fileserver service", async () => {
1922
server = await localPathFileserver({ client, service, path: tempDir });
2023
});
2124

22-
const project_id = "6b851643-360e-435e-b87e-f9a6ab64a8b1";
25+
const project_id = uuid();
2326
let fs;
2427
it("create a client", () => {
2528
fs = fsClient({ subject: `${service}.project-${project_id}` });
@@ -82,6 +85,27 @@ describe("use the simple fileserver", () => {
8285
expect(s.isFile()).toBe(false);
8386
});
8487

88+
it("readFile works", async () => {
89+
await fs.writeFile("a", Buffer.from([1, 2, 3]));
90+
const s = await fs.readFile("a");
91+
expect(s).toEqual(Buffer.from([1, 2, 3]));
92+
93+
await fs.writeFile("b.txt", "conat");
94+
const t = await fs.readFile("b.txt", "utf8");
95+
expect(t).toEqual("conat");
96+
});
97+
98+
it("readdir works", async () => {
99+
await fs.mkdir("dirtest");
100+
for (let i = 0; i < 5; i++) {
101+
await fs.writeFile(`dirtest/${i}`, `${i}`);
102+
}
103+
const fire = "🔥.txt";
104+
await fs.writeFile(join("dirtest", fire), "this is ️‍🔥!");
105+
const v = await fs.readdir("dirtest");
106+
expect(v).toEqual(["0", "1", "2", "3", "4", fire]);
107+
});
108+
85109
it("creating a symlink works (and using lstat)", async () => {
86110
await fs.writeFile("source1", "the source");
87111
await fs.symlink("source1", "target1");
@@ -99,15 +123,71 @@ describe("use the simple fileserver", () => {
99123
const stats0 = await fs.stat("source1");
100124
expect(stats0.isSymbolicLink()).toBe(false);
101125
});
102-
103-
104126

105127
it("closes the service", () => {
106128
server.close();
107129
});
108130
});
109131

132+
describe("security: dangerous symlinks can't be followed", () => {
133+
const service = `fs-${randomId()}`;
134+
let server;
135+
it("creates the simple fileserver service", async () => {
136+
server = await localPathFileserver({ client, service, path: tempDir2 });
137+
});
138+
139+
const project_id = uuid();
140+
const project_id2 = uuid();
141+
let fs, fs2;
142+
it("create two clients", () => {
143+
fs = fsClient({ subject: `${service}.project-${project_id}` });
144+
fs2 = fsClient({ subject: `${service}.project-${project_id2}` });
145+
});
146+
147+
it("create a secret in one", async () => {
148+
await fs.writeFile("password", "s3cr3t");
149+
await fs2.writeFile("a", "init");
150+
});
151+
152+
// This is setup bypassing security and is part of our threat model, due to users
153+
// having full access internally to their sandbox fs.
154+
it("directly create a file that is a symlink outside of the sandbox -- this should work", async () => {
155+
await symlink(
156+
join(tempDir2, project_id, "password"),
157+
join(tempDir2, project_id2, "link"),
158+
);
159+
const s = await readFile(join(tempDir2, project_id2, "link"), "utf8");
160+
expect(s).toBe("s3cr3t");
161+
});
162+
163+
it("fails to read the symlink content via the api", async () => {
164+
await expect(async () => {
165+
await fs2.readFile("link", "utf8");
166+
}).rejects.toThrow("outside of sandbox");
167+
});
168+
169+
it("directly create a relative symlink ", async () => {
170+
await symlink(
171+
join("..", project_id, "password"),
172+
join(tempDir2, project_id2, "link2"),
173+
);
174+
const s = await readFile(join(tempDir2, project_id2, "link2"), "utf8");
175+
expect(s).toBe("s3cr3t");
176+
});
177+
178+
it("fails to read the relative symlink content via the api", async () => {
179+
await expect(async () => {
180+
await fs2.readFile("link2", "utf8");
181+
}).rejects.toThrow("outside of sandbox");
182+
});
183+
184+
it("closes the server", () => {
185+
server.close();
186+
});
187+
});
188+
110189
afterAll(async () => {
111190
await after();
112191
await rm(tempDir, { force: true, recursive: true });
192+
// await rm(tempDir2, { force: true, recursive: true });
113193
});

0 commit comments

Comments
 (0)