fix(api): give desktop downloads a lifetime that follows the key (#1828)

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<HonoEnv>): 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<HonoEnv>): 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<HonoEnv>): 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));
 		},
 	);
 }

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<string, VersionFile>;
 };
 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<string | null> {
@@ -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`,

tools/ci/src/common.rs

@@ -469,25 +469,6 @@ where
     Ok(())
 }
 
-pub(crate) async fn upload_directory_to_s3_overwrite<F>(
-    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,

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<F>(
 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<F>(
+    s3_prefix: &str,
+    payload_root: &Path,
+    include: F,
+) -> Result<Vec<S3UploadPlanItem>>
+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> {
         Utc.with_ymd_and_hms(year, month, day, hour, minute, second)
             .single()