refactor(web-ng): dedup FirstPartyImporter onto ForgejoOciClient + native add-on import e2e (#3425) #3480

Merged
mfreeman451 merged 1 commit from feat/fpi-shared-transport-dedup into staging 2026-06-01 01:06:15 +00:00
Owner

Summary

Completes add-native-addon-build-signing §4.1's two follow-ups (from #3479) and hardens the native add-on import path.

1. Dedup FirstPartyImporter onto the shared ForgejoOciClient

The shared transport client landed in #3479 and is in use by the native importer, but FirstPartyImporter still held its own copies. It now does:

import ServiceRadarWebNG.Plugins.ForgejoOciClient, except: [default_repo_url: 0]

and drops its ~45 duplicated HTTP/OCI/Cosign/URL transport defps — 1006 → 470 lines. The kept code is only the Wasm-bundle-specific discovery/verify/result-shaping; bare transport calls resolve to the imported shared functions, so no call-site rewrites were needed. default_repo_url/0 stays a public accessor (delegating to the client) for the plugin_package_live caller.

Behavior is preserved — including the importer's historical 20-release scan ceiling (the shared client allows up to 50 for native add-ons, so FPI clamps before calling). Gated by the unchanged first_party_importer_test.exs, which drives every moved transport path (direct + OCI fetch, bearer challenge, redirects, digest/cosign/upload-sig verify) through FPI's public API.

2. End-to-end test of the web-ng native importer

native_addon_importer_test.exs exercises the full orchestration end to end: a fake ForgejoOciClient HTTP backend serves the release, the serviceradar-native-addon-index.json asset, the OCI manifest, and the bundle + per-arch blobs by digest; Cosign and the datasvc upload are stubbed (the upload via a new, test-only :native_addon_artifact_upload seam + build_mirror/2). The real core then verifies each tarball's sha256 + agent-release ed25519 and persists a staged AddonPackage. Covers the happy path + fail-closed paths.

3. Hardening (high-severity, found in review)

fetch_layer_blob previously only asserted a digest was a declared layer of the Cosign-verified manifest — it never re-hashed the returned bytes. The per-arch tarball is covered by the core's sha256 + ed25519, but the bundle (sole source of addon.yaml/config.schema.json) was otherwise unsigned, so a tampering/buggy registry could swap it. It now re-hashes every fetched blob against its digest (Client.digest_matches?), mirroring what the Wasm FirstPartyImporter already does. A regression test serves tampered bundle bytes under the still-declared digest and asserts {:error, {:blob_digest_mismatch, _}}.

Test coverage (native e2e)

  • imports a verified native add-on into a staged AddonPackage
  • rejects a tarball signed with a key other than the release key → :invalid_signature
  • rejects an index entry whose tarball digest is absent from the Cosign-verified manifest → :digest_not_in_manifest
  • rejects a blob whose bytes don't match its manifest-declared digest → :blob_digest_mismatch

Validation

  • mix compile --warnings-as-errors + mix credo --strict clean
  • 14/14 tests green (10 FPI regression + 4 native e2e) against an srql-fixtures scratch CNPG DB
  • An adversarial multi-agent review of the diff surfaced 9 candidates; 2 were confirmed (the 20→50 cap drift and the bundle-digest gap) and fixed here, the other 7 refuted.

🤖 Generated with Claude Code

## Summary Completes `add-native-addon-build-signing` §4.1's two follow-ups (from #3479) and hardens the native add-on import path. ### 1. Dedup `FirstPartyImporter` onto the shared `ForgejoOciClient` The shared transport client landed in #3479 and is in use by the native importer, but `FirstPartyImporter` still held its own copies. It now does: ```elixir import ServiceRadarWebNG.Plugins.ForgejoOciClient, except: [default_repo_url: 0] ``` and drops its ~45 duplicated HTTP/OCI/Cosign/URL transport `defp`s — **1006 → 470 lines**. The kept code is only the Wasm-bundle-specific discovery/verify/result-shaping; bare transport calls resolve to the imported shared functions, so no call-site rewrites were needed. `default_repo_url/0` stays a public accessor (delegating to the client) for the `plugin_package_live` caller. Behavior is preserved — including the importer's historical **20-release scan ceiling** (the shared client allows up to 50 for native add-ons, so FPI clamps before calling). Gated by the unchanged `first_party_importer_test.exs`, which drives every moved transport path (direct + OCI fetch, bearer challenge, redirects, digest/cosign/upload-sig verify) through FPI's public API. ### 2. End-to-end test of the web-ng native importer `native_addon_importer_test.exs` exercises the full orchestration end to end: a fake `ForgejoOciClient` HTTP backend serves the release, the `serviceradar-native-addon-index.json` asset, the OCI manifest, and the bundle + per-arch blobs by digest; Cosign and the datasvc upload are stubbed (the upload via a new, test-only `:native_addon_artifact_upload` seam + `build_mirror/2`). The real core then verifies each tarball's sha256 + agent-release ed25519 and persists a **staged `AddonPackage`**. Covers the happy path + fail-closed paths. ### 3. Hardening (high-severity, found in review) `fetch_layer_blob` previously only asserted a digest was a *declared* layer of the Cosign-verified manifest — it never re-hashed the *returned bytes*. The per-arch tarball is covered by the core's sha256 + ed25519, but the **bundle** (sole source of `addon.yaml`/`config.schema.json`) was otherwise unsigned, so a tampering/buggy registry could swap it. It now re-hashes every fetched blob against its digest (`Client.digest_matches?`), mirroring what the Wasm `FirstPartyImporter` already does. A regression test serves tampered bundle bytes under the still-declared digest and asserts `{:error, {:blob_digest_mismatch, _}}`. ## Test coverage (native e2e) - imports a verified native add-on into a staged `AddonPackage` - rejects a tarball signed with a key other than the release key → `:invalid_signature` - rejects an index entry whose tarball digest is absent from the Cosign-verified manifest → `:digest_not_in_manifest` - rejects a blob whose bytes don't match its manifest-declared digest → `:blob_digest_mismatch` ## Validation - `mix compile --warnings-as-errors` + `mix credo --strict` clean - **14/14 tests green** (10 FPI regression + 4 native e2e) against an `srql-fixtures` scratch CNPG DB - An adversarial multi-agent review of the diff surfaced 9 candidates; 2 were confirmed (the 20→50 cap drift and the bundle-digest gap) and fixed here, the other 7 refuted. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
refactor(web-ng): dedup FirstPartyImporter onto ForgejoOciClient + native add-on import e2e (#3425)
Some checks failed
Secret Scan / gitleaks (pull_request) Successful in 28s
lint / lint (push) Successful in 1m38s
Golang Tests / test-go (push) Successful in 2m2s
lint / lint (pull_request) Successful in 1m50s
CI / build (pull_request) Failing after 11m11s
Elixir Quality / Elixir Quality (pull_request) Failing after 27m30s
91988fe387
Completes add-native-addon-build-signing §4.1's two follow-ups and hardens the
native import path.

- Dedup: FirstPartyImporter now `import ForgejoOciClient, except: [default_repo_url: 0]`
  and drops its ~45 duplicated HTTP/OCI/Cosign/URL transport defps (1006 -> 470
  lines). The kept code is only the Wasm-bundle-specific discovery/verify/result
  shaping; bare transport calls resolve to the shared client. `default_repo_url/0`
  stays public (delegating to the client) for the LiveView caller. Behavior is
  preserved, including the importer's historical 20-release scan ceiling
  (clamped before the shared client's 50 cap). Gated by the unchanged
  first_party_importer_test.exs, which drives every moved transport path through
  FPI's public API.

- E2e test: native_addon_importer_test.exs exercises the full web-ng orchestration
  end to end — a fake ForgejoOciClient HTTP backend serves the release, the
  native-addon index, the OCI manifest, and the bundle + per-arch blobs by digest;
  Cosign and the datasvc upload are stubbed (the upload via a new, test-only
  :native_addon_artifact_upload seam). The real core then verifies each tarball's
  sha256 + agent-release ed25519 and persists a staged AddonPackage. Covers the
  happy path plus fail-closed paths: wrong release key, a digest absent from the
  Cosign-verified manifest, and a blob whose bytes don't match its declared digest.

- Hardening: fetch_layer_blob now re-hashes every fetched blob against its declared
  digest (Client.digest_matches?), not just membership in the Cosign-verified
  manifest. The bundle (sole source of addon.yaml/config.schema.json) was otherwise
  unsigned, so a tampering/buggy registry could have swapped it; this is its only
  byte-level integrity gate. Mirrors what the Wasm FirstPartyImporter already does.

Validation: compile --warnings-as-errors + credo --strict clean; 14/14 tests green
(10 FPI regression + 4 native e2e) against an srql-fixtures scratch CNPG DB.
Findings surfaced by an adversarial multi-agent review of the diff.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
mfreeman451 left a comment

lgtm

lgtm
mfreeman451 deleted branch feat/fpi-shared-transport-dedup 2026-06-01 01:06:15 +00:00
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
carverauto/serviceradar!3480
No description provided.