Skip to content

Commit 2a310a4

Browse files
fix: prevent createdAt forgery, customCreatedAt leak, and password drop in video duplicate (#2223)
- Enumerate explicit Video creation payload in Videos.duplicate instead of spreading the Video entity - Strip customCreatedAt and desktopRecordingUpload from metadata on duplication - Retain password protection on duplicated videos when the source video has a password - Exclude createdAt, updatedAt, and id in VideosRepo.create so database default timestamps are always applied Closes #2223
1 parent e018b68 commit 2a310a4

3 files changed

Lines changed: 56 additions & 4 deletions

File tree

apps/web/__tests__/unit/recording-storage-lifecycle.test.ts

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,13 +134,14 @@ type DatabaseMutation = {
134134
values?: Record<string, unknown>;
135135
};
136136

137-
function databaseFixture(video = recording()) {
137+
function databaseFixture(video = recording(), password?: string | null) {
138138
const encoded = Schema.encodeSync(Video.Video)(video);
139139
const original = {
140140
...encoded,
141141
bucket: encoded.bucketId,
142142
createdAt: video.createdAt,
143143
updatedAt: video.updatedAt,
144+
password: password ?? null,
144145
};
145146
const rows = new Map<string, Record<string, unknown>>([[videoId, original]]);
146147
const jobs = new Map<string, Record<string, unknown>>([
@@ -879,6 +880,12 @@ describe("recording storage lifecycle", () => {
879880
expect(database.rows.get("duplicate-video")?.metadata).not.toHaveProperty(
880881
"desktopRecordingUpload",
881882
);
883+
expect(database.rows.get("duplicate-video")).not.toHaveProperty(
884+
"createdAt",
885+
);
886+
expect(database.rows.get("duplicate-video")).not.toHaveProperty(
887+
"updatedAt",
888+
);
882889
expect(database.rows.get("owned-video")?.metadata).toHaveProperty(
883890
"desktopRecordingUpload",
884891
);
@@ -899,6 +906,32 @@ describe("recording storage lifecycle", () => {
899906
]);
900907
});
901908

909+
it("preserves password and strips customCreatedAt without copying original createdAt or updatedAt", async () => {
910+
const videoWithCustomDate = Video.Video.make({
911+
...recording(),
912+
metadata: Option.some({
913+
sourceName: "Dated recording",
914+
customCreatedAt: "2020-01-01T00:00:00.000Z",
915+
}),
916+
});
917+
const database = databaseFixture(videoWithCustomDate, "secret-pass-123");
918+
await storageFixture([
919+
[outputKey, "verified-video"],
920+
[thumbnailKey, "verified-thumbnail"],
921+
[previewKey, "verified-preview"],
922+
]);
923+
const result = await runVideoOperation("duplicate");
924+
expect(Exit.isSuccess(result)).toBe(true);
925+
const duplicated = database.rows.get("duplicate-video");
926+
expect(duplicated).toBeDefined();
927+
expect(duplicated).not.toHaveProperty("createdAt");
928+
expect(duplicated).not.toHaveProperty("updatedAt");
929+
expect(duplicated?.metadata).not.toHaveProperty("customCreatedAt");
930+
expect(duplicated).toMatchObject({
931+
password: "secret-pass-123",
932+
});
933+
});
934+
902935
it("does not duplicate source inventories, raw fragments, or comment attachments across pages", async () => {
903936
const database = databaseFixture();
904937
const retained = Array.from(

packages/web-backend/src/Videos/VideosRepo.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,10 +109,16 @@ export class VideosRepo extends Effect.Service<VideosRepo>()("VideosRepo", {
109109

110110
yield* db.use((db) =>
111111
db.transaction(async (db) => {
112+
const {
113+
createdAt: _createdAt,
114+
updatedAt: _updatedAt,
115+
id: _id,
116+
...insertData
117+
} = data as Record<string, unknown>;
112118
const promises: MySqlInsertBase<any, any, any>[] = [
113119
db.insert(Db.videos).values([
114120
{
115-
...data,
121+
...insertData,
116122
id,
117123
orgId: data.orgId,
118124
bucket: Option.getOrNull(data.bucketId ?? Option.none()),

packages/web-backend/src/Videos/index.ts

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -349,7 +349,7 @@ export class Videos extends Effect.Service<Videos>()("Videos", {
349349
const maybeVideo = yield* policy.getOwnedById(videoId);
350350
if (Option.isNone(maybeVideo))
351351
return yield* Effect.fail(new Video.NotFoundError());
352-
const [video] = maybeVideo.value;
352+
const [video, password] = maybeVideo.value;
353353

354354
const [bucket] = yield* storage.getAccessForVideo(video);
355355

@@ -410,16 +410,29 @@ export class Videos extends Effect.Service<Videos>()("Videos", {
410410
}
411411
} while (continuationToken);
412412
publicationAttempted = true;
413+
const hasPassword = Option.isSome(password);
413414
yield* repo.create(
414415
{
415-
...video,
416+
ownerId: video.ownerId,
417+
orgId: video.orgId,
418+
name: video.name,
419+
public: video.public,
416420
source:
417421
publishedKeys.size > 0 ? { type: "desktopMP4" } : video.source,
418422
metadata: Option.map(video.metadata, (metadata) => {
419423
const copied = { ...metadata };
420424
delete copied.desktopRecordingUpload;
425+
delete copied.customCreatedAt;
421426
return copied;
422427
}),
428+
bucketId: video.bucketId,
429+
storageIntegrationId: video.storageIntegrationId,
430+
folderId: video.folderId,
431+
transcriptionStatus: video.transcriptionStatus,
432+
width: video.width,
433+
height: video.height,
434+
duration: video.duration,
435+
...(hasPassword ? { password: password.value } : {}),
423436
},
424437
{ id: newVideoId },
425438
);

0 commit comments

Comments
 (0)