Skip to content

feat(backend): implement proxy cache repository filter API#23527

Open
stonezdj wants to merge 1 commit into
goharbor:mainfrom
stonezdj:feature/proxy-cache-filter-backend
Open

feat(backend): implement proxy cache repository filter API#23527
stonezdj wants to merge 1 commit into
goharbor:mainfrom
stonezdj:feature/proxy-cache-filter-backend

Conversation

@stonezdj

@stonezdj stonezdj commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Introduce repository filter pattern and kind on proxy cache projects to explicitly allow selected repositories. Requests that do not match the filter are denied before Harbor proxies content from upstream.

Fixes #13231

Thank you for contributing to Harbor!

Comprehensive Summary of your change

Issue being fixed

Fixes #(issue)

Please indicate you've done the following:

  • Well Written Title and Summary of the PR
  • Label the PR as needed. "release-note/ignore-for-release, release-note/new-feature, release-note/update, release-note/enhancement, release-note/community, release-note/breaking-change, release-note/docs, release-note/infra, release-note/deprecation"
  • Accepted the DCO. Commits without the DCO will delay acceptance.
  • Made sure tests are passing and test coverage is added if needed.
  • Considered the docs impact and opened a new docs issue or PR with docs changes if needed in website repository.

@stonezdj
stonezdj requested a review from a team as a code owner July 9, 2026 08:35
Introduce repository filter pattern and kind on proxy cache projects
to explicitly allow selected repositories. Requests that do not match the
filter are denied before Harbor proxies content from upstream.

Fixes goharbor#13231

Signed-off-by: stonezdj <stone.zhang@broadcom.com>
@codecov

codecov Bot commented Jul 9, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 43.80952% with 59 lines in your changes missing coverage. Please review.
✅ Project coverage is 66.34%. Comparing base (c86fd24) to head (e4763ca).
⚠️ Report is 15 commits behind head on main.

Files with missing lines Patch % Lines
src/server/v2.0/handler/project.go 0.00% 27 Missing ⚠️
src/server/v2.0/handler/project_metadata.go 0.00% 14 Missing ⚠️
src/lib/pattern/matcher.go 83.72% 7 Missing ⚠️
src/server/middleware/repoproxy/proxy.go 58.82% 7 Missing ⚠️
src/server/v2.0/handler/model/project.go 0.00% 4 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main   #23527      +/-   ##
==========================================
- Coverage   66.38%   66.34%   -0.04%     
==========================================
  Files        1072     1073       +1     
  Lines      117541   117665     +124     
  Branches     2965     2965              
