Skip to content

fix: oci creds incorrectly omitted when sourceRepos set to specific registry path#26824

Open
0xVox wants to merge 1 commit intoargoproj:masterfrom
0xVox:fix/repo-creds-not-matched-with-oci-repo
Open

fix: oci creds incorrectly omitted when sourceRepos set to specific registry path#26824
0xVox wants to merge 1 commit intoargoproj:masterfrom
0xVox:fix/repo-creds-not-matched-with-oci-repo

Conversation

@0xVox
Copy link

@0xVox 0xVox commented Mar 13, 2026

Summary

  • Fix GetPermittedReposCredentials to permit credential templates when their URL is a prefix of any permitted source repo in the AppProject
  • Add IsCredentialPermittedForAnySource method to AppProject to handle prefix-based credential matching
  • Add comprehensive tests for the new credential permission logic

Context

When an AppProject's sourceRepos uses *, all repository credentials pass the permission filter and are sent to the repo-server. However, when sourceRepos is scoped to specific repositories (e.g. reg.example.com/org/charts), credential templates with broader prefix URLs (e.g. reg.example.com) are filtered out, causing helm dependency build to fail with 403 errors when resolving OCI Helm chart dependencies.

This affects any Git-sourced Application containing a Helm chart with OCI registry dependencies where the AppProject has restrictive sourceRepos.

Rationale

GetPermittedReposCredentials uses IsSourcePermitted to decide whether a credential should be passed to the repo-server. IsSourcePermitted does exact glob matching: it checks whether the sourceRepos pattern matches the credential URL. But credential template URLs are prefix-based by design — a credential with URL reg.example.com is meant to provide authentication for everything under that hostname (e.g. reg.example.com/org/charts).

When sourceRepos contains a specific path like reg.example.com/org/charts, the glob match against the shorter credential URL reg.example.com always fails, so the credential is filtered out and never reaches the repo-server. The only workaround was to add the exact credential URL (bare hostname) to sourceRepos, which defeats the purpose of restricting which paths the project can access.

Fix

Added IsCredentialPermittedForAnySource on AppProject, which checks the reverse direction: whether any permitted sourceRepo falls under the credential's URL prefix. GetPermittedReposCredentials now calls this as a fallback when IsSourcePermitted doesn't match, so credentials are permitted if they could provide auth for any source the project is allowed to use.

The check normalises both URLs and strips oci:// schemes before comparison, so it handles all combinations of OCI and non-OCI sourceRepos entries.

I suspect this may also close out #26311

@0xVox 0xVox requested a review from a team as a code owner March 13, 2026 15:44
@bunnyshell
Copy link

bunnyshell bot commented Mar 13, 2026

🔴 Preview Environment stopped on Bunnyshell

See: Environment Details | Pipeline Logs

Available commands (reply to this comment):

  • 🔵 /bns:start to start the environment
  • 🚀 /bns:deploy to redeploy the environment
  • /bns:delete to remove the environment

@0xVox 0xVox force-pushed the fix/repo-creds-not-matched-with-oci-repo branch from 7c54d81 to b8604b5 Compare March 13, 2026 15:47
…eg path

Signed-off-by: 0xVox <tom.brew3@gmail.com>
@0xVox 0xVox force-pushed the fix/repo-creds-not-matched-with-oci-repo branch from b8604b5 to a602dc3 Compare March 13, 2026 15:48
Copy link
Member

@blakepettersson blakepettersson left a comment

Choose a reason for hiding this comment

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

The fix is to for the app project to have the relevant source prefix(es) inprojSources no?

@codecov
Copy link

codecov bot commented Mar 13, 2026

Codecov Report

❌ Patch coverage is 87.50000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 62.98%. Comparing base (3a4b0a0) to head (a602dc3).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
pkg/apis/application/v1alpha1/app_project_types.go 86.66% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #26824      +/-   ##
==========================================
- Coverage   62.98%   62.98%   -0.01%     
==========================================
  Files         414      414              
  Lines       56153    56168      +15     
==========================================
+ Hits        35370    35378       +8     
- Misses      17416    17420       +4     
- Partials     3367     3370       +3     

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

@0xVox
Copy link
Author

0xVox commented Mar 13, 2026

The fix is to for the app project to have the relevant source prefix(es) inprojSources no?

It certainly works that way, and the credentials will be passed through. But, if you were to attempt further granularity with the sourceRepos control with a path-level restriction, the creds were no longer passed through properly.

This fixes that, and enables that further granularity in sourceRepos control.

See: #26311 (comment)

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