From d339b82f8eebd3a414cb36830d3a6382d07b426f Mon Sep 17 00:00:00 2001 From: Hampus Date: Mon, 24 Aug 2026 12:26:33 +0200 Subject: [PATCH] fix(api): give desktop downloads a lifetime that follows the key (#1828) --- .../src/api/download/DownloadController.ts | 13 +- .../src/api/download/DownloadService.ts | 39 +++++- tools/ci/src/common.rs | 19 --- tools/ci/src/desktop.rs | 130 ++++++++++++++++-- 4 files changed, 166 insertions(+), 35 deletions(-) diff --git a/fluxer_api/src/api/download/DownloadController.ts b/fluxer_api/src/api/download/DownloadController.ts index 713e64512..7429e3a6b 100644 --- a/fluxer_api/src/api/download/DownloadController.ts +++ b/fluxer_api/src/api/download/DownloadController.ts @@ -18,7 +18,12 @@ import {OpenAPI} from '../middleware/ResponseTypeMiddleware'; import type {HonoEnv} from '../types/HonoEnv'; import {Validator} from '../Validator'; import type {DesktopChecksumFile, DownloadService, DownloadStreamResult} from './DownloadService'; -import {DESKTOP_REDIRECT_PREFIX, DOWNLOAD_PREFIX, UnsatisfiableRangeError} from './DownloadService'; +import { + DESKTOP_REDIRECT_PREFIX, + DOWNLOAD_PREFIX, + downloadCacheControlForKey, + UnsatisfiableRangeError, +} from './DownloadService'; function artifactFilename(key: string, filenameOverride?: string): string { return filenameOverride ?? key.split('/').pop() ?? 'download'; @@ -290,7 +295,7 @@ export function DownloadController(routes: Hono): void { if (!checksum) { return ctx.text('Not Found', 404); } - return checksumFileResponse(ctx, checksum, 'public, max-age=86400'); + return checksumFileResponse(ctx, checksum, downloadCacheControlForKey(checksum.key)); }, ); routes.on( @@ -316,7 +321,7 @@ export function DownloadController(routes: Hono): void { if (!key) { return ctx.text('Not Found', 404); } - return streamArtifactResponse(ctx, downloadService, key, 'public, max-age=86400'); + return streamArtifactResponse(ctx, downloadService, key, downloadCacheControlForKey(key)); }, ); routes.on( @@ -340,7 +345,7 @@ export function DownloadController(routes: Hono): void { if (!key) { return ctx.text('Not Found', 404); } - return streamArtifactResponse(ctx, downloadService, key, 'public, max-age=300'); + return streamArtifactResponse(ctx, downloadService, key, downloadCacheControlForKey(key)); }, ); } diff --git a/fluxer_api/src/api/download/DownloadService.ts b/fluxer_api/src/api/download/DownloadService.ts index e3efa0459..59ad385b6 100644 --- a/fluxer_api/src/api/download/DownloadService.ts +++ b/fluxer_api/src/api/download/DownloadService.ts @@ -51,6 +51,35 @@ function desktopBucketPrefix(test?: boolean): string { return test ? DESKTOP_TEST_BUCKET_PREFIX : DESKTOP_BUCKET_PREFIX; } +const MUTABLE_DOWNLOAD_CACHE_CONTROL = 'public, max-age=300'; +const VERSIONED_ARTIFACT_CACHE_CONTROL = 'public, max-age=31536000'; + +function isDesktopReleaseFeedFilename(filename: string): boolean { + return ( + filename === 'manifest.json' || + filename.endsWith('.yml') || + filename.endsWith('.yaml') || + filename.startsWith('RELEASES') || + (filename.startsWith('releases') && filename.endsWith('.json')) || + (filename.startsWith('assets') && filename.endsWith('.json')) + ); +} + +function isVersionedDesktopArtifactKey(key: string): boolean { + if (!key.startsWith(`${DESKTOP_BUCKET_PREFIX}/`)) { + return false; + } + const filename = key.split('/').pop() ?? ''; + if (filename.length === 0) { + return false; + } + return !isDesktopReleaseFeedFilename(filename); +} + +export function downloadCacheControlForKey(key: string): string { + return isVersionedDesktopArtifactKey(key) ? VERSIONED_ARTIFACT_CACHE_CONTROL : MUTABLE_DOWNLOAD_CACHE_CONTROL; +} + function desktopArtifactPrefix(params: { channel: DesktopChannel; plat: DesktopPlatform; @@ -108,6 +137,7 @@ type VersionInfo = { files: Record; }; export type DesktopChecksumFile = { + key: string; filename: string; sha256: string; body: string; @@ -461,7 +491,7 @@ export class DownloadService { return null; } const filename = this.filenameFromKey(key); - return this.buildDesktopChecksumFile(filename, file.sha256); + return this.buildDesktopChecksumFile(key, filename, file.sha256); } async resolveVersionedDesktopChecksumFile(params: { @@ -479,14 +509,14 @@ export class DownloadService { const filename = this.filenameFromKey(key); const objectSha256 = await this.readDesktopSha256ForArtifactKey(key); if (objectSha256) { - return this.buildDesktopChecksumFile(filename, objectSha256); + return this.buildDesktopChecksumFile(key, filename, objectSha256); } const latest = await this.getLatestDesktopVersion(params); const file = latest?.version === params.version ? latest.files[params.format] : undefined; if (!file?.sha256 || !this.isValidSha256(file.sha256)) { return null; } - return this.buildDesktopChecksumFile(filename, file.sha256); + return this.buildDesktopChecksumFile(key, filename, file.sha256); } async resolveDownloadKey(params: {path: string; test?: boolean}): Promise { @@ -1151,8 +1181,9 @@ export class DownloadService { return key.split('/').pop() ?? 'download'; } - private buildDesktopChecksumFile(filename: string, sha256: string): DesktopChecksumFile { + private buildDesktopChecksumFile(key: string, filename: string, sha256: string): DesktopChecksumFile { return { + key, filename, sha256, body: `${sha256} ${filename}\n`, diff --git a/tools/ci/src/common.rs b/tools/ci/src/common.rs index 7fcadc313..c46bf45d1 100644 --- a/tools/ci/src/common.rs +++ b/tools/ci/src/common.rs @@ -469,25 +469,6 @@ where Ok(()) } -pub(crate) async fn upload_directory_to_s3_overwrite( - client: &S3Client, - bucket: &str, - prefix: &str, - root: &Path, - include: F, -) -> Result<()> -where - F: Fn(&Path) -> bool, -{ - let plan = directory_upload_plan(prefix, root, include)?; - let stats = upload_s3_plan_overwrite(client, bucket, plan).await?; - println!( - "Overwrite upload complete for s3://{bucket}/{prefix}: uploaded {}", - stats.uploaded - ); - Ok(()) -} - pub(crate) async fn upload_s3_plan_append_only( client: &S3Client, bucket: &str, diff --git a/tools/ci/src/desktop.rs b/tools/ci/src/desktop.rs index 5248ff25f..ccfc890fb 100644 --- a/tools/ci/src/desktop.rs +++ b/tools/ci/src/desktop.rs @@ -1,12 +1,13 @@ // SPDX-License-Identifier: AGPL-3.0-or-later use crate::common::{ - CalverEnv, CommandSpec, append_github_env, append_github_output, append_github_path, capture, - collect_files, command_succeeds, copy_dir_contents, count_files, count_files_min_depth, - download_file, download_s3_prefix, env_bool, env_string, join_s3_key, output_bytes, - output_text, parse_bool, path_to_s3_key, remove_dir_if_exists, remove_file_if_exists, - require_any_env, require_env, require_home, resolve_calver, run_command, runner_temp, - s3_client, title_case, trim_option, upload_directory_to_s3, upload_directory_to_s3_overwrite, + CalverEnv, CommandSpec, S3UploadPlanItem, append_github_env, append_github_output, + append_github_path, capture, collect_files, command_succeeds, copy_dir_contents, count_files, + count_files_min_depth, directory_upload_plan, download_file, download_s3_prefix, env_bool, + env_string, join_s3_key, output_bytes, output_text, parse_bool, path_to_s3_key, + remove_dir_if_exists, remove_file_if_exists, require_any_env, require_env, require_home, + resolve_calver, run_command, runner_temp, s3_client, title_case, trim_option, + upload_directory_to_s3, upload_s3_plan_append_only, upload_s3_plan_overwrite, }; use crate::functions::write_json_pretty; use anyhow::{Context, Result, anyhow, bail, ensure}; @@ -3090,11 +3091,62 @@ async fn upload_payload_directory( where F: Fn(&Path) -> bool, { + let plan = desktop_payload_upload_plan(s3_prefix, payload_root, include)?; if overwrite_existing { - upload_directory_to_s3_overwrite(client, bucket, s3_prefix, payload_root, include).await + let stats = upload_s3_plan_overwrite(client, bucket, plan).await?; + println!( + "Overwrite upload complete for s3://{bucket}/{s3_prefix}: uploaded {}", + stats.uploaded + ); } else { - upload_directory_to_s3(client, bucket, s3_prefix, payload_root, include).await + let stats = upload_s3_plan_append_only(client, bucket, plan).await?; + println!( + "Append-only upload complete for s3://{bucket}/{s3_prefix}: uploaded {}, skipped existing {}", + stats.uploaded, stats.skipped_existing + ); } + Ok(()) +} + +fn desktop_payload_upload_plan( + s3_prefix: &str, + payload_root: &Path, + include: F, +) -> Result> +where + F: Fn(&Path) -> bool, +{ + Ok(directory_upload_plan(s3_prefix, payload_root, include)? + .into_iter() + .map(|item| { + let cache_control = desktop_object_cache_control(&item.key); + item.with_cache_control(cache_control) + }) + .collect()) +} + +pub(crate) const MUTABLE_DOWNLOAD_CACHE_CONTROL: &str = "public, max-age=300"; +pub(crate) const VERSIONED_ARTIFACT_CACHE_CONTROL: &str = "public, max-age=31536000"; + +fn desktop_object_cache_control(key: &str) -> &'static str { + if is_versioned_desktop_artifact_key(key) { + VERSIONED_ARTIFACT_CACHE_CONTROL + } else { + MUTABLE_DOWNLOAD_CACHE_CONTROL + } +} + +fn is_versioned_desktop_artifact_key(key: &str) -> bool { + if !key.starts_with("desktop/") { + return false; + } + let Some(filename) = key.rsplit('/').next() else { + return false; + }; + if filename.is_empty() { + return false; + } + !is_payload_metadata_key(Path::new(filename)) && !filename.ends_with(".yaml") } fn should_overwrite_payload(s3_prefix: &str, test_build: bool) -> bool { @@ -3321,6 +3373,68 @@ mod tests { use crate::common::{directory_upload_plan, parse_version_instant, s3_directory_prefix}; use chrono::{DateTime, TimeZone, Utc}; + #[test] + fn every_uploaded_desktop_object_carries_a_cache_instruction() { + let temp = tempfile::tempdir().unwrap(); + let root = temp.path().join("desktop").join("stable").join("darwin"); + fs::create_dir_all(root.join("arm64")).unwrap(); + fs::write(root.join("arm64").join("Fluxer-1.2.3-arm64.dmg"), "dmg").unwrap(); + fs::write(root.join("arm64").join("manifest.json"), "{}").unwrap(); + fs::write(root.join("arm64").join("latest-mac.yml"), "version: 1").unwrap(); + + let plan = + desktop_payload_upload_plan("desktop", temp.path().join("desktop").as_path(), |_| true) + .unwrap(); + + assert!( + !plan.is_empty(), + "the sample payload produced no upload plan" + ); + for item in &plan { + assert!( + item.cache_control.is_some(), + "{} would be stored with no cache instruction at all", + item.key + ); + } + } + + #[test] + fn the_stored_lifetime_follows_the_key_not_the_upload_batch() { + assert_eq!( + desktop_object_cache_control("desktop/stable/darwin/arm64/Fluxer-1.2.3-arm64.dmg"), + VERSIONED_ARTIFACT_CACHE_CONTROL + ); + assert_eq!( + desktop_object_cache_control( + "desktop/stable/darwin/arm64/Fluxer-1.2.3-arm64.dmg.sha256" + ), + VERSIONED_ARTIFACT_CACHE_CONTROL + ); + assert_eq!( + desktop_object_cache_control("desktop/stable/darwin/arm64/manifest.json"), + MUTABLE_DOWNLOAD_CACHE_CONTROL, + "the release pointer must stay reachable when it moves" + ); + assert_eq!( + desktop_object_cache_control("desktop/stable/win32/x64/latest.yml"), + MUTABLE_DOWNLOAD_CACHE_CONTROL + ); + assert_eq!( + desktop_object_cache_control("desktop/stable/win32/x64/RELEASES.json"), + MUTABLE_DOWNLOAD_CACHE_CONTROL + ); + assert_eq!( + desktop_object_cache_control("desktop-test/canary/linux/x64/Fluxer-1.2.3.AppImage"), + MUTABLE_DOWNLOAD_CACHE_CONTROL, + "test artifacts are overwritten in place, so they are not immutable" + ); + assert_ne!( + VERSIONED_ARTIFACT_CACHE_CONTROL, MUTABLE_DOWNLOAD_CACHE_CONTROL, + "the two policies collapsed into one, so this test proves nothing" + ); + } + fn dt(year: i32, month: u32, day: u32, hour: u32, minute: u32, second: u32) -> DateTime { Utc.with_ymd_and_hms(year, month, day, hour, minute, second) .single()