scope overwrite_from_index_token for containerized architecture

[yashvardhannanavati]

1. cherry-picks cca5101 from master
2. adds a commit to adopt the change to the containerized workflow : 5fd2361

[qodo-for-releng]

PR Summary by Qodo

Scope overwrite tokens across containerized image pulls

🐞 Bug fix 🧪 Tests 🕐 40+ Minutes

AI Description

• Scope overwrite tokens to uncovered images sharing the source index namespace.
• Preserve existing Docker credentials during containerized bundle and fragment pulls.
• Add regression coverage across legacy and containerized build paths.

Diagram

graph TD
  A["Build handlers"] --> B["Token selector"] --> C{"Path auth exists?"}
  C -->|Yes| D["Existing credentials"] --> F["Image pulls"] --> G["Catalog build"]
  C -->|No| E["Scoped token"] --> F

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Isolated Docker config per operation

• ➕ Provides stronger credential isolation between nested pull operations.
• ➕ Makes restoration semantics independent of the worker template.
• ➖ Requires broader changes across OPM, Podman, and image inspection call sites.
• ➖ Introduces repeated configuration construction and additional filesystem churn.

2. Pass credentials directly to pull commands

• ➕ Avoids mutating the shared Docker configuration.
• ➕ Scopes credentials naturally to individual commands.
• ➖ Authentication options differ across OPM, Podman, and supporting tooling.
• ➖ Would require invasive command wrappers and could expose credentials in process arguments.

Recommendation: Keep the PR's centralized image-selection helper and narrow credential windows. It provides consistent behavior across existing tools with substantially less risk than redesigning credential transport; isolated per-operation configurations remain a worthwhile future improvement if nested authentication grows more complex.

Files changed (13) +1076 / -81

Bug fix (7) +338 / -72

build.pyScope legacy add authentication to required bundle pulls +20/-3

Scope legacy add authentication to required bundle pulls

• Selects same-namespace bundles without path-scoped credentials before applying the overwrite token during resolution. It also preserves Docker configuration while authenticating source-index inspection.

iib/workers/tasks/build.py

build_containerized_add.pyApply scoped authentication throughout containerized add +31/-10

Apply scoped authentication throughout containerized add

• Limits overwrite credentials to eligible bundles during resolution and OPM registry addition. Source-index authentication now explicitly covers OPM version detection for private images.

iib/workers/tasks/build_containerized_add.py

build_containerized_fbc_operations.pyScope credentials for containerized FBC fragment processing +17/-6

Scope credentials for containerized FBC fragment processing

• Filters fragment authentication by namespace and existing Docker credentials, authenticates private source-index inspection, and forwards the source index to downstream extraction.

iib/workers/tasks/build_containerized_fbc_operations.py

build_containerized_rm.pyAuthenticate private index inspection during containerized removal +5/-2

Authenticate private index inspection during containerized removal

• Wraps OPM version detection in an append-mode overwrite-token context so private source indexes remain accessible without replacing unrelated credentials.

iib/workers/tasks/build_containerized_rm.py

build_fbc_operations.pyAvoid unnecessary overwrite tokens during fragment resolution +15/-5

Avoid unnecessary overwrite tokens during fragment resolution

• Uses the shared selector to apply overwrite credentials only when FBC fragments share the source namespace and lack covering Docker authentication.

iib/workers/tasks/build_fbc_operations.py

opm_operations.pyScope authentication around OPM image pulls +76/-28

Scope authentication around OPM image pulls

• Applies filtered overwrite credentials around bundle addition, catalog access, fragment extraction, and index addition. Containerized fragment processing now accepts the source index needed for namespace matching.

iib/workers/tasks/opm_operations.py

utils.pyAdd path-aware Docker authentication selection +174/-18

Add path-aware Docker authentication selection

• Adds Docker-auth discovery and image filtering by registry, namespace, and existing path coverage. Extends token configuration to multiple images with namespace fallback while preserving unrelated credentials and failing safely on unreadable configuration.

iib/workers/tasks/utils.py

Tests (6) +738 / -9

test_build_containerized_add.pyTest scoped token use in containerized add +143/-0

Test scoped token use in containerized add

• Adds end-to-end handler coverage proving same-namespace bundles receive overwrite credentials while unrelated namespaces retain their existing authentication.

