feat(feeds): private org: feeds, and materialize once per invocation

[mobileoverlord]

Owner: the secure-feeds series (241 → 242 → 243 → 244). Both sessions push as mobileoverlord, so GitHub cannot tell you which one wrote a PR — this line can.

Makes org: feeds work, and stops re-resolving feeds for every container.

Private organization feeds

repos:
  acme:
    org: acme          # release/channel optional; branch defaults to main
distro:
  feeds: [acme]

The CLI exchanges its Connect credential for a short-lived feed token, injects it into the generated repository file, and dnf reads a feed that refuses anonymous access. Previously org: parsed and was refused.

The token never becomes a build input. Resolution records only what is stable — the organization as the credential identity, and a connect://<org>/<path> placeholder as the URL — and the recorded document is written before anything is minted. A per-build token in the stamp hash would invalidate every cached sysroot once per build, and a server-side host change should not either, because the organization is the input and the host is a detail of how it was served.

The private tree mirrors the public one, so an organization feed is a distro-shaped feed whose release version is <release>/orgs/<org>/<branch>, and no new path construction was needed. The minted URL also goes through the loopback rewrite, which URL feeds get at resolution time and this had to get at mint time, or a locally hosted Connect resolves to nothing inside the container.

Errors name the remedy: not logged in points at avocado login, 403 says the account is not entitled, 404 says the deployment does not serve feed tokens yet.

Feeds are materialized once per invocation

Previously every container run re-resolved, re-minted and wrote a fresh temporary directory. Two consequences:

• A single avocado build minted five feed tokens. It is one per invocation now, and the end-to-end rig asserts it, so a regression fails a test rather than surfacing later as a rate limit refusing an ordinary build.
• The mount list is part of a container's shape, so a per-run directory gave every step a unique shape and prevented sharing a container between steps.

Each stage writes into its own subdirectory and AVOCADO_FEEDS_DIR selects it, so the mount is identical across steps while dnf still sees only its stage's feeds. Credentials now live as long as the invocation rather than one step; nothing reaches the SDK volume or the project directory, and the directory is removed when the process ends. The condition that would invalidate that trade, untrusted code running in the shared container, is named at the function.

Test infrastructure included

scripts/local-feeds/edge.py implements the observable contract of three pieces that do not exist yet — the token mint, the ES256 verifier, and the rate limiter — so this could be built and tested before any of them. contract-tests.sh asserts that contract against a base URL, naming no implementation, so the same file can later run against a real deployment. org-feed-rig.sh proves the whole path: mint, inject, authenticate, install, and no token in the recorded document.

Two behaviours worth calling out, both found by review against the server design. A missing credential is answered with 401 and a Basic challenge, not 403, because dnf authenticates only when challenged and only to Basic; 403 fails every fetch while working perfectly under curl -u. And the organization is a path segment, not a prefix, because the release precedes it — a prefix check would silently degrade into any authenticated organization being able to read any other.

Status

The server side of this is several phases out, so org: currently fails closed with a clear message rather than working. Merging it early is what lets the client be finished and tested; landing it later is also fine. Say which you prefer.

Stacked on #242.

[Copilot]

🟡 Changes recommended

There are confirmed correctness/security issues around Connect placeholder handling and org input validation, plus an async-mutex-hold-across-I/O pattern that can cause unnecessary global blocking.

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

Pull request overview

This PR extends the feed system to support private Connect-hosted org: feeds (via short-lived minted feed tokens) and changes feed materialization to occur once per CLI invocation (per target) rather than once per container step, improving token usage and container reuse.

Changes:

• Add org:/Connect feed support with token minting at materialization time, while keeping the canonical feed document stable and secret-free.
• Cache a single minted feed set + shared feeds tempdir per invocation/target and materialize stage-specific .repo sets into subdirectories.
• Add local “edge contract” test infrastructure and an end-to-end org-feed rig script.

File summaries

