commit aee8d70c68032e6d2a769479f9f26a4f37add9a9
parent 10b8a9dad2ce5c7c85c994cd0c4f4935332e9c6e
Author: Ryan Sepassi <rsepassi@gmail.com>
Date: Mon, 1 Jun 2026 15:02:35 -0700
dist: fix review findings in the package layer
- pkg_build_native_regions: free the index/content MemWriters on every
error path (goto fail) instead of leaking them; this code is now a public
library API (cfree_pkg_create) where error-path leaks matter.
- tofu: a trust-on-first-use acceptance whose pin cannot be persisted now
fails verification (exit 1) as the original tool did, instead of silently
succeeding (exit 0); pkg_pin_tofu returns status and the driver propagates it.
- add _Static_asserts binding CfreePkgShape to the internal PkgNativeShape
(the create path casts between them), matching the existing compression asserts.
Note: the per-file ' extracted <path>' progress line dropped (CLI progress
that does not belong in libcfree); the 'unpacked N V to DIR' summary remains.
Found by the adversarial review of the migration.
Diffstat:
2 files changed, 44 insertions(+), 22 deletions(-)
diff --git a/driver/cmd/pkg.c b/driver/cmd/pkg.c
@@ -293,17 +293,21 @@ done:
/* verify / unpack */
/* ---------------------------------------------------------------------- */
-static void pkg_pin_tofu(DriverEnv* env, const CfreeContext* ctx,
- const char* tpath, const CfreePkgVerifyResult* r) {
+/* Persist a trust-on-first-use pin. Returns 0 on success; nonzero if it could
+ * not be written (no trust path, or I/O failure) — the original tool failed
+ * verification when the pin could not be persisted, so the caller treats a
+ * nonzero return as a verify failure. */
+static int pkg_pin_tofu(DriverEnv* env, const CfreeContext* ctx,
+ const char* tpath, const CfreePkgVerifyResult* r) {
char line[PKG_TRUST_LINE_MAX], parent[PKG_PATH_BUF], hex[PKG_KEYID_HEX];
CfreeFileData old;
CfreeWriter* w = NULL;
int had_old, ok = 1, wrote = 0;
- if (!tpath[0]) return;
+ if (!tpath[0]) return 1;
cfree_hex_encode(hex, r->keyid, CFREE_PKG_KEYID_LEN);
if (cfree_trust_format_entry(line, sizeof line, r->keyid, r->tofu_pk,
"tofu-pinned") != CFREE_OK)
- return;
+ return 1;
had_old = pkg_read(ctx, tpath, &old);
pkg_parent_dir(tpath, parent, sizeof parent);
if (parent[0]) driver_mkdir_p(env, parent);
@@ -316,6 +320,7 @@ static void pkg_pin_tofu(DriverEnv* env, const CfreeContext* ctx,
}
if (had_old) pkg_release(ctx, &old);
if (wrote) driver_printf("pkg: tofu-pinned key id %s to %s\n", hex, tpath);
+ return wrote ? 0 : 1;
}
static int pkg_verify_or_unpack(DriverEnv* env, const CfreeContext* ctx,
@@ -391,18 +396,25 @@ static int pkg_verify_or_unpack(DriverEnv* env, const CfreeContext* ctx,
host = driver_cas_host(env);
if (cfree_pkg_verify(ctx, &host, &opts, &result) == CFREE_OK) {
- int quiet = unpack && !explicit_verify;
- if (result.tofu_pin) pkg_pin_tofu(env, ctx, tpath, &result);
- if (!quiet) {
- char idhex[PKG_KEYID_HEX];
- cfree_hex_encode(idhex, result.keyid, CFREE_PKG_KEYID_LEN);
- driver_printf("ok: %s %s signer %s [%s]\n", result.name, result.version,
- idhex, result.trusted);
+ /* A trust-on-first-use acceptance must persist its pin, like the original
+ * tool — a pin that can't be written fails verification (exit 1). */
+ if (result.tofu_pin && pkg_pin_tofu(env, ctx, tpath, &result) != 0) {
+ driver_errf(PKG_TOOL,
+ "tofu: could not persist key id to trusted-keys "
+ "(set CFREE_TRUSTED_KEYS or HOME)");
+ } else {
+ int quiet = unpack && !explicit_verify;
+ if (!quiet) {
+ char idhex[PKG_KEYID_HEX];
+ cfree_hex_encode(idhex, result.keyid, CFREE_PKG_KEYID_LEN);
+ driver_printf("ok: %s %s signer %s [%s]\n", result.name,
+ result.version, idhex, result.trusted);
+ }
+ if (opts.unpack_dir)
+ driver_printf("unpacked %s %s to %s\n", result.name, result.version,
+ opts.unpack_dir);
+ rc = 0;
}
- if (opts.unpack_dir)
- driver_printf("unpacked %s %s to %s\n", result.name, result.version,
- opts.unpack_dir);
- rc = 0;
}
if (trust_loaded) pkg_release(ctx, &trustfd);
diff --git a/src/api/package.c b/src/api/package.c
@@ -55,6 +55,12 @@ typedef enum PkgNativeShape {
PKG_NATIVE_THIN
} PkgNativeShape;
+/* cfree_pkg_create casts CfreePkgShape straight to PkgNativeShape. */
+_Static_assert((int)CFREE_PKG_SHAPE_FAT == PKG_NATIVE_FAT, "shape fat");
+_Static_assert((int)CFREE_PKG_SHAPE_METADATA == PKG_NATIVE_METADATA,
+ "shape metadata");
+_Static_assert((int)CFREE_PKG_SHAPE_THIN == PKG_NATIVE_THIN, "shape thin");
+
typedef struct PkgBlob {
CfreeFileData fd;
int loaded;
@@ -584,7 +590,7 @@ static int pkg_build_native_regions(const CfreeCasHost* host,
CfreeWriter* index = pkg_mem(ctx);
CfreeWriter* content = pkg_mem(ctx);
size_t bi;
- if (!index || !content) return DIST_ERR;
+ if (!index || !content) goto fail;
for (bi = 0; bi < src->n_blobs; ++bi) {
const PkgBlob* blob = &src->blobs[bi];
size_t off = 0, ci = 0;
@@ -609,14 +615,14 @@ static int pkg_build_native_regions(const CfreeCasHost* host,
pkg_hash(r.stored_hash, raw, raw_len);
if (embed_content &&
cfree_writer_write(content, raw, raw_len) != CFREE_OK)
- return DIST_ERR;
+ goto fail;
if (!embed_content) {
char rel[PKG_PATH_BUF];
if (pkg_external_chunk_path(rel, sizeof rel, blob->id,
r.chunk_index) != DIST_OK ||
pkg_write_external_file(host, ctx, external_dir, rel, raw,
raw_len) != DIST_OK)
- return DIST_ERR;
+ goto fail;
}
} else {
uint8_t tmp[DIST_CFPKG3_CHUNK_SIZE_DEFAULT + 1024u];
@@ -624,25 +630,25 @@ static int pkg_build_native_regions(const CfreeCasHost* host,
if (dist_lz4_compress_block(tmp, sizeof tmp, &stored_len, raw,
raw_len) != DIST_OK) {
pkg_diagf(ctx, "create: lz4-block-v1 compression failed");
- return DIST_ERR;
+ goto fail;
}
r.stored_size = stored_len;
pkg_hash(r.stored_hash, tmp, stored_len);
if (embed_content &&
cfree_writer_write(content, tmp, stored_len) != CFREE_OK)
- return DIST_ERR;
+ goto fail;
if (!embed_content) {
char rel[PKG_PATH_BUF];
if (pkg_external_chunk_path(rel, sizeof rel, blob->id,
r.chunk_index) != DIST_OK ||
pkg_write_external_file(host, ctx, external_dir, rel, tmp,
stored_len) != DIST_OK)
- return DIST_ERR;
+ goto fail;
}
}
dist_cfpkg3_encode_index_record(recbuf, &r);
if (cfree_writer_write(index, recbuf, sizeof recbuf) != CFREE_OK)
- return DIST_ERR;
+ goto fail;
off += raw_len;
++ci;
}
@@ -650,6 +656,10 @@ static int pkg_build_native_regions(const CfreeCasHost* host,
*index_out = index;
*content_out = content;
return DIST_OK;
+fail:
+ if (content) cfree_writer_close(content);
+ if (index) cfree_writer_close(index);
+ return DIST_ERR;
}
static int pkg_create_cfpkg(const CfreeCasHost* host, const CfreeContext* ctx,