tests/test_workers/test_tasks/test_build_containerized_add.py

test_build_containerized_fbc_operations.pyTest containerized FBC credential scoping +122/-4

Test containerized FBC credential scoping

• Updates source-index expectations and verifies fragment resolution scopes tokens by namespace. It also confirms the source index reaches the downstream extraction operation.

tests/test_workers/test_tasks/test_build_containerized_fbc_operations.py

test_build_containerized_rm.pyTest private index authentication during removal +12/-2

Test private index authentication during removal

• Verifies containerized removal applies the overwrite token to the resolved private index before OPM version detection and updates affected mocks.

tests/test_workers/test_tasks/test_build_containerized_rm.py

test_build_fbc_operations.pyTest conditional legacy fragment authentication +124/-0

Test conditional legacy fragment authentication

• Covers both covered and uncovered FBC fragments, ensuring the token context is skipped when existing credentials suffice and entered otherwise.

tests/test_workers/test_tasks/test_build_fbc_operations.py

test_opm_operations.pyTest scoped credentials across OPM operations +116/-1

Test scoped credentials across OPM operations

• Updates OPM expectations for list-aware token contexts and adds containerized fragment extraction tests for namespace scoping and absent source indexes.

tests/test_workers/test_tasks/test_opm_operations.py

test_utils.pyTest path-aware and multi-image Docker authentication +221/-2

Test path-aware and multi-image Docker authentication

• Adds coverage for auth-key discovery, configuration loading failures, namespace fallback, multiple images, empty lists, and image eligibility filtering. Existing assertions now account for preserved and namespace-scoped credentials.

tests/test_workers/test_tasks/test_utils.py

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 11:33 AM UTC · Completed 12:03 PM UTC

Commit: 5fd2361 · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $11.30

[qodo-for-releng]

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📜 Skill insights (0)

1. Private bundle pulls fail despite token 🐞 Bug ≡ Correctness

Description

docker_config_has_path_auth_for_image treats key membership as sufficient without checking whether
the matching entry contains usable authentication data. When a namespace or repository key maps to
an empty or stale object and no credential helper supplies credentials, same-namespace bundle and
fragment workflows omit the supplied overwrite token and reach their registry pulls unauthenticated.

Code

iib/workers/tasks/utils.py[R793-794]

+    path_keys = keys[:-1] if len(keys) > 1 else []
+    return any(key in auths for key in path_keys)

Relevance

●●● Strong

Empty or stale auth entries falsely suppress token injection, directly causing private
same-namespace pulls to fail.

PR-#1370

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The coverage function returns true solely when a generated path key exists in auths, regardless of
that key's value. get_images_needing_overwrite_token then excludes such images from its result,
and the changed bundle-resolution workflow only installs the overwrite token for images returned by
that selector.

iib/workers/tasks/utils.py[779-794]
iib/workers/tasks/utils.py[823-841]
iib/workers/tasks/build.py[858-874]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new Docker-config coverage check considers an image authenticated whenever its namespace or repository key exists, even if that entry contains no usable credentials. This can suppress `overwrite_from_index_token` and leave private bundle or fragment pulls unauthenticated.

## Fix Focus Areas
- iib/workers/tasks/utils.py[779-794]
- iib/workers/tasks/utils.py[823-841]

## Recommended Fix
Inspect each matching auth entry rather than checking key membership alone. Count an entry as covering only when it contains usable inline authentication such as a nonempty `auth` or `identitytoken`, while explicitly handling any supported credential-helper configuration; add tests for empty and stale matching entries.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Context sources

Review mode: 🧠 Deep: This is a broad, security-sensitive authentication change spanning multiple independent build and OPM paths, with substantial new credential-scoping logic and many opportunities for subtle defects.

Tip of the day

💡 Did you know, you can reply 'qodo' on any finding to push back, ask questions, or dig deeper

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[fullsend-ai-review]

Risk Assessment: moderate (2/5)

Details

Moderate risk: the PR spans 13 files with 1157 lines and a large blast radius, but is a focused cherry-pick of a token-scoping fix extended to containerized workflows, has no security-sensitive or CI changes, carries a near-threshold test ratio of 0.46, and touches historically fix-heavy modules with low recent churn.

[fullsend-ai-review]

Review