File	Description
src/utils/feeds.rs	Introduces Connect feed kind, org placeholder URLs, token minting, and per-stage materialization subdirs.
src/utils/config.rs	Adds per-invocation/per-target caching of minted feed sets and makes materialize_feeds async.
src/commands/sdk/install.rs	Updates calls to async materialize_feeds for SDK and kernel/rootfs stages.
src/commands/sdk/dnf.rs	Updates dnf passthrough to await async materialize_feeds.
src/commands/runtime/install.rs	Updates runtime install to await async materialize_feeds.
src/commands/runtime/dnf.rs	Updates runtime dnf passthrough to await async materialize_feeds.
src/commands/rootfs/install.rs	Updates rootfs install to await async materialize_feeds.
src/commands/initramfs/install.rs	Updates initramfs install to await async materialize_feeds.
src/commands/fetch.rs	Updates extension fetch command to await async materialize_feeds.
src/commands/ext/install.rs	Updates extension install to await async materialize_feeds.
src/commands/ext/fetch.rs	Updates extension fetch flow to await async materialize_feeds.
src/commands/ext/dnf.rs	Updates extension dnf passthrough to await async materialize_feeds.
scripts/local-feeds/org-feed-rig.sh	Adds end-to-end rig proving mint → inject → authenticated dnf install and “one mint per invocation”.
scripts/local-feeds/edge.py	Adds local implementation of the edge/mint/limiter contract for testing.
scripts/local-feeds/contract-tests.sh	Adds executable assertions for the edge contract (auth, mint, rate limiting).
.gitignore	Ignores Python bytecode artifacts from local-feeds scripts.

Review details

Suppressed comments (1)

src/utils/config.rs:4080

