chore(server): final-review polish — stale CI refs, dockerignore key, boundary pins, zero-byte tempfile contract case

This commit is contained in:
jubnl
2026-08-19 22:46:52 +02:00
parent 6fbff83592
commit f86e55f104
6 changed files with 36 additions and 5 deletions
+1
View File
@@ -16,6 +16,7 @@ chart/
docs/
docker-compose.yml
docker-compose.minio-test.yml
minio-licence.key
unraid-template.xml
*.sqlite
*.sqlite-shm
+9 -2
View File
@@ -84,7 +84,14 @@ const S3_VARS = [
'TREK_S3_RETRIES',
'TREK_S3_TIMEOUT_MS',
] as const;
const S3_REQUIRED = S3_VARS.slice(0, 4);
// Explicit, not `S3_VARS.slice(0, 4)` — the required set must not silently
// shift if S3_VARS is ever reordered.
const S3_REQUIRED: (typeof S3_VARS)[number][] = [
'TREK_S3_ENDPOINT',
'TREK_S3_BUCKET',
'TREK_S3_ACCESS_KEY_ID',
'TREK_S3_SECRET_ACCESS_KEY',
];
/**
* All-or-nothing: any TREK_S3_* set means the operator wants off-box storage;
@@ -92,7 +99,7 @@ const S3_REQUIRED = S3_VARS.slice(0, 4);
* Prefix shape delegates to storage-keys — the single source of key rules.
*/
function s3Preconditions(raw: RawEnv): string[] {
const present = (key: string): boolean => Boolean(raw[key]?.trim());
const present = (key: (typeof S3_VARS)[number]): boolean => Boolean(raw[key]?.trim());
if (!S3_VARS.some(present)) return [];
const problems: string[] = [];
for (const key of S3_REQUIRED) {
+8 -1
View File
@@ -124,7 +124,8 @@ function isNotFound(err: unknown): boolean {
* `signedHeaders()` (aws4.js:311-322, which just map/join whatever
* `filterHeaders()` produced) sign `content-length` *twice*
* (`SignedHeaders=content-length;content-length;...`, confirmed live
* against MinIO in the repro below). MinIO's own canonicalization of the
* against MinIO (see the manual procedure in docker-compose.minio-test.yml)).
* MinIO's own canonicalization of the
* headers it actually received doesn't reproduce that duplicate, so the
* signatures disagree — 403. Any non-empty body makes
* `headers['content-length']` a truthy number, so aws4's guard never fires
@@ -480,6 +481,12 @@ function peekStream(source: Readable, limit: number): Promise<{ chunks: Buffer[]
const chunk = Buffer.isBuffer(raw) ? raw : Buffer.from(raw);
chunks.push(chunk);
size += chunk.length;
// Strictly `>`, not `>=`: a stream ending at exactly `limit` bytes is
// "ended within the limit" and buffers to PutObject, while a
// LocalTempFile of exactly `MULTIPART_THRESHOLD` (see put()'s `size <
// MULTIPART_THRESHOLD` check) goes to Upload instead. Both boundaries
// are correct on their own terms — deliberate asymmetry, not a bug;
// do not "align" them.
if (size > limit) {
cleanup();
resolve({ chunks, size, ended: false });
@@ -1,6 +1,7 @@
import { describe, it, expect, vi, afterEach } from 'vitest';
import { validateEnvAtBoot, readEnv } from '../../../src/app-config/env';
import { deriveS3 } from '../../../src/app-config/derive';
describe('validateEnvAtBoot', () => {
afterEach(() => {
@@ -193,6 +194,11 @@ describe('validateEnvAtBoot — S3 all-or-nothing preconditions', () => {
/Invalid environment configuration \(2 problems\)/,
);
});
it('S3-BOOT-006: a slash-wrapped key prefix passes boot validation (shape-checked stripped of its slashes), while deriveS3 keeps it un-normalized', () => {
const withPrefix = { ...full, TREK_S3_KEY_PREFIX: '/trek/prod/' };
expect(() => validateEnvAtBoot(withPrefix)).not.toThrow();
expect(deriveS3(withPrefix).keyPrefix).toBe('/trek/prod/');
});
});
describe('readEnv', () => {
@@ -8,8 +8,8 @@ import { S3Driver } from '../../../../src/nest/storage/drivers/s3.driver';
import { describeStorageDriver, type DriverHarness } from './storage-driver.contract';
/**
* Env-gated: engages only when TREK_TEST_S3_* is set (CI's MinIO job; locally
* via `docker compose -f docker-compose.minio-test.yml up`). Local runs stay
* Env-gated: engages only when TREK_TEST_S3_* is set (manual-only, via
* `docker compose -f docker-compose.minio-test.yml up`). Local runs stay
* hermetic. Deviation from local/mirror's bare top-level call: the gate must
* wrap describeStorageDriver because the contract file owns its own describe.
*/
@@ -98,6 +98,16 @@ export function describeStorageDriver(name: string, makeHarness: () => Promise<D
expect(await h.driver.stat('a/empty.bin')).toBeNull();
});
it('roundtrips a zero-byte LocalTempFile source and consumes the temp file', async () => {
const tmp = await h.makeTempFile('');
await h.driver.put('a/empty-tmpfile.bin', { tmpPath: tmp });
expect((await readAll('a/empty-tmpfile.bin')).length).toBe(0);
expect((await h.driver.stat('a/empty-tmpfile.bin'))!.size).toBe(0);
expect(fs.existsSync(tmp)).toBe(false); // ownership transferred
await h.driver.delete('a/empty-tmpfile.bin');
expect(await h.driver.stat('a/empty-tmpfile.bin')).toBeNull();
});
it('rejects getStream on a missing key with StorageNotFoundError', async () => {
await expect(h.driver.getStream('a/nope.bin')).rejects.toBeInstanceOf(StorageNotFoundError);
});