chainguard-dev
fix(index): close file before sign
We are delegate the sync index data with the fs to linux, we have more guarantees with this change
The outFile file should closed before opening it again for signing, this probably causes condition races sometimes
I discovered it testing mountpoint-s3 and get
mountpoint_s3::fuse: open failed: inode error: inode 9 (full key "os/x86_64/APKINDEX.tar.gz") is not readable while being written
We saw some prs to fix BAD signature, maybe this can fix?
cc @mesaglio
I discovered it testing mountpoint-s3 and get
it's generally not advisable to write to storage locations like these. melange does a lot of in-place changes, and any network i/o failures with networked filesystems will result in non-atomic updates of .apk and/or APKINDEX.tar.gz. Meaning it might be old, partially written file, or new one unsigned, or fully updated and signed.
In our actions, we typically use bucket as input, but use a temporary output. Once the command is successful we upload the result out of the action (sign elsewhere) and then push to the bucket.
Similar strategy is advisable for any other buckets / publication implmeentations. Complete the build, then upload new .apk, then upload new apkindex.tar.gz last. Such that any new files references by the index are available ahead of the index update.
that's generally not advisable to write to storage locations like these. melange does a lot of in-place changes, and any network i/o failures with networked filesystems will result in non-atomic updates of .apk and/or APKINDEX.tar.gz.
ok, but that doesn't mean we should sign files what are currently open and have not been flushed. The fix is still valid, no?
that's generally not advisable to write to storage locations like these. melange does a lot of in-place changes, and any network i/o failures with networked filesystems will result in non-atomic updates of .apk and/or APKINDEX.tar.gz.
ok, but that doesn't mean we should sign files what are currently open and have not been flushed. The fix is still valid, no?
sure, the fix is valid. but likely there is more to do still; and it still is not safe. If we want to make this safe, it would be better to store the apk in memory; generate update in memory; sign it in memory; write out to a tmp file; atomically rename to target file; whilst holding a lock on it.