• resolve_connect_credentials() returns the highest minted tier so the client can mirror it into the User-Agent for edge rate-limiting, but the return value is currently ignored here. As a result, AVOCADO_FEED_UA continues to report tier/1 even if Connect issues a higher tier.

    let mut set = set;
    set.resolve_connect_credentials().await?;
    let root = std::sync::Arc::new(
        tempfile::Builder::new()

• Files reviewed: 15/16 changed files
• Comments generated: 4
• Review effort level: Lite

---

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

[Copilot]

🟡 Changes recommended

The review identified a concrete HTTP correctness bug in the local edge adaptor (HEAD handling) plus an unimplemented tier propagation path that the PR description/docstrings claim should exist.

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

Review details

• Files reviewed: 15/16 changed files
• Comments generated: 3
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

There are unresolved operational/documentation issues (notably identity/key-id consistency for rate limiting/log correlation and a misleading concurrency comment) that should be addressed before merging.

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

Review details

Suppressed comments (1)

src/utils/config.rs:4069

• The doc comment says the mutex lock is held across the mint, but the map lock is released before initialization and the single-mint guarantee comes from the per-target OnceCell. This is internally contradictory with the comment a few lines below and may mislead future changes to the concurrency behavior.

/// Get this invocation's feed set for `target`, minting on first use.
///
/// The lock is held across the mint deliberately: `sdk install` runs the rootfs
/// and initramfs installs concurrently, and without it both would mint.
async fn invocation_feeds(

• Files reviewed: 16/17 changed files
• Comments generated: 2
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

The current Connect-mint accounting can silently mis-attribute identity/tier when multiple org: feeds are minted in one invocation, which can break the rate-limit/attribution contract.

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

Review details

Suppressed comments (1)

src/utils/config.rs:4068

• The doc comment says “The lock is held across the mint”, but the implementation explicitly releases the map mutex before initialization and relies on the per-target OnceCell to prevent double-minting. This mismatch makes the concurrency story harder to reason about and can mislead future changes.

/// Get this invocation's feed set for `target`, minting on first use.
///
/// The lock is held across the mint deliberately: `sdk install` runs the rootfs
/// and initramfs installs concurrently, and without it both would mint.

• Files reviewed: 16/17 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

It introduces a new auth/token minting path plus cross-step credential sharing and invocation-wide caching, which is security- and correctness-sensitive and warrants final human review.

Review details

Suppressed comments (1)

src/utils/config.rs:4068

• These docs say the mutex lock is held across the mint, but the implementation explicitly releases the map lock before initialization and relies on the per-target OnceCell to deduplicate mints. The comment is currently self-contradictory with the explanation immediately below.

/// Get this invocation's feed set for `target`, minting on first use.
///
/// The lock is held across the mint deliberately: `sdk install` runs the rootfs
/// and initramfs installs concurrently, and without it both would mint.

• Files reviewed: 16/17 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

There are a few correctness/behavior issues around Connect token minting and stage scoping (plus a misleading concurrency comment and suppressed config-load errors) that should be addressed before approval.

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

Review details

Suppressed comments (2)

Previously missed (1) — in code that hasn't changed since the last review.

src/utils/feeds.rs:780

• load_config() errors (read/parse failures) are currently discarded via .ok().flatten(), which can turn a real config problem into a confusing "not logged in" error. Propagating the error will preserve the underlying cause and context.

src/utils/config.rs:4068

• The doc comment says the lock is held across the mint, but the implementation releases the HashMap mutex before initialization and relies on the per-target OnceCell to ensure a single mint. This is misleading for future maintainers.

/// Get this invocation's feed set for `target`, minting on first use.
///
/// The lock is held across the mint deliberately: `sdk install` runs the rootfs
/// and initramfs installs concurrently, and without it both would mint.

• Files reviewed: 16/17 changed files
• Comments generated: 2
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

There are confirmed correctness and input-validation issues in org: feed handling (credentials can bypass minting; branch/path segment validation; tier floor handling) that should be fixed before approval.

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

Review details

Suppressed comments (1)

src/utils/feeds.rs:669

• channel is used as the org feed's branch path segment, but it isn't validated like org is. Values containing /, whitespace, or .. will produce a malformed connect://... placeholder and later a minted URL that doesn't match the expected /private/<rel>/orgs/<org>/<branch>/... layout. Consider validating the effective branch with the same single-segment rule as org.

                let branch = def.channel.clone().unwrap_or_else(|| "main".to_string());
                let path = format!("{rel}/orgs/{org}/{branch}/target/{target}");

• Files reviewed: 16/17 changed files
• Comments generated: 2
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

The mint request User-Agent can misattribute non-default-profile mints, and org: feeds should validate the channel/branch segment to avoid malformed or unsafe Connect feed paths.

Review details

Suppressed comments (3)

Previously missed (2) — in code that hasn't changed since the last review.

src/utils/feeds.rs:539

• For org: feeds, channel is used as the branch path segment (.../orgs/<org>/<branch>/...), but only org is validated as a single URL-safe segment. Allowing slashes or whitespace in channel will produce malformed Connect feed URLs and can break the server-side path parsing logic.
  scripts/local-feeds/edge.py:332
• The --dir help text says the feed root is served under /private/<org>/, but the handler actually serves objects under /private/<release>/orgs/<org>/<branch>/... (and rewrites away those leading segments when mapping to the local directory). Updating the help string will make it easier to run the rig correctly.

src/utils/feeds.rs:900

• The feed-token mint request always uses user_agent() for its User-Agent, which may identify the default Connect profile even when the mint is performed with a non-default profile (selected by org). That undermines the goal of attributing requests to the credential that actually made them and can also skew any per-credential mint rate limiting.

            let url = format!(
                "{}/api/orgs/{org}/feed-tokens",
                api_url.trim_end_matches('/')
            );
            let resp = client
                .post(&url)
                .bearer_auth(&account_token)
                .json(&serde_json::json!({}))
                .send()
                .await
                .with_context(|| {
                    format!("repos.{}: requesting a feed token from {url}", feed.name)
                })?;

• Files reviewed: 16/17 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

The new Connect mint path currently suppresses Connect config load/parse errors, which can produce misleading “not logged in” guidance instead of surfacing the real failure mode.

Review details

Suppressed comments (2)

Previously missed (1) — in code that hasn't changed since the last review.

src/utils/feeds.rs:847

• resolve_connect_credentials currently ignores errors from connect::client::load_config() via .ok().flatten(). If the Connect config exists but is unreadable or invalid JSON, this will be treated as “not logged in”, which hides the real problem and gives the user the wrong remediation.

src/utils/config.rs:4071

• The doc comment for invocation_feeds still says it is “minting on first use” and that the lock is held across the mint, but the function now only initializes the per-target OnceCell/tempdir and does no minting. Updating this avoids misleading future changes around the locking strategy.

/// Get this invocation's feed set for `target`, minting on first use.
///
/// The lock is held across the mint deliberately: `sdk install` runs the rootfs
/// and initramfs installs concurrently, and without it both would mint.

• Files reviewed: 16/17 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

It introduces security- and caching-critical behavior (token minting, per-invocation shared state, async/concurrency changes) that warrants final human review, and there is at least one identified correctness concern around cached vs re-resolved feed sets.

Review details

• Files reviewed: 16/17 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

Config::materialize_feeds still re-resolves feeds on subsequent calls and can fail mid-invocation before reusing the pinned OnceCell set, breaking the intended “resolve once per invocation” behavior.

Review details

Suppressed comments (1)

src/utils/config.rs:4138

• materialize_feeds still resolves feeds on every call, even after the per-target OnceCell has pinned a set for this invocation. If resolution can move or fail mid-invocation (e.g., path-feed repodata changes, env-driven releasever), later calls can error out before they ever reuse the cached set, defeating the “resolve once per invocation” guarantee and potentially breaking builds that should be coherent.

        let project_root = self.project_root(config_path);
        let Some(set) =
            crate::utils::feeds::ResolvedFeedSet::resolve(self, target, &project_root, None)?
        else {
            return Ok(None);

• Files reviewed: 16/17 changed files
• Comments generated: 0 new
• Review effort level: Lite