assafvayner HF Staff commited on
Commit
874bd3c
·
1 Parent(s): 0d57d79

fix(storage): atomic temp files, directory-safe get/delete, list contract docs

Browse files
src/lib/server/storage/local-fs-store.ts CHANGED
@@ -1,9 +1,10 @@
 
1
  import { appendFile, copyFile, mkdir, readdir, readFile, rename, rm, stat, writeFile } from 'node:fs/promises';
2
  import { dirname, join, relative, sep } from 'node:path';
3
  import { assertValidPath, type BucketStore, type StoreEntry } from './store';
4
 
5
- function isEnoent(e: unknown): boolean {
6
- return typeof e === 'object' && e !== null && (e as { code?: string }).code === 'ENOENT';
7
  }
8
 
9
  /** Directory-backed store for local development and tests. Writes are atomic (temp file + rename). */
@@ -19,7 +20,7 @@ export class LocalFsStore implements BucketStore {
19
  try {
20
  return new Uint8Array(await readFile(this.abs(path)));
21
  } catch (e) {
22
- if (isEnoent(e)) return null;
23
  throw e;
24
  }
25
  }
@@ -27,9 +28,14 @@ export class LocalFsStore implements BucketStore {
27
  async put(path: string, bytes: Uint8Array, contentType?: string): Promise<void> {
28
  const target = this.abs(path);
29
  await mkdir(dirname(target), { recursive: true });
30
- const tmp = `${target}.${process.pid}.${Date.now()}.tmp`;
31
- await writeFile(tmp, bytes);
32
- await rename(tmp, target);
 
 
 
 
 
33
  }
34
 
35
  async append(path: string, bytes: Uint8Array): Promise<void> {
@@ -45,15 +51,18 @@ export class LocalFsStore implements BucketStore {
45
  try {
46
  entries = await readdir(dir, { withFileTypes: true });
47
  } catch (e) {
48
- if (isEnoent(e)) return;
49
  throw e;
50
  }
51
  for (const ent of entries) {
52
  const full = join(dir, ent.name);
 
53
  if (ent.isDirectory()) {
54
- await walk(full);
 
 
 
55
  } else if (ent.isFile() && !ent.name.endsWith('.tmp')) {
56
- const rel = relative(this.root, full).split(sep).join('/');
57
  if (rel.startsWith(prefix)) {
58
  const s = await stat(full);
59
  out.push({ path: rel, size: s.size, uploadedAt: s.mtime });
@@ -66,12 +75,26 @@ export class LocalFsStore implements BucketStore {
66
  }
67
 
68
  async delete(paths: string[]): Promise<void> {
69
- for (const p of paths) await rm(this.abs(p), { force: true });
 
 
 
 
 
 
 
70
  }
71
 
72
  async copy(src: string, dst: string): Promise<void> {
73
  const target = this.abs(dst);
74
  await mkdir(dirname(target), { recursive: true });
75
- await copyFile(this.abs(src), target);
 
 
 
 
 
 
 
76
  }
77
  }
 
1
+ import { randomUUID } from 'node:crypto';
2
  import { appendFile, copyFile, mkdir, readdir, readFile, rename, rm, stat, writeFile } from 'node:fs/promises';
3
  import { dirname, join, relative, sep } from 'node:path';
4
  import { assertValidPath, type BucketStore, type StoreEntry } from './store';
5
 
6
+ function hasCode(e: unknown, ...codes: string[]): boolean {
7
+ return typeof e === 'object' && e !== null && codes.includes((e as { code?: string }).code ?? '');
8
  }
9
 
10
  /** Directory-backed store for local development and tests. Writes are atomic (temp file + rename). */
 
20
  try {
21
  return new Uint8Array(await readFile(this.abs(path)));
22
  } catch (e) {
23
+ if (hasCode(e, 'ENOENT', 'EISDIR')) return null;
24
  throw e;
25
  }
26
  }
 
28
  async put(path: string, bytes: Uint8Array, contentType?: string): Promise<void> {
29
  const target = this.abs(path);
30
  await mkdir(dirname(target), { recursive: true });
31
+ const tmp = `${target}.${randomUUID()}.tmp`;
32
+ try {
33
+ await writeFile(tmp, bytes);
34
+ await rename(tmp, target);
35
+ } catch (e) {
36
+ await rm(tmp, { force: true });
37
+ throw e;
38
+ }
39
  }
40
 
41
  async append(path: string, bytes: Uint8Array): Promise<void> {
 
51
  try {
52
  entries = await readdir(dir, { withFileTypes: true });
53
  } catch (e) {
54
+ if (hasCode(e, 'ENOENT')) return;
55
  throw e;
56
  }
57
  for (const ent of entries) {
58
  const full = join(dir, ent.name);
59
+ const rel = relative(this.root, full).split(sep).join('/');
60
  if (ent.isDirectory()) {
61
+ const relDir = `${rel}/`;
62
+ if (relDir.startsWith(prefix) || prefix.startsWith(relDir)) {
63
+ await walk(full);
64
+ }
65
  } else if (ent.isFile() && !ent.name.endsWith('.tmp')) {
 
66
  if (rel.startsWith(prefix)) {
67
  const s = await stat(full);
68
  out.push({ path: rel, size: s.size, uploadedAt: s.mtime });
 
75
  }
76
 
77
  async delete(paths: string[]): Promise<void> {
78
+ for (const p of paths) {
79
+ try {
80
+ await rm(this.abs(p), { force: true });
81
+ } catch (e) {
82
+ if (hasCode(e, 'EISDIR', 'ERR_FS_EISDIR')) continue;
83
+ throw e;
84
+ }
85
+ }
86
  }
87
 
88
  async copy(src: string, dst: string): Promise<void> {
89
  const target = this.abs(dst);
90
  await mkdir(dirname(target), { recursive: true });
91
+ const tmp = `${target}.${randomUUID()}.tmp`;
92
+ try {
93
+ await copyFile(this.abs(src), tmp);
94
+ await rename(tmp, target);
95
+ } catch (e) {
96
+ await rm(tmp, { force: true });
97
+ throw e;
98
+ }
99
  }
100
  }
src/lib/server/storage/prefixed-store.ts CHANGED
@@ -21,23 +21,23 @@ export class PrefixedStore implements BucketStore {
21
  return this.prefix + path;
22
  }
23
 
24
- get(path: string): Promise<Uint8Array | null> {
25
  return this.inner.get(this.full(path));
26
  }
27
- put(path: string, bytes: Uint8Array, contentType?: string): Promise<void> {
28
  return this.inner.put(this.full(path), bytes, contentType);
29
  }
30
- append(path: string, bytes: Uint8Array): Promise<void> {
31
  return this.inner.append(this.full(path), bytes);
32
  }
33
  async list(prefix: string): Promise<StoreEntry[]> {
34
  const entries = await this.inner.list(this.prefix + prefix);
35
  return entries.map((e) => ({ ...e, path: e.path.slice(this.prefix.length) }));
36
  }
37
- delete(paths: string[]): Promise<void> {
38
  return this.inner.delete(paths.map((p) => this.full(p)));
39
  }
40
- copy(src: string, dst: string): Promise<void> {
41
  return this.inner.copy(this.full(src), this.full(dst));
42
  }
43
 
 
21
  return this.prefix + path;
22
  }
23
 
24
+ async get(path: string): Promise<Uint8Array | null> {
25
  return this.inner.get(this.full(path));
26
  }
27
+ async put(path: string, bytes: Uint8Array, contentType?: string): Promise<void> {
28
  return this.inner.put(this.full(path), bytes, contentType);
29
  }
30
+ async append(path: string, bytes: Uint8Array): Promise<void> {
31
  return this.inner.append(this.full(path), bytes);
32
  }
33
  async list(prefix: string): Promise<StoreEntry[]> {
34
  const entries = await this.inner.list(this.prefix + prefix);
35
  return entries.map((e) => ({ ...e, path: e.path.slice(this.prefix.length) }));
36
  }
37
+ async delete(paths: string[]): Promise<void> {
38
  return this.inner.delete(paths.map((p) => this.full(p)));
39
  }
40
+ async copy(src: string, dst: string): Promise<void> {
41
  return this.inner.copy(this.full(src), this.full(dst));
42
  }
43
 
src/lib/server/storage/store.ts CHANGED
@@ -13,9 +13,17 @@ export interface BucketStore {
13
  get(path: string): Promise<Uint8Array | null>;
14
  /** Creates or overwrites. */
15
  put(path: string, bytes: Uint8Array, contentType?: string): Promise<void>;
16
- /** Appends bytes to an existing object, creating it if missing. */
 
 
 
 
17
  append(path: string, bytes: Uint8Array): Promise<void>;
18
- /** Lists files whose path starts with `prefix`, sorted by path. */
 
 
 
 
19
  list(prefix: string): Promise<StoreEntry[]>;
20
  /** Deletes objects; missing ones are ignored. */
21
  delete(paths: string[]): Promise<void>;
@@ -28,6 +36,7 @@ const SEGMENT_RE = /^[A-Za-z0-9._-]+$/;
28
  export function assertValidPath(path: string): void {
29
  if (path.length === 0 || path.length > 512) throw new Error(`Invalid path: ${path}`);
30
  if (path.startsWith('/') || path.endsWith('/')) throw new Error(`Invalid path: ${path}`);
 
31
  for (const seg of path.split('/')) {
32
  if (seg === '' || seg === '.' || seg === '..' || !SEGMENT_RE.test(seg)) throw new Error(`Invalid path: ${path}`);
33
  }
 
13
  get(path: string): Promise<Uint8Array | null>;
14
  /** Creates or overwrites. */
15
  put(path: string, bytes: Uint8Array, contentType?: string): Promise<void>;
16
+ /**
17
+ * Appends bytes to an existing object, creating it if missing. Not atomic on every
18
+ * backend: a crash mid-append may leave a truncated final line, so readers must
19
+ * tolerate a trailing partial line.
20
+ */
21
  append(path: string, bytes: Uint8Array): Promise<void>;
22
+ /**
23
+ * Lists files whose path starts with `prefix`, sorted by path. The prefix is matched
24
+ * literally against the full path (`'events'` also matches `eventsX/…`); callers
25
+ * should pass a trailing `/` to match only within a directory.
26
+ */
27
  list(prefix: string): Promise<StoreEntry[]>;
28
  /** Deletes objects; missing ones are ignored. */
29
  delete(paths: string[]): Promise<void>;
 
36
  export function assertValidPath(path: string): void {
37
  if (path.length === 0 || path.length > 512) throw new Error(`Invalid path: ${path}`);
38
  if (path.startsWith('/') || path.endsWith('/')) throw new Error(`Invalid path: ${path}`);
39
+ if (path.endsWith('.tmp')) throw new Error(`Invalid path: ${path}`);
40
  for (const seg of path.split('/')) {
41
  if (seg === '' || seg === '.' || seg === '..' || !SEGMENT_RE.test(seg)) throw new Error(`Invalid path: ${path}`);
42
  }
tests/unit/storage/local-fs-store.test.ts CHANGED
@@ -59,4 +59,44 @@ describe('LocalFsStore', () => {
59
  expect(() => assertValidPath('a//b')).toThrow();
60
  expect(() => assertValidPath('ok/path.json')).not.toThrow();
61
  });
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
62
  });
 
59
  expect(() => assertValidPath('a//b')).toThrow();
60
  expect(() => assertValidPath('ok/path.json')).not.toThrow();
61
  });
62
+
63
+ it('enforces path validation on store methods', async () => {
64
+ await expect(store.get('../x')).rejects.toThrow();
65
+ await expect(store.put('../x', enc('x'))).rejects.toThrow();
66
+ });
67
+
68
+ it('rejects paths ending in .tmp', async () => {
69
+ await expect(store.put('note.tmp', enc('x'))).rejects.toThrow();
70
+ });
71
+
72
+ it('handles concurrent puts to the same key', async () => {
73
+ const values = Array.from({ length: 20 }, (_, i) => `value-${i}`);
74
+ await Promise.all(values.map((v) => store.put('concurrent.json', enc(v))));
75
+ const final = dec(await store.get('concurrent.json'));
76
+ expect(values).toContain(final);
77
+ const entries = await store.list('');
78
+ expect(entries.some((e) => e.path.endsWith('.tmp'))).toBe(false);
79
+ });
80
+
81
+ it('leaves no temp file behind when a write fails', async () => {
82
+ await store.put('obj/x', enc('x'));
83
+ await expect(store.put('obj', enc('y'))).rejects.toThrow();
84
+ const entries = await store.list('');
85
+ expect(entries.some((e) => e.path.endsWith('.tmp'))).toBe(false);
86
+ });
87
+
88
+ it('distinguishes list("events") from list("events/")', async () => {
89
+ await store.put('events/a.json', enc('a'));
90
+ await store.put('eventsX/b.json', enc('b'));
91
+ const withSlash = await store.list('events/');
92
+ expect(withSlash.map((e) => e.path)).toEqual(['events/a.json']);
93
+ const withoutSlash = await store.list('events');
94
+ expect(withoutSlash.map((e) => e.path)).toEqual(['events/a.json', 'eventsX/b.json']);
95
+ });
96
+
97
+ it('treats a directory path as missing for get and delete', async () => {
98
+ await store.put('adir/x.json', enc('x'));
99
+ expect(await store.get('adir')).toBeNull();
100
+ await expect(store.delete(['adir'])).resolves.not.toThrow();
101
+ });
102
  });
tests/unit/storage/prefixed-store.test.ts CHANGED
@@ -57,4 +57,9 @@ describe('PrefixedStore', () => {
57
  expect(() => new PrefixedStore(inner, '')).toThrow();
58
  expect(() => new PrefixedStore(inner, '../x')).toThrow();
59
  });
 
 
 
 
 
60
  });
 
57
  expect(() => new PrefixedStore(inner, '')).toThrow();
58
  expect(() => new PrefixedStore(inner, '../x')).toThrow();
59
  });
60
+
61
+ it('rejects paths that escape the prefix', async () => {
62
+ await expect(a.get('../b/z.txt')).rejects.toThrow();
63
+ await expect(a.put('../../x', encodeText('x'))).rejects.toThrow();
64
+ });
65
  });