==========================================
+ Hits        78024    78062      +38     
- Misses      35224    35306      +82     
- Partials     4293     4297       +4     
Flag Coverage Δ
unittests 66.34% <43.80%> (-0.04%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/server/v2.0/handler/model/project.go 0.00% <0.00%> (ø)
src/lib/pattern/matcher.go 83.72% <83.72%> (ø)
src/server/middleware/repoproxy/proxy.go 6.99% <58.82%> (+3.27%) ⬆️
src/server/v2.0/handler/project_metadata.go 12.78% <0.00%> (-1.51%) ⬇️
src/server/v2.0/handler/project.go 4.70% <0.00%> (-0.18%) ⬇️

... and 13 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@stonezdj
stonezdj force-pushed the feature/proxy-cache-filter-backend branch from 918d806 to e4763ca Compare July 9, 2026 09:01
@Vad1mo
Vad1mo requested a review from Copilot July 10, 2026 07:02

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.

Pull request overview

Adds configurable repository allowlist filtering for proxy-cache projects.

Changes:

  • Adds doublestar and regex repository matchers.
  • Exposes and validates filter metadata through project APIs.
  • Enforces filtering for manifest proxying and adds matcher tests.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
api/v2.0/swagger.yaml Documents filter fields.
src/lib/pattern/matcher.go Implements repository matching.
src/lib/pattern/matcher_test.go Tests matcher behavior.
src/pkg/project/models/pro_meta.go Defines metadata keys.
src/server/middleware/repoproxy/proxy.go Filters manifest requests.
src/server/middleware/repoproxy/proxy_test.go Tests proxy filter matching.
src/server/v2.0/handler/model/project.go Hides fields for regular projects.
src/server/v2.0/handler/project.go Validates project filter metadata.
src/server/v2.0/handler/project_metadata.go Supports metadata API updates.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +239 to +240
// Apply repository filter: if proxy_cache_filter_pattern is set, the repository must match
if filterPattern, ok := p.GetMetadata(proModels.ProMetaProxyCacheFilterPattern); ok && filterPattern != "" {
// see https://github.com/goharbor/harbor/issues/12940 to get more info
if params.Project.Metadata != nil && p.IsProxy() {
params.Project.Metadata.EnableContentTrust = nil
if err := validateProxyCacheRepositoryFilter(params.Project.Metadata); err != nil {
metas[proModels.ProMetaMaxUpstreamConn] = strconv.FormatInt(v, 10)
case proModels.ProMetaProxyCacheFilterPattern:
// plain pattern string; empty means match all — no further validation needed at metadata level
if err := pattern.ValidateRepositoryFilter(value, pattern.KindDoublestar); err != nil {

@wy65701436 wy65701436 Jul 24, 2026

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.

Is hard code the KindDoublestar intended?

@Vad1mo

Vad1mo commented Jul 10, 2026

Copy link
Copy Markdown
Member

Comparing this against the merged proposal (goharbor/community#280), a few deviations:

1. Pattern validation at the metadata endpoint ignores the configured kind

The proposal requires: "Invalid pattern in proxy_cache_filter_pattern is rejected with 400 Bad Request when creating/updating project metadata."

In project_metadata.go, validate() checks each key independently, so the pattern is always validated as doublestar:

case proModels.ProMetaProxyCacheFilterPattern:
    if err := pattern.ValidateRepositoryFilter(value, pattern.KindDoublestar); err != nil {

With proxy_cache_filter_kind=regex, an invalid regex that is a valid glob (e.g. library/(*) passes validation here, and since invalid patterns are treated as non-match at runtime, every pull in the project silently 404s. A valid regex that is an invalid glob is wrongly rejected. validateProxyCacheRepositoryFilter in project.go gets this right — the metadatas endpoint needs the same cross-field logic.

2. Metadata endpoint accepts the keys on non-proxy projects

Proposal: "For non-proxy projects, these keys are ignored on create/update and omitted from response." Create/Update strip them and ToSwagger hides them, but PUT/POST /projects/{id}/metadatas stores both keys on any project.

3. Regex "auto-anchoring" isn't equivalent to real anchoring

Proposal: regex is "always auto-anchored." RegexMatcher.Match checks that the leftmost match (FindStringIndex) spans the whole string. Go regexp alternation is leftmost-first, so a|ab fails to match "ab" even though ^(?:a|ab)$ matches. Compiling ^(?: + pattern + )$ implements the stated semantics correctly. Also worth documenting explicitly that patterns must match the full repository path (^library/ matches nothing) — the companion UI PR #23536 currently describes this as "head-anchored", which doesn't match this implementation.

4. No audit entry when the filter changes

Proposal, Audit section: "Record project update audit entries when filter set is changed." Only the blocked-pull log is implemented.

5. Blob path is not filtered (matches the proposal's implementation section, but not its Abstract)

Enforcement is in handleManifest only, exactly as the proposal's implementation section specifies. However the Abstract says requests are "denied before Harbor proxies content from upstream" — a client that knows a blob digest out-of-band can still pull blobs of a filtered-out repository through the blob proxy path. If the filter is meant as a security boundary rather than a convenience, blob requests need the same check; otherwise this limitation should be documented.

@Vad1mo

Vad1mo commented Jul 10, 2026

Copy link
Copy Markdown
Member

backward-compatibility analysis

  • The filter applies retroactively to already-cached content: adding a filter to an existing proxy project blocks pulls of previously cached non-matching repos (the proposal explicitly applies the pattern to local content too). Clusters pulling those images break at the moment the filter is saved, not at the next upstream fetch.
  • Downgrade/rollback: the metadata rows persist, older Harbor silently ignores them, and filter enforcement vanishes without any warning — a silent security regression if the filter was doing compliance work.

if err != nil {
return false, err
}
match := re.FindStringIndex(value)

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.

FindStringIndex returns the leftmost match, not necessarily a full-string match, so this can return false negatives.

Example: pattern a|ab against value ab — the leftmost match is a ([0,1]), so this returns false even though the pattern matches the whole string.

// filterPattern is the plain pattern string; filterKind is "doublestar" or "regex" (defaults to doublestar when empty).
// If filterPattern is empty, all repositories are allowed.
// An invalid pattern is treated as no match.
func matchRepositoryFilter(repository, filterPattern, filterKind string) bool {

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.

the matchRepositoryFilter is applied only for manifest, should we apply it for blob?

Comment on lines +1 to +117
// Copyright Project Harbor Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package pattern

import (
"regexp"
"strings"

"github.com/bmatcuk/doublestar"

"github.com/goharbor/harbor/src/lib/errors"
)

const (
// KindRegex indicates regular expression pattern matching
KindRegex = "regex"
// KindDoublestar indicates doublestar (glob) pattern matching
KindDoublestar = "doublestar"
)

// RepositoryFilter represents a repository filter configuration with pattern and kind
type RepositoryFilter struct {
// Filter is the pattern expression to match against
Filter string
// Kind is the type of pattern matching: "regex" or "doublestar"
Kind string
}

// NewRepositoryFilter creates a RepositoryFilter from separate pattern and kind strings.
// If kind is empty, it defaults to KindDoublestar.
func NewRepositoryFilter(filterPattern, kind string) *RepositoryFilter {
return &RepositoryFilter{
Filter: strings.TrimSpace(filterPattern),
Kind: strings.TrimSpace(kind),
}
}

// Match returns true if the value matches the filter pattern.
// Returns true if the filter is empty.
func (rf *RepositoryFilter) Match(value string) (bool, error) {
if rf == nil || rf.Filter == "" {
return true, nil
}
matcher := NewMatcher(rf.Kind)
return matcher.Match(value, rf.Filter)
}

// ValidateRepositoryFilter validates the repository filter kind and pattern.
// Empty pattern is valid and means all repositories are allowed.
func ValidateRepositoryFilter(filterPattern, kind string) error {
rf := NewRepositoryFilter(filterPattern, kind)
if rf.Kind != "" && rf.Kind != KindRegex && rf.Kind != KindDoublestar {
return errors.Errorf("unsupported repository filter kind %q", kind)
}
_, err := rf.Match("")
return err
}

// Matcher is an interface for matching strings against patterns
type Matcher interface {
// Match returns true if the value matches the pattern
Match(value, pattern string) (bool, error)
}

// RegexMatcher implements Matcher using regular expressions
type RegexMatcher struct{}

// Match matches value against a regular expression pattern
func (r *RegexMatcher) Match(value, pattern string) (bool, error) {
pattern = strings.TrimSpace(pattern)
if pattern == "" {
return true, nil
}
re, err := regexp.Compile(pattern)
if err != nil {
return false, err
}
match := re.FindStringIndex(value)
return match != nil && match[0] == 0 && match[1] == len(value), nil
}

// DoublestarMatcher implements Matcher using doublestar (glob) patterns
type DoublestarMatcher struct{}

// Match matches value against a doublestar (glob) pattern
func (d *DoublestarMatcher) Match(value, pattern string) (bool, error) {
pattern = strings.TrimSpace(pattern)
if pattern == "" {
return true, nil
}
return doublestar.Match(pattern, value)
}

// NewMatcher creates a new Matcher based on the specified kind.
// Defaults to KindDoublestar when kind is empty or unrecognised.
func NewMatcher(kind string) Matcher {
switch kind {
case KindRegex:
return &RegexMatcher{}
case KindDoublestar:
fallthrough
default:
return &DoublestarMatcher{}
}
}

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.

func ValidateRepositoryFilter(filterPattern, kind string) error {
    filterPattern = strings.TrimSpace(filterPattern)
    if filterPattern == "" {
        return nil
    }
    switch kind {
    case KindRegex:
        _, err := regexp.Compile(filterPattern)
        return err
    case KindDoublestar, "":
        if !doublestar.ValidatePattern(filterPattern) {
            return doublestar.ErrBadPattern
        }
        return nil
    default:
        return errors.Errorf("unsupported repository filter kind %q", kind)
    }
}


Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Limit Proxy Cache Images - by adding filter

6 participants