Skip to content

Restore TUF fallback for mapped network errors - #26

Merged
mxcl merged 2 commits into
masterfrom
codex/fix-tuf-network-fallback
Sep 3, 2026
Merged

Restore TUF fallback for mapped network errors#26
mxcl merged 2 commits into
masterfrom
codex/fix-tuf-network-fallback

Conversation

@mxcl

@mxcl mxcl commented Sep 3, 2026

Copy link
Copy Markdown
Owner

Summary

NetworkTransfer maps ordinary URLError values into AppUpdaterNetworkError, but TUFClient.isNetworkFailure still recognized only raw URL errors and two older AppUpdaterError cases. A transient transport failure during Sigstore TUF refresh therefore skipped the verified cache and embedded trusted-root fallback and surfaced as an attestation verification failure.

  • recognize AppUpdaterNetworkError as the transport failure it represents
  • cover the mapped offline error with a focused regression test

This restores the fallback behavior from before network errors gained safe host-aware descriptions. Cryptographically invalid, expired, rolled-back, malformed, and oversized metadata still fail closed.

Reported through automic-vault#93.

Tests

  • swift test --filter TUFTests/testMappedNetworkFailureUsesEmbeddedFallback (red before, green after)
  • all 76 package tests, run in split suites to stay within the local command window

@coveralls

coveralls commented Sep 3, 2026

Copy link
Copy Markdown

Coverage Report for CI Build 33704109791

Coverage increased (+0.01%) to 70.502%

Details

  • Coverage increased (+0.01%) from the base build.
  • Patch coverage: 1 of 1 lines across 1 file are fully covered (100%).
  • No coverage regressions found.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

No coverage regressions found.


Coverage Stats

Coverage Status
Relevant Lines: 2607
Covered Lines: 1838
Line Coverage: 70.5%
Coverage Strength: 441.82 hits per line

💛 - Coveralls

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

isNetworkFailure currently treats all AppUpdaterNetworkError values as network failures, which can incorrectly classify cancellations unless .cancelled is explicitly excluded.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR restores Sigstore TUF refresh fallback behavior by treating mapped URLError values (AppUpdaterNetworkError) as transport/network failures, ensuring the client can fall back to verified cached metadata or embedded trusted-root when the network is transiently unavailable.

Changes:

  • Update TUFClient.isNetworkFailure to recognize AppUpdaterNetworkError as a network failure.
  • Add a regression test covering the mapped offline error path to ensure embedded fallback is used.
File summaries
File Description
Attestation/TUF.swift Extend network-failure detection to include AppUpdaterNetworkError so TUF refresh can fall back appropriately.
Tests/AppUpdaterTests/TUFTests.swift Add regression test verifying mapped offline errors trigger the embedded fallback trusted-root path.
Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread Attestation/TUF.swift

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The change precisely aligns the network-failure classifier with the mapped error type and is backed by focused regression tests covering both the fallback and cancellation behavior.

Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

@mxcl
mxcl merged commit 4826e72 into master Sep 3, 2026
2 checks passed
@mxcl
mxcl deleted the codex/fix-tuf-network-fallback branch September 3, 2026 13:51
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.

3 participants