Skip to content

Commit 02cc5f2

Browse files
committed
finish testing conat fs interface and fix some subtle issues with stat found when testing
1 parent 4644d6a commit 02cc5f2

File tree

3 files changed

+164
-12
lines changed

3 files changed

+164
-12
lines changed

src/packages/conat/files/fs.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,16 @@ export async function fsServer({ service, fs, client }: Options) {
148148
await (await fs(this.subject)).rmdir(path, options);
149149
},
150150
async stat(path: string): Promise<IStats> {
151-
return await (await fs(this.subject)).stat(path);
151+
const s = await (await fs(this.subject)).stat(path);
152+
return {
153+
...s,
154+
// for some reason these times get corrupted on transport from the nodejs datastructure,
155+
// so we make them standard Date objects.
156+
atime: new Date(s.atime),
157+
mtime: new Date(s.mtime),
158+
ctime: new Date(s.ctime),
159+
birthtime: new Date(s.birthtime),
160+
};
152161
},
153162
async symlink(target: string, path: string) {
154163
await (await fs(this.subject)).symlink(target, path);

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,7 @@ describe("the filesystem operations", () => {
126126
it("make a file readonly, then change it back", async () => {
127127
await vol.fs.writeFile("c.txt", "hi");
128128
await vol.fs.chmod("c.txt", "440");
129+
await fs.sync();
129130
expect(async () => {
130131
await vol.fs.appendFile("c.txt", " there");
131132
}).rejects.toThrow("EACCES");
@@ -219,6 +220,7 @@ describe("test snapshots", () => {
219220
});
220221

221222
it("unlock our snapshot and delete it", async () => {
223+
await fs.sync();
222224
await vol.snapshots.unlock("snap1");
223225
await vol.snapshots.delete("snap1");
224226
expect(await vol.snapshots.exists("snap1")).toBe(false);

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

Lines changed: 152 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { localPathFileserver } from "../local-path";
2-
import { mkdtemp, readFile, rm, symlink } from "node:fs/promises";
2+
import { link, 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";
@@ -106,7 +106,131 @@ describe("use all the standard api functions of fs", () => {
106106
expect(v).toEqual(["0", "1", "2", "3", "4", fire]);
107107
});
108108

109-
it("creating a symlink works (and using lstat)", async () => {
109+
it("realpath works", async () => {
110+
await fs.writeFile("file0", "file0");
111+
await fs.symlink("file0", "file1");
112+
expect(await fs.readFile("file1", "utf8")).toBe("file0");
113+
const r = await fs.realpath("file1");
114+
expect(r).toBe("file0");
115+
116+
await fs.writeFile("file2", "file2");
117+
await fs.link("file2", "file3");
118+
expect(await fs.readFile("file3", "utf8")).toBe("file2");
119+
const r3 = await fs.realpath("file3");
120+
expect(r3).toBe("file3");
121+
});
122+
123+
it("rename a file", async () => {
124+
await fs.writeFile("bella", "poo");
125+
await fs.rename("bella", "bells");
126+
expect(await fs.readFile("bells", "utf8")).toBe("poo");
127+
await fs.mkdir("x");
128+
await fs.rename("bells", "x/belltown");
129+
});
130+
131+
it("rm a file", async () => {
132+
await fs.writeFile("bella-to-rm", "poo");
133+
await fs.rm("bella-to-rm");
134+
expect(await fs.exists("bella-to-rm")).toBe(false);
135+
});
136+
137+
it("rm a directory", async () => {
138+
await fs.mkdir("rm-dir");
139+
expect(async () => {
140+
await fs.rm("rm-dir");
141+
}).rejects.toThrow("Path is a directory");
142+
await fs.rm("rm-dir", { recursive: true });
143+
expect(await fs.exists("rm-dir")).toBe(false);
144+
});
145+
146+
it("rm a nonempty directory", async () => {
147+
await fs.mkdir("rm-dir2");
148+
await fs.writeFile("rm-dir2/a", "a");
149+
await fs.rm("rm-dir2", { recursive: true });
150+
expect(await fs.exists("rm-dir2")).toBe(false);
151+
});
152+
153+
it("rmdir empty directory", async () => {
154+
await fs.mkdir("rm-dir3");
155+
await fs.rmdir("rm-dir3");
156+
expect(await fs.exists("rm-dir3")).toBe(false);
157+
});
158+
159+
it("stat not existing path", async () => {
160+
expect(async () => {
161+
await fs.stat(randomId());
162+
}).rejects.toThrow("no such file or directory");
163+
});
164+
165+
it("stat a file", async () => {
166+
await fs.writeFile("abc.txt", "hi");
167+
const stat = await fs.stat("abc.txt");
168+
expect(stat.size).toBe(2);
169+
expect(stat.isFile()).toBe(true);
170+
expect(stat.isSymbolicLink()).toBe(false);
171+
expect(stat.isDirectory()).toBe(false);
172+
expect(stat.isBlockDevice()).toBe(false);
173+
expect(stat.isCharacterDevice()).toBe(false);
174+
expect(stat.isSymbolicLink()).toBe(false);
175+
expect(stat.isFIFO()).toBe(false);
176+
expect(stat.isSocket()).toBe(false);
177+
});
178+
179+
it("stat a directory", async () => {
180+
await fs.mkdir("my-stat-dir");
181+
const stat = await fs.stat("my-stat-dir");
182+
expect(stat.isFile()).toBe(false);
183+
expect(stat.isSymbolicLink()).toBe(false);
184+
expect(stat.isDirectory()).toBe(true);
185+
expect(stat.isBlockDevice()).toBe(false);
186+
expect(stat.isCharacterDevice()).toBe(false);
187+
expect(stat.isSymbolicLink()).toBe(false);
188+
expect(stat.isFIFO()).toBe(false);
189+
expect(stat.isSocket()).toBe(false);
190+
});
191+
192+
it("stat a symlink", async () => {
193+
await fs.writeFile("sl2", "the source");
194+
await fs.symlink("sl2", "target-sl2");
195+
const stat = await fs.stat("target-sl2");
196+
// this is how stat works!
197+
expect(stat.isFile()).toBe(true);
198+
expect(stat.isSymbolicLink()).toBe(false);
199+
// so use lstat
200+
const lstat = await fs.lstat("target-sl2");
201+
expect(lstat.isFile()).toBe(false);
202+
expect(lstat.isSymbolicLink()).toBe(true);
203+
});
204+
205+
it("truncate a file", async () => {
206+
await fs.writeFile("t", "");
207+
await fs.truncate("t", 10);
208+
const s = await fs.stat("t");
209+
expect(s.size).toBe(10);
210+
});
211+
212+
it("delete a file with unlink", async () => {
213+
await fs.writeFile("to-unlink", "");
214+
await fs.unlink("to-unlink");
215+
expect(await fs.exists("to-unlink")).toBe(false);
216+
});
217+
218+
it("sets times of a file", async () => {
219+
await fs.writeFile("my-times", "");
220+
const statsBefore = await fs.stat("my-times");
221+
const atime = Date.now() - 100_000;
222+
const mtime = Date.now() - 10_000_000;
223+
// NOTE: fs.utimes in nodejs takes *seconds*, not ms, hence
224+
// dividing by 1000 here:
225+
await fs.utimes("my-times", atime / 1000, mtime / 1000);
226+
const s = await fs.stat("my-times");
227+
expect(s.atimeMs).toBeCloseTo(atime);
228+
expect(s.mtimeMs).toBeCloseTo(mtime);
229+
expect(s.atime.valueOf()).toBeCloseTo(atime);
230+
expect(s.mtime.valueOf()).toBeCloseTo(mtime);
231+
});
232+
233+
it("creating a symlink works (as does using lstat)", async () => {
110234
await fs.writeFile("source1", "the source");
111235
await fs.symlink("source1", "target1");
112236
expect(await fs.readFile("target1", "utf8")).toEqual("the source");
@@ -151,36 +275,53 @@ describe("security: dangerous symlinks can't be followed", () => {
151275

152276
// This is setup bypassing security and is part of our threat model, due to users
153277
// 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 () => {
278+
it("directly create a dangerous file that is a symlink outside of the sandbox -- this should work", async () => {
155279
await symlink(
156280
join(tempDir2, project_id, "password"),
157-
join(tempDir2, project_id2, "link"),
281+
join(tempDir2, project_id2, "danger"),
158282
);
159-
const s = await readFile(join(tempDir2, project_id2, "link"), "utf8");
283+
const s = await readFile(join(tempDir2, project_id2, "danger"), "utf8");
160284
expect(s).toBe("s3cr3t");
161285
});
162286

163287
it("fails to read the symlink content via the api", async () => {
164288
await expect(async () => {
165-
await fs2.readFile("link", "utf8");
289+
await fs2.readFile("danger", "utf8");
166290
}).rejects.toThrow("outside of sandbox");
167291
});
168292

169-
it("directly create a relative symlink ", async () => {
293+
it("directly create a dangerous relative symlink ", async () => {
170294
await symlink(
171295
join("..", project_id, "password"),
172-
join(tempDir2, project_id2, "link2"),
296+
join(tempDir2, project_id2, "danger2"),
173297
);
174-
const s = await readFile(join(tempDir2, project_id2, "link2"), "utf8");
298+
const s = await readFile(join(tempDir2, project_id2, "danger2"), "utf8");
175299
expect(s).toBe("s3cr3t");
176300
});
177301

178302
it("fails to read the relative symlink content via the api", async () => {
179303
await expect(async () => {
180-
await fs2.readFile("link2", "utf8");
304+
await fs2.readFile("danger2", "utf8");
181305
}).rejects.toThrow("outside of sandbox");
182306
});
183307

308+
// This is not a vulnerability, because there's no way for the user
309+
// to create a hard link like this from within an nfs mount (say)
310+
// of their own folder.
311+
it("directly create a hard link", async () => {
312+
await link(
313+
join(tempDir2, project_id, "password"),
314+
join(tempDir2, project_id2, "danger3"),
315+
);
316+
const s = await readFile(join(tempDir2, project_id2, "danger3"), "utf8");
317+
expect(s).toBe("s3cr3t");
318+
});
319+
320+
it("a hardlink *can* get outside the sandbox", async () => {
321+
const s = await fs2.readFile("danger3", "utf8");
322+
expect(s).toBe("s3cr3t");
323+
});
324+
184325
it("closes the server", () => {
185326
server.close();
186327
});
@@ -189,5 +330,5 @@ describe("security: dangerous symlinks can't be followed", () => {
189330
afterAll(async () => {
190331
await after();
191332
await rm(tempDir, { force: true, recursive: true });
192-
// await rm(tempDir2, { force: true, recursive: true });
333+
await rm(tempDir2, { force: true, recursive: true });
193334
});

0 commit comments

Comments
 (0)