Skip to content

app: write zipped archive 0600, flush before deleting - #4833

Open
OisinKyne wants to merge 4 commits into
mainfrom
app/bundle-output-perms
Open

OisinKyne wants to merge 4 commits into
mainfrom
app/bundle-output-perms

Conversation

@OisinKyne

Copy link
Copy Markdown
Contributor

charon create cluster --zipped and charon dkg --zipped created the archive with os.Create (usually world-readable 0644), although it holds validator key shares and the ENR private key. It is now 0600, including an archive left from an earlier run. BundleOutput also closed the tar and gzip writers only in defers, after deleting the originals and ignoring their errors, so a failed flush (e.g. a full disk) could lose key shares. The archive is now flushed, synced and closed before anything is deleted.

Worth testing with the dappnode package import to ensure there are no issues uncovered by this change (the main place --zipped is used)

category: bug
ticket: none

`charon create cluster --zipped` and `charon dkg --zipped` created the archive with os.Create (usually world-readable 0644), although it holds validator key shares and the ENR private key. It is now 0600, including an archive left from an earlier run. BundleOutput also closed the tar and gzip writers only in defers, after deleting the originals and ignoring their errors, so a failed flush (e.g. a full disk) could lose key shares. The archive is now flushed, synced and closed before anything is deleted.

category: bug
ticket: none
@codecov

codecov Bot commented Nov 10, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 75.86207% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 67.75%. Comparing base (0483b7f) to head (b746783).

Files with missing lines Patch % Lines
app/utils.go 75.86% 7 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4833      +/-   ##
==========================================
+ Coverage   67.73%   67.75%   +0.01%     
==========================================
  Files         255      255              
  Lines       33749    33689      -60     
==========================================
- Hits        22860    22825      -35     
+ Misses      10888    10863      -25     
  Partials        1        1              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

OisinKyne and others added 3 commits November 10, 2026 15:11
BundleOutput now writes to a 0600 temporary file in the target directory
and renames it into place once it is flushed, synced and closed, so the
archive name never holds a partial archive. A failed run removes the
temporary file and leaves the originals. An archive left by an earlier
run is replaced rather than archived into the new one.
@sonarqubecloud

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants