Proposal: Probe-Based Authentication Dispatch - #2
Conversation
📝 WalkthroughWalkthroughChangesRepository Metadata Updates
Arm64 Harbor Support
AI Model Processor
Proxy Cache Repository Filtering
Proxy Cache Authentication Path Prefix
Garbage Collection Performance
Proxy Cache Referrers API
Single Active Replication
Playwright UI E2E Migration
Valkey Runtime Migration
Probe-Based Authentication Dispatch
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
🟠 Major comments (29)
proposals/new/proxy_cache_referer_api.md-87-87 (1)
87-87: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftDo not treat all existing accessory records as local.
The merge logic filters by
source, but defaulting every migrated row tolocalmisclassifies previously cached upstream accessories and can produce duplicates or incorrect provenance indefinitely. Preserve anunknownstate, derive provenance where possible, or make merging digest-based and independent of this backfill assumption.Also applies to: 140-146
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@proposals/new/proxy_cache_referer_api.md` at line 87, Update the accessory migration/backfill and merge logic around the source field so existing records are not all classified as "local". Preserve an "unknown" source when provenance cannot be determined, derive the actual source where available, and ensure upstream/local merging does not duplicate records or rely on the incorrect backfill assumption; apply the same correction to the related logic at the referenced merge section.proposals/new/proxy_cache_referer_api.md-104-111 (1)
104-111: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftDefine pagination over the merged result, not only on the last upstream page.
Appending local referrers to the final upstream page means clients that stop after the first page never see local entries. The implementation must merge, deduplicate, order, and paginate the combined result while preserving correct
Linkmetadata.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@proposals/new/proxy_cache_referer_api.md` around lines 104 - 111, Implement ProxyReferrerMiddleware pagination over the complete merged referrer set rather than appending local referrers only to the upstream’s final page. After retrieving upstream or cached referrers, merge and deduplicate them with local referrers, apply the requested ordering and pagination to the combined result, and generate Link metadata from the merged pagination state.proposals/probe-based-auth-dispatch.md-111-122 (1)
111-122: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winPreserve the registry path prefix in the auth probe.
GET /v2/only works for root-mounted registries; path-prefixed endpoints need<configured-prefix>/v2/(or equivalent URL-path derivation) or proxy auth will fail the same way as the path-prefix bug inproposals/new/fix-proxy-cache-auth-path-prefix.md.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@proposals/probe-based-auth-dispatch.md` around lines 111 - 122, Update the Registry Proxy Auth Probe request path to derive and preserve the configured registry path prefix, issuing the probe against <configured-prefix>/v2/ (or the equivalent URL-path derivation) instead of always using /v2/. Keep the existing Bearer, Basic, and anonymous response handling unchanged.proposals/probe-based-auth-dispatch.md-104-109 (1)
104-109: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winBind cached tokens to the authenticated principal and action scope. In
proposals/probe-based-auth-dispatch.md:104-109, 219-222, repository-only caching can let two users on the same repo share a token, and repo-level scoping still mixes pull/push/delete permissions. Include the principal and requested actions in the cache key, or spell out why token sharing is safe.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@proposals/probe-based-auth-dispatch.md` around lines 104 - 109, Update the token-caching guidance in the “Registry Proxy User Credential Passthrough” section and its corresponding section to bind cached tokens to the authenticated principal, repository scope, and requested actions. Ensure pull, push, and delete permissions cannot share a cache entry across users or action scopes, or explicitly document why such sharing is safe.proposals/new/proxy_cache_referer_api.md-134-138 (1)
134-138: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winFix the migration SQL syntax.
IF NOT EXISTSmust followADD COLUMNin PostgreSQL; this form will fail as written.Proposed correction
-ALTER TABLE artifact_accessory IF NOT EXISTS ADD COLUMN source VARCHAR(64) DEFAULT 'local' NOT NULL; +ALTER TABLE artifact_accessory ADD COLUMN IF NOT EXISTS source VARCHAR(64) NOT NULL DEFAULT 'local';🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@proposals/new/proxy_cache_referer_api.md` around lines 134 - 138, Update the artifact_accessory ALTER TABLE statement so IF NOT EXISTS follows ADD COLUMN, preserving the source column definition, default value, and NOT NULL constraint.proposals/new/proxy_cache_referer_api.md-113-127 (1)
113-127: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftExpose the upstream status in
ListReferrers. The middleware needs to distinguish404from other failures to honor the passthrough/fallback behavior, so return the HTTP status explicitly or wrap the error in a typed status-bearing error.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@proposals/new/proxy_cache_referer_api.md` around lines 113 - 127, Update the default RegistryClient.ListReferrers implementation to expose the upstream HTTP status, either by returning it explicitly or by wrapping failures in the established typed status-bearing error; preserve successful index and header results while allowing callers to distinguish 404 from other failures for passthrough/fallback handling.proposals/new/fix-proxy-cache-auth-path-prefix.md-1-1 (1)
1-1: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftMake the registry auth probe path-prefix-aware.
The
/v2/probe breaks registries served under a URL prefix.
proposals/new/fix-proxy-cache-auth-path-prefix.md#49-63: derive the auth probe prefix from the configured registry URL instead of splitting on the first/v2/.proposals/probe-based-auth-dispatch.md#111-122: build the probe from the configured endpoint too; hardcodingGET /v2/will miss the prefix for proxy-backed registries.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@proposals/new/fix-proxy-cache-auth-path-prefix.md` at line 1, Update the proposal’s auth probe guidance to derive the probe path prefix from the configured registry URL/endpoint, rather than splitting at the first “/v2/” or hardcoding “GET /v2/”. Apply this consistently to the registry auth probe and probe-based auth dispatch sections so proxy-backed registries with URL path prefixes are addressed.proposals/new/fix-proxy-cache-auth-path-prefix.md-49-63 (1)
49-63: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDon't split on the first
/v2/segment
A registry path prefix can also contain/v2/, sostrings.Indexcan pick the wrong boundary and build the auth probe against the wrong URL. Derive the prefix from the configured endpoint instead, and add a regression test with a prefix like/artifactory/v2/docker-virtual.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@proposals/new/fix-proxy-cache-auth-path-prefix.md` around lines 49 - 63, Update the auth probe URL construction in the authorizer flow to derive the registry path prefix from the configured endpoint rather than splitting the outgoing request path at the first “/v2/” occurrence. Preserve prefixes containing “/v2/”, such as “/artifactory/v2/docker-virtual”, and add a regression test covering that configured endpoint and the resulting probe URL.proposals/probe-based-auth-dispatch.md-54-62 (1)
54-62: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftDefine the backend order and fallthrough criteria.
In the dispatch section,priority orderis still underspecified, andFail → continuein the OIDC flow doesn’t separate invalid credentials from transient backend outages. Spell out the exact backend precedence and only continue on recoverable/unavailable errors; rejected credentials should stop the chain.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@proposals/probe-based-auth-dispatch.md` around lines 54 - 62, Clarify the dispatch section by explicitly listing the backend precedence used by the dispatcher. Define fallthrough so only recoverable or unavailable backend errors continue to the next backend, while invalid or rejected credentials terminate dispatch and return the authentication failure; preserve fatal Match() errors as immediate failures.proposals/new/gc_perf_enhancement.md-40-43 (1)
40-43: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftDefine atomicity and recovery for tag persistence versus the backend push.
The database write and digest-based registry
PUTare separate operations. If the database succeeds first and the backend push fails, Harbor can expose a tag pointing to a nonexistent digest; if the backend succeeds and the database write fails, the manifest becomes orphaned. “Consistency at the DB level” does not solve this cross-system failure window. Specify rollback/compensating deletion, idempotent retry, reconciliation, and the success-response ordering before claiming no breaking changes.Also applies to: 67-71
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@proposals/new/gc_perf_enhancement.md` around lines 40 - 43, Define the atomicity and recovery protocol for tag persistence and the digest-based backend PUT in the Harbor proxy flow. Specify operation ordering before returning success, compensating deletion or rollback for partial failures, idempotent retries, and reconciliation for unresolved states; update the claims about consistency and breaking changes to reflect these guarantees.proposals/new/single-active-replication.md-98-117 (1)
98-117: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winUse the actual API paths in the examples.
The repository contract documents
/api/replication/policiesand/api/replication/policies/:id, while these examples omit the/apiprefix. Document the externally routed paths to avoid implementing or testing the wrong endpoints.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@proposals/new/single-active-replication.md` around lines 98 - 117, Update the Create Policy and Update Policy REST examples to use the repository’s externally routed paths: `/api/replication/policies` and `/api/replication/policies/1`, preserving the existing request bodies.proposals/new/single-active-replication.md-56-58 (1)
56-58: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftMake the single-active check atomic.
The proposed check-then-create flow is only best effort: two Core instances can both observe no running execution and then both create active executions. Checking only
runningalso leaves queued/pending executions outside the exclusion. Use an atomic database claim/lock or equivalent coordination mechanism, and explicitly define which execution states count as active.Also applies to: 73-76
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@proposals/new/single-active-replication.md` around lines 56 - 58, Update the single-active replication design to use an atomic database claim/lock or equivalent coordination mechanism around execution creation, preventing concurrent Core instances from both claiming the same policy. Explicitly define the execution states considered active, including queued/pending states as appropriate, and ensure the claim is released when the execution finishes or is cancelled.proposals/new/single-active-replication.md-81-94 (1)
81-94: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winDefine a non-null default for the new column.
ADD COLUMN ... booleanleaves existing policies asNULL, which conflicts with the stated default of unchecked and makes legacy records ambiguous. UseNOT NULL DEFAULT FALSEor specify an explicit backfill and how NULL is handled by the ORM/API.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@proposals/new/single-active-replication.md` around lines 81 - 94, Update the single_active_replication schema migration and Policy model behavior so the new field is non-null and defaults to false for existing and newly created policies. Use a NOT NULL DEFAULT FALSE column definition, ensuring ORM/API handling preserves false rather than allowing NULL.proposals/new/single-active-replication.md-41-44 (1)
41-44: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winClarify whether overlapping executions are skipped or deferred.
Lines 41–44 say a task will wait until the current replication finishes, but lines 73–76 create a record marked
skipped. A skipped execution is permanently dropped, not delayed. Define the intended behavior and document whether a later retry or scheduler run is guaranteed.Also applies to: 73-76
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@proposals/new/single-active-replication.md` around lines 41 - 44, Clarify the Single Active Replication behavior consistently: specify whether overlapping replication tasks are deferred until the active task finishes or permanently skipped when the execution record is marked skipped. Update the descriptions around the option behavior and the skipped-record flow so they state the exact outcome, including whether a later retry or scheduler run is guaranteed.proposals/new/AI-model-processor.md-87-89 (1)
87-89: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winClarify that these are new API endpoints.
The proposal says no new APIs are needed, but then introduces new
/additions/readme,/additions/license, and/additions/filesroutes. Treat these as a new API surface and document route registration, authorization, compatibility, and error behavior, or explain precisely how they extend an existing API.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@proposals/new/AI-model-processor.md` around lines 87 - 89, Update the API section of the proposal to explicitly classify the /additions/readme, /additions/license, and /additions/files routes as new API endpoints, documenting their registration, authorization, compatibility, and error behavior; alternatively, precisely explain how each route extends the existing artifact “addition” API.proposals/new/AI-model-processor.md-261-335 (1)
261-335: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winMake the files-list response a valid, unambiguous JSON contract.
The prose says the endpoint returns a JSON object, but the example is an array and contains trailing commas, so it is not valid JSON. Choose one response shape, remove the trailing commas, and define the semantics of recursive directories and
size.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@proposals/new/AI-model-processor.md` around lines 261 - 335, Update the files-list response contract in the documented endpoint section to choose and consistently describe either an object or array shape, matching the example and prose. Remove all trailing commas so the example is valid JSON, and explicitly define recursive directory representation and the meaning and units of file size for entries returned by the endpoint.proposals/new/AI-model-processor.md-20-22 (1)
20-22: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftDefine the supported Model Spec versions and fallback behavior. The proposal says manifests may differ by spec version, but it never states which versions are supported or how Harbor should handle unknown/unsupported artifacts. Spell out version detection, parser dispatch, and whether parsing fails or degrades for mismatched inputs.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@proposals/new/AI-model-processor.md` around lines 20 - 22, Update the proposal’s AI model metadata parsing section to define the supported Model Spec versions, how the manifest version is detected, and how parsing is dispatched to the appropriate version-specific parser. Specify Harbor’s behavior for unknown or unsupported versions, including whether it rejects parsing or falls back to degraded handling.proposals/new/AI-model-processor.md-178-195 (1)
178-195: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftDefine safe tar extraction semantics. README and LICENSE are described as
*.tarlayers, but the API returns plain text. Specify which archive member is read, how duplicate or missing members are handled, accepted encodings, size limits, and how path-traversal/symlink entries are rejected for untrusted artifacts.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@proposals/new/AI-model-processor.md` around lines 178 - 195, Expand the README API specification to define safe extraction from the identified tar layer: select the exact README member, specify behavior for duplicate or missing members, document accepted text encodings and response size limits, and reject path-traversal or symlink entries when processing untrusted artifacts. Anchor these rules to the README layer characteristics and GET endpoint while preserving the documented status codes.proposals/arm64-support-harbor.md-59-61 (1)
59-61: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftDefine the offline installer architecture contract.
An offline installer cannot resolve a registry manifest tag without access to the registry. Choose and document whether bundles contain both architectures or whether installation detects the host architecture and selects a per-architecture image set.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@proposals/arm64-support-harbor.md` around lines 59 - 61, Clarify the offline installer contract in the architecture and packaging guidance: specify whether each bundle includes both amd64 and arm64 images or whether installation detects the host architecture and selects a per-architecture image set. Ensure the documented approach does not rely on resolving unified registry manifest tags without registry access, and align the offline installer package instructions with that choice.proposals/arm64-support-harbor.md-75-85 (1)
75-85: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftResolve the Trivy Adapter build classification.
The proposal describes the Trivy Adapter both as a binary fetched by
Dockerfile.binaryand as an adapter built from source withGOARCH. These are different build contracts; specify the exact source, build, and artifact flow before implementation.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@proposals/arm64-support-harbor.md` around lines 75 - 85, Clarify the Trivy Adapter entry in the Binary Dependencies section by choosing and documenting one build contract: identify whether the adapter binary is fetched from an upstream release or compiled from source, then specify its source location, GOARCH-aware build step, and resulting artifact flow through Dockerfile.binary. Remove or revise the conflicting classification so the proposal presents a single unambiguous flow.proposals/new/ui-e2e-migrate-playwright.md-275-283 (1)
275-283: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winMake sequential execution effective inside the container.
The Docker commands do not pass
CI, soprocess.env.CImay be unset andworkersbecomes Playwright’s default rather than1. That reintroduces the Docker-daemon and port-collision problems this section says it prevents. Pass-e CI=true, or invoke--workers=1explicitly.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@proposals/new/ui-e2e-migrate-playwright.md` around lines 275 - 283, Update the documented Docker test commands in the Playwright migration configuration so containerized runs explicitly enforce sequential execution, either by passing CI=true into the container or by adding Playwright’s --workers=1 option. Keep the existing playwright.config.ts CI-based behavior unchanged for other environments.proposals/new/ui-e2e-migrate-playwright.md-52-55 (1)
52-55: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winRemove hardcoded credentials from the image and examples.
COSIGN_PASSWORD, the NSS database password, and Harbor passwords are committed asHarbor12345; the first two are also baked into image layers. This contradicts the credential-safety claim at Lines 288-290. Use CI secret injection or ephemeral test credentials, and never publish literal passwords in runnable commands.Also applies to: 109-112, 127-133, 151-159
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@proposals/new/ui-e2e-migrate-playwright.md` around lines 52 - 55, Remove all literal Harbor12345 credentials from the Dockerfile and examples, including COSIGN_PASSWORD, NSS database, and Harbor password values. Update the affected commands and environment configuration to consume CI-injected secrets or generate ephemeral test credentials at runtime, ensuring no password is baked into image layers or shown in runnable examples.proposals/new/ui-e2e-migrate-playwright.md-114-115 (1)
114-115: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftClarify the Docker isolation model.
Mounting
/var/run/docker.sockuses the host Docker daemon and gives the container effective control over it;VOLUME /var/lib/dockerdoes not create Docker-in-Docker. Either document this trust boundary and remove the misleading volume, or run a genuinely isolated daemon in a dedicated test runner.Also applies to: 127-128
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@proposals/new/ui-e2e-migrate-playwright.md` around lines 114 - 115, Clarify the Docker isolation model in the Docker setup: remove the misleading `/var/lib/docker` volume unless a genuinely isolated Docker daemon is started, and document that mounting `/var/run/docker.sock` uses the host daemon and grants the container effective control over it. Apply the same correction to the corresponding setup at the other referenced location.proposals/new/ui-e2e-migrate-playwright.md-288-300 (1)
288-300: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftAdd artifact redaction and retention controls.
Environment-variable injection and GitHub masking do not make screenshots, videos, traces, or network artifacts safe by themselves. Since this proposal explicitly captures browser and request/response evidence, define redaction, access control, retention, and cleanup requirements before enabling these artifacts in CI. (playwright.dev)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@proposals/new/ui-e2e-migrate-playwright.md` around lines 288 - 300, Expand the Credential Safety section around the Playwright defineConfig artifact settings to specify redaction of sensitive data, restricted access to captured artifacts, explicit retention limits, and CI cleanup requirements. Ensure these controls apply to screenshots, videos, traces, and network/request-response evidence before enabling artifact capture, rather than relying only on environment variables and GitHub masking.proposals/new/ui-e2e-migrate-playwright.md-30-30 (1)
30-30: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winInstall or explicitly exclude every CLI required by the proposal.
The Dockerfile installs
docker,curl, andjq, but nothelm,cosign,notation, ororas; the commentedCOPYdoes not provide them either. Any migrated test invoking those tools will fail inside the image.Also applies to: 90-97
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@proposals/new/ui-e2e-migrate-playwright.md` at line 30, Update the proposed container image setup to install every CLI required by the migration—helm, cosign, notation, and oras in addition to docker, curl, and jq—or explicitly remove those tools from the proposal and any dependent tests. Ensure the active Dockerfile commands, rather than the commented COPY, provide all CLIs that migrated tests invoke.proposals/new/ui-e2e-migrate-playwright.md-80-83 (1)
80-83: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winUse HTTPS and a scoped keyring for the Chrome repo Remove
--no-check-certificate, avoid the plain-HTTP Google repo, and import the signing key into a dedicated keyring withsigned-byinstead ofapt-key.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@proposals/new/ui-e2e-migrate-playwright.md` around lines 80 - 83, Update the Chrome repository setup command to remove wget’s --no-check-certificate, fetch the signing key over HTTPS into a dedicated keyring, and configure the repository URL with HTTPS plus the matching signed-by option. Remove the apt-key usage and ensure the keyring directory and file are created as needed before apt-get update.proposals/new/ui-e2e-migrate-playwright.md-46-46 (1)
46-46: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftUse a supported, pinned toolchain.
ubuntu:20.04andNode 20are outside Playwright’s current support matrix (Ubuntu 22.04+ and Node 22/24/26), so this image will be off-matrix; switch to a supported base image and pin npm/Playwright versions instead ofnpm@latestto keep CI reproducible.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@proposals/new/ui-e2e-migrate-playwright.md` at line 46, Update the proposed Playwright toolchain around the Ubuntu base image and Node setup to use a currently supported Ubuntu release and supported Node version, then replace npm@latest and any unpinned Playwright dependency with explicitly pinned versions. Keep the resulting image and package versions reproducible and within Playwright’s support matrix.proposals/new/valkey.md-105-108 (1)
105-108: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftGate in-place volume reuse on a versioned migration procedure.
The proposal promises an upgrade that reuses the existing data volume before identifying the bundled Redis version and target Valkey version. Valkey documents compatible persistence formats for Redis OSS
<=7.2, but Redis CE7.4+data files are not compatible; physical migration also requires quiescing writers, taking a snapshot, and validating the restored dataset. (valkey.io)Make this conditional on an explicit source/target version matrix, backup and rollback steps, and verification of both cache data and job queues. Otherwise remove “reusing the existing data volume” from the supported Phase 1 upgrade path.
Suggested wording
-- In-place upgrade from a prior Harbor release that used internal `redis-photon`, reusing the existing data volume. +- In-place upgrade only for explicitly validated Redis/Valkey version pairs, after backup, writer quiescence, and restore verification; otherwise use the documented logical migration or cache reset path.Also applies to: 187-194
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@proposals/new/valkey.md` around lines 105 - 108, Revise the Phase 1 upgrade claims in the proposal to make existing data-volume reuse conditional on an explicit source/target Redis and Valkey version matrix, including compatibility boundaries. Add required writer quiescence, backup/snapshot, restore validation, rollback, and verification steps for cache data and job queues; otherwise remove the in-place volume reuse scenario from the supported upgrade paths.Source: MCP tools
proposals/new/valkey.md-58-66 (1)
58-66: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftProvide a transition plan for the image rename.
Changing
goharbor/redis-photontogoharbor/valkey-photonbreaks exact-image references in air-gapped bundles, mirrors, custom compose files, admission policies, and deployment automation, even though Harbor’s logical configuration keys remain namedredis. Publish a temporary compatibility tag/alias or explicitly document this as an operational breaking change with a preflight and migration procedure.Also applies to: 80-86
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@proposals/new/valkey.md` around lines 58 - 66, Add a transition plan to the image-rename proposal covering exact-image references in air-gapped bundles, mirrors, custom compose files, admission policies, and deployment automation. Specify either a temporary compatibility alias from goharbor/redis-photon to goharbor/valkey-photon or clearly document the rename as an operational breaking change, including preflight checks and migration steps while preserving Harbor configuration keys named redis.
🟡 Minor comments (6)
proposals/new/filter_repository_proxycache.md-91-95 (1)
91-95: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winCorrect the doublestar example.
^library/**is not a valid equivalent of the stated doublestar pattern: the leading^is treated literally and can cause every repository to be rejected. Uselibrary/**, or change the kind toregexand use^library/.*$.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@proposals/new/filter_repository_proxycache.md` around lines 91 - 95, Correct the proxy_cache_filter example by removing the leading ^ while keeping proxy_cache_filter_kind set to doublestar, so the pattern is library/**. Do not use regex syntax unless also changing the filter kind to regex.proposals/new/gc_perf_enhancement.md-125-129 (1)
125-129: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winFix the typo in the future-work list.
Change “Code impletation bases on the proposed mentioned above” to a grammatically correct sentence such as “Implement the code based on the proposal above.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@proposals/new/gc_perf_enhancement.md` around lines 125 - 129, Correct the first item under the “Future Work” section by replacing the misspelled, ungrammatical wording with a clear sentence such as “Implement the code based on the proposal above.”Source: Linters/SAST tools
proposals/new/single-active-replication.md-9-9 (1)
9-9: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winFix the proposal wording and typos.
Correct
adds an optiontoadds an option, useNon-Goals, capitalize the sentence beginning withif, and fixshceduled-replicationtoscheduled-replication.Also applies to: 33-33, 58-58, 113-113
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@proposals/new/single-active-replication.md` at line 9, Correct the proposal wording and typos: change “adds an option,” rename “Non-goals” to “Non-Goals,” capitalize the sentence beginning with “if,” and replace every occurrence of “shceduled-replication” with “scheduled-replication,” including the referenced sections.Source: Linters/SAST tools
proposals/new/single-active-replication.md-53-60 (1)
53-60: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUse proposed, not completed, terminology.
This PR is a design proposal with no implementation changes, but this section says features were “Added” and “Implemented.” Rename the section to “Proposed Changes” and use future tense to avoid implying that the UI, schema, and runtime behavior already exist.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@proposals/new/single-active-replication.md` around lines 53 - 60, Rename the “Changes Made” section to “Proposed Changes” and rewrite its bullets in future tense, including the UI checkbox, best-effort execution check, single-active behavior, policy model flag, and database column, so the proposal does not imply these changes are implemented.proposals/arm64-support-harbor.md-111-117 (1)
111-117: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winFix the matrix example syntax.
arch:amd64andarch:arm64need a space after:; otherwise theincludeentries won’t definearchas intended.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@proposals/arm64-support-harbor.md` around lines 111 - 117, Correct the strategy matrix entries by adding a space after the colon in both arch mappings, changing the include values to valid YAML key-value syntax while preserving their existing amd64 and arm64 values.proposals/new/ui-e2e-migrate-playwright.md-135-140 (1)
135-140: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPersist the HTML report volume.
This command mounts
/app/test-resultsbut runs the HTML reporter, whose default output is/app/playwright-report; the report will disappear when the container exits. Mount/app/playwright-reportor setPLAYWRIGHT_HTML_OUTPUT_DIRto the mounted directory. (playwright.dev)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@proposals/new/ui-e2e-migrate-playwright.md` around lines 135 - 140, Update the “Run tests with HTML report” Docker command so the Playwright HTML report output at /app/playwright-report persists after the container exits, either by mounting that path to a host directory or by setting PLAYWRIGHT_HTML_OUTPUT_DIR to the existing mounted test-results directory.
🧹 Nitpick comments (2)
proposals/new/valkey.md (2)
70-76: 🩺 Stability & Availability | 🔵 Trivial | 🏗️ Heavy liftExpand the runtime acceptance matrix beyond installation success.
The compatibility claim depends on more than preserving
redis.confpaths. Test the actual entrypoint, health checks, password authentication, persistence/restart behavior, shutdown signals, CLI references, and Harbor jobservice queue processing with the pinned Valkey image.Also applies to: 101-108
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@proposals/new/valkey.md` around lines 70 - 76, Expand the phase 1 runtime acceptance criteria to test the pinned valkey-photon image beyond installation: verify the entrypoint, health checks, password authentication, persistence across restart, shutdown signal handling, CLI references, and Harbor jobservice queue processing. Add these scenarios to the acceptance matrix while preserving the documented compatibility expectations and image/process naming.
23-27: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winScope the licensing rationale to Harbor’s exact Redis version.
The proposal should distinguish Redis OSS
<=7.2(BSD-3-Clause) from Redis7.4/8.xlicensing; current Redis also offers AGPLv3 from Redis 8 onward. Tie the decision to the exact version Harbor currently bundles and the target licensing requirements, rather than describing the upstream runtime generically. (redis.io)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@proposals/new/valkey.md` around lines 23 - 27, The licensing rationale in the Valkey migration proposal should identify the exact Redis version Harbor currently bundles and scope the license comparison to that version. Distinguish Redis OSS ≤7.2 under BSD-3-Clause from Redis 7.4 and 8.x licensing, including AGPLv3 availability from Redis 8 onward, and connect the migration decision to Harbor’s target licensing requirements rather than describing Redis generically.Source: MCP tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Major comments:
In `@proposals/arm64-support-harbor.md`:
- Around line 59-61: Clarify the offline installer contract in the architecture
and packaging guidance: specify whether each bundle includes both amd64 and
arm64 images or whether installation detects the host architecture and selects a
per-architecture image set. Ensure the documented approach does not rely on
resolving unified registry manifest tags without registry access, and align the
offline installer package instructions with that choice.
- Around line 75-85: Clarify the Trivy Adapter entry in the Binary Dependencies
section by choosing and documenting one build contract: identify whether the
adapter binary is fetched from an upstream release or compiled from source, then
specify its source location, GOARCH-aware build step, and resulting artifact
flow through Dockerfile.binary. Remove or revise the conflicting classification
so the proposal presents a single unambiguous flow.
In `@proposals/new/AI-model-processor.md`:
- Around line 87-89: Update the API section of the proposal to explicitly
classify the /additions/readme, /additions/license, and /additions/files routes
as new API endpoints, documenting their registration, authorization,
compatibility, and error behavior; alternatively, precisely explain how each
route extends the existing artifact “addition” API.
- Around line 261-335: Update the files-list response contract in the documented
endpoint section to choose and consistently describe either an object or array
shape, matching the example and prose. Remove all trailing commas so the example
is valid JSON, and explicitly define recursive directory representation and the
meaning and units of file size for entries returned by the endpoint.
- Around line 20-22: Update the proposal’s AI model metadata parsing section to
define the supported Model Spec versions, how the manifest version is detected,
and how parsing is dispatched to the appropriate version-specific parser.
Specify Harbor’s behavior for unknown or unsupported versions, including whether
it rejects parsing or falls back to degraded handling.
- Around line 178-195: Expand the README API specification to define safe
extraction from the identified tar layer: select the exact README member,
specify behavior for duplicate or missing members, document accepted text
encodings and response size limits, and reject path-traversal or symlink entries
when processing untrusted artifacts. Anchor these rules to the README layer
characteristics and GET endpoint while preserving the documented status codes.
In `@proposals/new/fix-proxy-cache-auth-path-prefix.md`:
- Line 1: Update the proposal’s auth probe guidance to derive the probe path
prefix from the configured registry URL/endpoint, rather than splitting at the
first “/v2/” or hardcoding “GET /v2/”. Apply this consistently to the registry
auth probe and probe-based auth dispatch sections so proxy-backed registries
with URL path prefixes are addressed.
- Around line 49-63: Update the auth probe URL construction in the authorizer
flow to derive the registry path prefix from the configured endpoint rather than
splitting the outgoing request path at the first “/v2/” occurrence. Preserve
prefixes containing “/v2/”, such as “/artifactory/v2/docker-virtual”, and add a
regression test covering that configured endpoint and the resulting probe URL.
In `@proposals/new/gc_perf_enhancement.md`:
- Around line 40-43: Define the atomicity and recovery protocol for tag
persistence and the digest-based backend PUT in the Harbor proxy flow. Specify
operation ordering before returning success, compensating deletion or rollback
for partial failures, idempotent retries, and reconciliation for unresolved
states; update the claims about consistency and breaking changes to reflect
these guarantees.
In `@proposals/new/proxy_cache_referer_api.md`:
- Line 87: Update the accessory migration/backfill and merge logic around the
source field so existing records are not all classified as "local". Preserve an
"unknown" source when provenance cannot be determined, derive the actual source
where available, and ensure upstream/local merging does not duplicate records or
rely on the incorrect backfill assumption; apply the same correction to the
related logic at the referenced merge section.
- Around line 104-111: Implement ProxyReferrerMiddleware pagination over the
complete merged referrer set rather than appending local referrers only to the
upstream’s final page. After retrieving upstream or cached referrers, merge and
deduplicate them with local referrers, apply the requested ordering and
pagination to the combined result, and generate Link metadata from the merged
pagination state.
- Around line 134-138: Update the artifact_accessory ALTER TABLE statement so IF
NOT EXISTS follows ADD COLUMN, preserving the source column definition, default
value, and NOT NULL constraint.
- Around line 113-127: Update the default RegistryClient.ListReferrers
implementation to expose the upstream HTTP status, either by returning it
explicitly or by wrapping failures in the established typed status-bearing
error; preserve successful index and header results while allowing callers to
distinguish 404 from other failures for passthrough/fallback handling.
In `@proposals/new/single-active-replication.md`:
- Around line 98-117: Update the Create Policy and Update Policy REST examples
to use the repository’s externally routed paths: `/api/replication/policies` and
`/api/replication/policies/1`, preserving the existing request bodies.
- Around line 56-58: Update the single-active replication design to use an
atomic database claim/lock or equivalent coordination mechanism around execution
creation, preventing concurrent Core instances from both claiming the same
policy. Explicitly define the execution states considered active, including
queued/pending states as appropriate, and ensure the claim is released when the
execution finishes or is cancelled.
- Around line 81-94: Update the single_active_replication schema migration and
Policy model behavior so the new field is non-null and defaults to false for
existing and newly created policies. Use a NOT NULL DEFAULT FALSE column
definition, ensuring ORM/API handling preserves false rather than allowing NULL.
- Around line 41-44: Clarify the Single Active Replication behavior
consistently: specify whether overlapping replication tasks are deferred until
the active task finishes or permanently skipped when the execution record is
marked skipped. Update the descriptions around the option behavior and the
skipped-record flow so they state the exact outcome, including whether a later
retry or scheduler run is guaranteed.
In `@proposals/new/ui-e2e-migrate-playwright.md`:
- Around line 275-283: Update the documented Docker test commands in the
Playwright migration configuration so containerized runs explicitly enforce
sequential execution, either by passing CI=true into the container or by adding
Playwright’s --workers=1 option. Keep the existing playwright.config.ts CI-based
behavior unchanged for other environments.
- Around line 52-55: Remove all literal Harbor12345 credentials from the
Dockerfile and examples, including COSIGN_PASSWORD, NSS database, and Harbor
password values. Update the affected commands and environment configuration to
consume CI-injected secrets or generate ephemeral test credentials at runtime,
ensuring no password is baked into image layers or shown in runnable examples.
- Around line 114-115: Clarify the Docker isolation model in the Docker setup:
remove the misleading `/var/lib/docker` volume unless a genuinely isolated
Docker daemon is started, and document that mounting `/var/run/docker.sock` uses
the host daemon and grants the container effective control over it. Apply the
same correction to the corresponding setup at the other referenced location.
- Around line 288-300: Expand the Credential Safety section around the
Playwright defineConfig artifact settings to specify redaction of sensitive
data, restricted access to captured artifacts, explicit retention limits, and CI
cleanup requirements. Ensure these controls apply to screenshots, videos,
traces, and network/request-response evidence before enabling artifact capture,
rather than relying only on environment variables and GitHub masking.
- Line 30: Update the proposed container image setup to install every CLI
required by the migration—helm, cosign, notation, and oras in addition to
docker, curl, and jq—or explicitly remove those tools from the proposal and any
dependent tests. Ensure the active Dockerfile commands, rather than the
commented COPY, provide all CLIs that migrated tests invoke.
- Around line 80-83: Update the Chrome repository setup command to remove wget’s
--no-check-certificate, fetch the signing key over HTTPS into a dedicated
keyring, and configure the repository URL with HTTPS plus the matching signed-by
option. Remove the apt-key usage and ensure the keyring directory and file are
created as needed before apt-get update.
- Line 46: Update the proposed Playwright toolchain around the Ubuntu base image
and Node setup to use a currently supported Ubuntu release and supported Node
version, then replace npm@latest and any unpinned Playwright dependency with
explicitly pinned versions. Keep the resulting image and package versions
reproducible and within Playwright’s support matrix.
In `@proposals/new/valkey.md`:
- Around line 105-108: Revise the Phase 1 upgrade claims in the proposal to make
existing data-volume reuse conditional on an explicit source/target Redis and
Valkey version matrix, including compatibility boundaries. Add required writer
quiescence, backup/snapshot, restore validation, rollback, and verification
steps for cache data and job queues; otherwise remove the in-place volume reuse
scenario from the supported upgrade paths.
- Around line 58-66: Add a transition plan to the image-rename proposal covering
exact-image references in air-gapped bundles, mirrors, custom compose files,
admission policies, and deployment automation. Specify either a temporary
compatibility alias from goharbor/redis-photon to goharbor/valkey-photon or
clearly document the rename as an operational breaking change, including
preflight checks and migration steps while preserving Harbor configuration keys
named redis.
In `@proposals/probe-based-auth-dispatch.md`:
- Around line 111-122: Update the Registry Proxy Auth Probe request path to
derive and preserve the configured registry path prefix, issuing the probe
against <configured-prefix>/v2/ (or the equivalent URL-path derivation) instead
of always using /v2/. Keep the existing Bearer, Basic, and anonymous response
handling unchanged.
- Around line 104-109: Update the token-caching guidance in the “Registry Proxy
User Credential Passthrough” section and its corresponding section to bind
cached tokens to the authenticated principal, repository scope, and requested
actions. Ensure pull, push, and delete permissions cannot share a cache entry
across users or action scopes, or explicitly document why such sharing is safe.
- Around line 54-62: Clarify the dispatch section by explicitly listing the
backend precedence used by the dispatcher. Define fallthrough so only
recoverable or unavailable backend errors continue to the next backend, while
invalid or rejected credentials terminate dispatch and return the authentication
failure; preserve fatal Match() errors as immediate failures.
---
Minor comments:
In `@proposals/arm64-support-harbor.md`:
- Around line 111-117: Correct the strategy matrix entries by adding a space
after the colon in both arch mappings, changing the include values to valid YAML
key-value syntax while preserving their existing amd64 and arm64 values.
In `@proposals/new/filter_repository_proxycache.md`:
- Around line 91-95: Correct the proxy_cache_filter example by removing the
leading ^ while keeping proxy_cache_filter_kind set to doublestar, so the
pattern is library/**. Do not use regex syntax unless also changing the filter
kind to regex.
In `@proposals/new/gc_perf_enhancement.md`:
- Around line 125-129: Correct the first item under the “Future Work” section by
replacing the misspelled, ungrammatical wording with a clear sentence such as
“Implement the code based on the proposal above.”
In `@proposals/new/single-active-replication.md`:
- Line 9: Correct the proposal wording and typos: change “adds an option,”
rename “Non-goals” to “Non-Goals,” capitalize the sentence beginning with “if,”
and replace every occurrence of “shceduled-replication” with
“scheduled-replication,” including the referenced sections.
- Around line 53-60: Rename the “Changes Made” section to “Proposed Changes” and
rewrite its bullets in future tense, including the UI checkbox, best-effort
execution check, single-active behavior, policy model flag, and database column,
so the proposal does not imply these changes are implemented.
In `@proposals/new/ui-e2e-migrate-playwright.md`:
- Around line 135-140: Update the “Run tests with HTML report” Docker command so
the Playwright HTML report output at /app/playwright-report persists after the
container exits, either by mounting that path to a host directory or by setting
PLAYWRIGHT_HTML_OUTPUT_DIR to the existing mounted test-results directory.
---
Nitpick comments:
In `@proposals/new/valkey.md`:
- Around line 70-76: Expand the phase 1 runtime acceptance criteria to test the
pinned valkey-photon image beyond installation: verify the entrypoint, health
checks, password authentication, persistence across restart, shutdown signal
handling, CLI references, and Harbor jobservice queue processing. Add these
scenarios to the acceptance matrix while preserving the documented compatibility
expectations and image/process naming.
- Around line 23-27: The licensing rationale in the Valkey migration proposal
should identify the exact Redis version Harbor currently bundles and scope the
license comparison to that version. Distinguish Redis OSS ≤7.2 under
BSD-3-Clause from Redis 7.4 and 8.x licensing, including AGPLv3 availability
from Redis 8 onward, and connect the migration decision to Harbor’s target
licensing requirements rather than describing Redis generically.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: dc686b9e-fed6-4259-acf4-80c1d32f5ae3
⛔ Files ignored due to path filters (18)
presentations/deep_dive/Harbor Maintainer Talk - KubeCon EU 2026.pdfis excluded by!**/*.pdfpresentations/introduction/Harbor Lightning Talk - KubeCon + CloudNativeCon Europe 2026.pdfis excluded by!**/*.pdfpresentations/introduction/Harbor_ KubeCon INDIA 2025 Lightning Talk.pdfis excluded by!**/*.pdfproposals/images/ai-model/ai-model-processor.pngis excluded by!**/*.pngproposals/images/ai-model/file-open.pngis excluded by!**/*.pngproposals/images/ai-model/files.pngis excluded by!**/*.pngproposals/images/ai-model/license.pngis excluded by!**/*.pngproposals/images/ai-model/readme.pngis excluded by!**/*.pngproposals/images/ai-model/summary.pngis excluded by!**/*.pngproposals/images/arm64/harbor-multiarch-arm64-amd64.pngis excluded by!**/*.pngproposals/images/gc-perf/gc_after.pngis excluded by!**/*.pngproposals/images/gc-perf/gc_pervious.pngis excluded by!**/*.pngproposals/images/gc-perf/gc_tag_del_checkbox.pngis excluded by!**/*.pngproposals/images/gc-perf/push_with_digest.pngis excluded by!**/*.pngproposals/images/gc-perf/push_with_digest_flow.pngis excluded by!**/*.pngproposals/images/gc-perf/push_with_tag.pngis excluded by!**/*.pngproposals/images/gc-perf/push_with_tag_flow.pngis excluded by!**/*.pngproposals/new/images/filter_repository_proxycache_ui.pngis excluded by!**/*.png
📒 Files selected for processing (12)
ADOPTERS.mdMAINTAINERS.mdproposals/arm64-support-harbor.mdproposals/new/AI-model-processor.mdproposals/new/filter_repository_proxycache.mdproposals/new/fix-proxy-cache-auth-path-prefix.mdproposals/new/gc_perf_enhancement.mdproposals/new/proxy_cache_referer_api.mdproposals/new/single-active-replication.mdproposals/new/ui-e2e-migrate-playwright.mdproposals/new/valkey.mdproposals/probe-based-auth-dispatch.md
01b4059 to
94adb6d
Compare
94adb6d to
9adf15c
Compare
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
…uthenticated request handling, action scoping, and /v2 probe Signed-off-by: Ross Golder <ross@golder.org>
51bb288 to
232aa74
Compare
1 participant
Summary
Proposes replacing Harbor's single-backend
auth_modeconfig switch with per-backendMatch()probing, so the dispatcher tries backends in priority order instead of trusting one exclusive mode string.This was originally proposed directly as code upstream on goharbor/harbor (#23458, recreated as #23483), which was closed without merge for being too large and lacking prior design discussion. This proposal retargets the same idea at
container-registry/harbor-next, where the implementation has since been split into four independent, reviewable PRs:Opening as a draft for early feedback before requesting formal review.
Test plan
N/A — design proposal document only, no code changes.
Summary by cubic
Adds the Probe-Based Authentication Dispatch proposal to replace Harbor’s single
auth_modewith per-backendMatch()probing and ordered fallback, preventing lockouts and enabling flexible multi-backend auth. The doc also covers fixes for locked-user handling, correct/v2Basic vs Bearer challenges, and per-user registry proxy credential passthrough with per-scope token caching, with links to the split PRs incontainer-registry/harbor-next./v2/) and tightened action scoping.Written for commit 232aa74. Summary will update on new commits.