Findings

Medium

• [logic-error] iib/workers/tasks/build.py:898 — Opm.set_opm_version(from_index_resolved) is called outside any set_registry_token auth context in the non-containerized handle_add_request. This reads a label from from_index via skopeo_inspect, which requires authentication for private images. All three containerized variants in this PR explicitly wrap this call with set_registry_token(overwrite_from_index_token, from_index_resolved, append=True) with a comment explaining the need. The non-containerized path does not apply the same fix, so private from_index images will fail at this step. The same gap exists in build_fbc_operations.py line 105. Note: prepare_request_for_build reads the same label within an auth context via get_index_image_info, so with the dogpile cache enabled the second call may hit cache; with the default null cache, the second call would fail for private images.
  Remediation: Wrap Opm.set_opm_version(from_index_resolved) with set_registry_token(overwrite_from_index_token, from_index_resolved, append=True) in both build.py and build_fbc_operations.py, matching the containerized variants.

• [code-organization] iib/workers/tasks/opm_operations.py:1420 — opm_index_add duplicates the run_cmd(cmd, {'cwd': base_dir}, exc_msg='Failed to add the bundles to the index image') call verbatim in both branches of the if token_images: ... else: ... block. Comments elsewhere in the same file (lines 275 and 318) explicitly document that always entering set_registry_token covers both cases because it no-ops on an empty list. The inconsistency within the same file creates a maintenance hazard: if run_cmd arguments are updated in one branch but not the other.
  Remediation: Remove the if/else and always call with set_registry_token(overwrite_from_index_token, token_images, append=True): run_cmd(...).

• [missing-authorization] — Non-trivial change (hundreds of lines across 7 source files and 6 test files) with no linked issue. The PR body states it cherry-picks master commit cca51015 and extends the fix to the containerized workflow, but no issue authorization is present.
  Remediation: Link a GitHub issue that explicitly authorizes the backporting and extension.

• [stale-doc] README.md:319 — The iib_docker_config_template config entry says IIB will "set the overwrite_from_index_token for the specific repository of the from_index container image when applicable." This PR changes that behavior: the token is now scoped to same-namespace bundles/FBC fragments that lack path-scoped Docker credentials, and registry-only docker-config keys no longer count as coverage.
  Remediation: Update the description to reflect the new same-namespace, path-auth-aware scoping.

• [stale-doc] docs/gettingstarted.md:319 — Identical stale description as README.md. After this PR the token is scoped to same-namespace images lacking path-scoped Docker auth, not solely to the from_index repository.
  Remediation: Apply the same correction as README.md.

Low

• [API-shape-patterns] iib/workers/tasks/build.py:861 — Three different patterns are used across the PR to call set_registry_token when the image list might be empty: (1) containerized handlers pass get_images_needing_overwrite_token() directly (always-enter); (2) non-containerized handlers pre-compute the list and use if/else with a nonlocal inner function; (3) opm_index_add uses if/else with a duplicated run_cmd call. All three are functionally correct since set_registry_token no-ops on a falsy container_image, but the inconsistency adds cognitive load.
  Remediation: Adopt the always-enter pattern uniformly.

• [edge-case] iib/workers/tasks/opm_operations.py:1529 — opm_deprecatetruncate calls set_registry_token(overwrite_target_index_token, from_index) with append=False (the default). The new namespace fallback logic now also sets a namespace-level auth key when no path-scoped auth exists. This is a behavior change for this call site that was not explicitly targeted by the PR.

• [authorization] iib/workers/tasks/utils.py:940 — Namespace-level token fallback broadens auth blast radius within same namespace. When no path-scoped Docker config auth exists for an image, set_registry_token stamps the token at both the repo key and the namespace key. The blast radius is bounded by three conditions (same registry, same namespace, no pre-existing path-scoped auth), and the code comments explicitly acknowledge this trade-off.

• [documentation-comment-format] tests/test_workers/test_tasks/test_utils.py:1503 — test_set_registry_token_multiple_images has no docstring while the two adjacent new test functions added by the same PR both carry descriptive one-line docstrings.
  Remediation: Add a one-line docstring.

---

Next steps:

• /fs-fix — agent addresses review findings automatically
• /fs-fix <your instruction> — agent fixes with your specific guidance
• Push commits directly — review re-runs automatically on push
• /fs-fix-stop — disable automatic fix runs for this PR

[fullsend-ai-review]

Note: The following review comments could not be posted on the diff (GitHub returned 422) and are included here instead:

• iib/workers/tasks/opm_operations.py (file-level): Line 1529 · [low] edge-case

opm_deprecatetruncate calls set_registry_token with append=False. The new namespace fallback logic is an unintended behavior change for this call site.

• tests/test_workers/test_tasks/test_utils.py (file-level): Line 1503 · [low] documentation-comment-format

test_set_registry_token_multiple_images has no docstring while adjacent tests added by the same PR do.

Suggested fix: Add a one-line docstring.

[fullsend-ai-review]

See the review comment for full details.

[fullsend-ai-retro]

🤖 Finished Retro · ✅ Success · Started 6:06 AM UTC · Completed 6:23 AM UTC

Commit: 5fd2361 · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $4.63

[fullsend-ai-retro]

Retro Analysis: PR #1388

Timeline

1. 2026-09-11T11:32 — PR #1388 opened by yashvardhannanavati: "scope overwrite_from_index_token for containerized architecture" (13 files, +1076/−81). Cherry-pick from master plus containerized workflow adaptation.
2. 2026-09-11T11:33 — qodo-for-releng bot posts summary and 1 medium bug finding about docker_config_has_path_auth_for_image not validating auth entry completeness.
3. 2026-09-11T11:33–12:03 — fullsend-ai-review runs (workflow 34594551974), model claude-opus-4-6, effort high, cost $11.30.
4. 2026-09-11T12:03 — Review agent posts risk assessment (moderate, 2/5) and CHANGES_REQUESTED with 4 medium + 4 low findings.
5. 2026-09-14T14:46 — Human reviewer lipoja approves.
6. 2026-09-15T06:05 — PR merged by author.

Review Finding Quality

The review agent produced 8 findings. Verification against source code:

#	Sev	Category	Verdict
M1	Med	logic-error: set_opm_version outside auth context	Valid — PR wraps this in 3 containerized handlers but omits non-containerized equivalents
M2	Med	code-organization: duplicated run_cmd in opm_index_add	Valid — contradicts the PR's own clean no-op pattern used elsewhere
M3	Med	missing-authorization: no linked issue	False positive — PR is a cherry-pick with original commit tracking
M4	Med	stale-doc: README/docs outdated	Valid (line number off: cited 319, content at 381)
L1	Low	API-shape: inconsistent calling patterns	Valid
L2	Low	edge-case: opm_deprecatetruncate namespace fallback	Uncertain
L3	Low	authorization: namespace fallback broadens blast radius	False positive — code has explicit multi-line documentation stating this is deliberate
L4	Low	missing docstring on test	False positive — wrong line number; no tests in the file use docstrings

Precision: 50% (4 valid, 3 false positive, 1 uncertain).

Key Observations

1. Valid bugs shipped unaddressed. M1 and M2 identify genuine inconsistencies in this PR. The auth context gap (M1) means Opm.set_opm_version can fail on private indices in the non-containerized path — the author added the wrapping in 3 containerized handlers but missed the non-containerized equivalents. Neither finding was addressed before merge.

2. False positive rate may have contributed to findings being ignored. At 37.5% false positives, the signal-to-noise ratio is low enough that reviewers may learn to dismiss agent findings. The "no linked issue" finding on a cherry-pick is particularly noisy.

3. Complementary finding missed. The qodo bot caught that docker_config_has_path_auth_for_image checks key membership without verifying auth entry validity. The fullsend agent was reading the same file (flagged L3 at nearby lines) but missed this validation gap in a new function.

Evidence for Existing Issues

• release-engineering/iib#1366 (AGENTS.md): This retro provides additional evidence. The missed qodo finding about Docker config auth validation and the false positive about test docstrings (project convention) both support the need for domain-specific review guidance.
• fullsend-ai/fullsend#3679 (cherry-pick detection): Finding M3 ("no linked issue" on a cherry-pick) is a direct instance of the problem this issue aims to solve.

Proposals skipped (target repo not allowed)

File manually or update create_issues.allow_targets in config.yaml:

• Review sub-agents should not flag explicitly documented design trade-offs (fullsend-ai/agents)