Skip to content

Conversation

@bupd
Copy link
Collaborator

@bupd bupd commented Aug 28, 2025

Describe whats different in next

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.

Summary by cubic

Add Dagger-based pipelines and a local dev stack to build, run, and publish Harbor faster. Also adds Single Active Replication, a replication adapter whitelist, CVE status in APIs/UI, CNAI raw doc support, and several stability/CI upgrades.

  • New Features

    • Dagger: local dev services (Postgres, Redis, Registry/Ctl, Core, Jobservice, Portal, Nginx) and a CI workflow to build/publish; docs in .dagger/README.md.
    • Replication: Single Active Replication per policy (backend, DB, UI toggle). Skips a run if one is already active.
    • Replication: adapter whitelist via REPLICATION_ADAPTER_WHITELIST; proxy-cache list aligned.
    • Vulnerabilities: persist and expose CVE status; show in Security Hub and artifact details. Return existing reports when the scanner is unhealthy. Don’t skip checks solely because an artifact isn’t scannable.
    • CNAI: support raw doc layers and auto-detect tar/raw in parsers.
    • Misc: prefer local manifest when present; improved ORM filters; jobservice sets MAX_JOB_DURATION_SECONDS from config; swagger adds 400/409 on select routes; Go 1.24.6 and Actions v5 upgrades.
  • Migration

    • DB: new columns vulnerability_record.status and replication_policy.single_active_replication; migration 0170_2.14.0 runs with the standard upgrade.
    • Config: harbor.yml version bumped to 2.14.0. Optionally set REPLICATION_ADAPTER_WHITELIST to enable hidden/inactive adapters.
    • Local dev (optional): go run ./.dagger/scripts/parseMakefile.go, then dagger call run-dev up -v.

Summary by CodeRabbit

  • New Features

    • Vulnerability “Status” added to API and displayed in UI (Security Hub and artifact vulnerabilities), with a new status filter.
    • Replication: “Single active replication” option to prevent overlapping runs; UI support and “Skipped” status shown when applicable.
    • Local development via Dagger with configs and README to run Harbor services.
  • Enhancements

    • Graceful handling when scanners are unhealthy.
    • CNAI README/LICENSE parsing supports raw blobs.
    • Registry adapter whitelist configurable.
  • Bug Fixes

    • Cleanup project metadata on retention policy deletion; improved repository queries and permission/error handling.
  • Chores

    • Go upgraded to 1.24.6; CI workflows refreshed; base image refreshed.
  • Documentation

    • Portal README and release matrix updated.

yuzp1996 and others added 30 commits July 14, 2025 14:26
Set Fix and CVE3Score in VulnerabilityRecord from VulnerabilityItem.

Follow-up of goharbor#21915
Fixes goharbor#21463

Signed-off-by: Spyros Trigazis <[email protected]>
This commit adds the field "status" to the struct of a vulnerability and adds
column "status" to vulnerability record table.  It makes sure the statuses
of CVEs returned by trivy scanner are persisted and can be returned via
the vulnerabilities addition API of an artifact.

Signed-off-by: Daniel Jiang <[email protected]>
improving the portal readme file

Signed-off-by: rgcr <[email protected]>
…rbor#22145)

Updated the Minor Release Support Matrix to include v2.13

Signed-off-by: Mooneeb Hussain <[email protected]>
This commit makes change to the API GET /api/v2.0/vul to make it include
"status" of CVEs in the response.

It also makes update in the UI to add the "Status" column to the data
grids in security hub and artifact details page.

Signed-off-by: Daniel Jiang <[email protected]>
…rbor#22113)

---
updated-dependencies:
- dependency-name: helm.sh/helm/v3
  dependency-version: 3.18.3
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: miner <[email protected]>
…rbor#22188)

---
updated-dependencies:
- dependency-name: helm.sh/helm/v3
  dependency-version: 3.18.4
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…#22187)

fixes goharbor#22143

This commit makes update to the vulnerable policy middleware.  So that
it will skip the sheck only when the artifact is not scannable AND it
does not have a scan report.

Signed-off-by: Daniel Jiang <[email protected]>
fixes goharbor#21925

According to https://github.com/goharbor/harbor/wiki/Harbor-Replicaiton-Adapter-Owner, some replication adapters are no longer actively maintained by the Harbor community. To address this, a whitelist environment variable is introduced to define the list of actively supported adapters, which will be used by the Harbor portal and API to display and allow usage.

If you still wish to view and use the unsupported or inactive adapters, you must manually update the whitelist and include the desired adapter names. For the list of adapter names, refer to https://github.com/goharbor/harbor/blob/main/src/pkg/reg/model/registry.go#L22

Signed-off-by: wang yan <[email protected]>
to extend the enhancement from goharbor#21924 to fuzzy and range match. After the enhance, the orm.ExerSep is not supported in any sort of query keywords.

Signed-off-by: wy65701436 <[email protected]>
to the latest golang version v1.24.5 from v1.24.3

Signed-off-by: wy65701436 <[email protected]>
… logic (goharbor#22233)

remove extra build_base=false && pull_base_from_dockerhub=false logic
we do not block the case using local chached image(docker build --pull=false) while build_base=false
However we need gurantee always pull latest image while build package
And when there's some rate limit issue in the CICD situation we could set pull_base_from_dockerhub=false

Signed-off-by: my036811 <[email protected]>
… to 1.1.25 in /src (goharbor#22225)

chore(deps): bump github.com/volcengine/volcengine-go-sdk in /src

Bumps [github.com/volcengine/volcengine-go-sdk](https://github.com/volcengine/volcengine-go-sdk) from 1.1.19 to 1.1.25.
- [Release notes](https://github.com/volcengine/volcengine-go-sdk/releases)
- [Commits](volcengine/volcengine-go-sdk@v1.1.19...v1.1.25)

---
updated-dependencies:
- dependency-name: github.com/volcengine/volcengine-go-sdk
  dependency-version: 1.1.25
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: miner <[email protected]>
According to PR goharbor#22198, some inactive adapters have been removed from the default adapter list.
This pull request updates PERMITTED_REGISTRY_TYPES_FOR_PROXY_CACHE accordingly, since the list of proxy cache adapters should be a subset of the replication adapters.

Signed-off-by: wy65701436 <[email protected]>
feat(replication): add Single Active Replication per policy

* Added single_active_replication field to schema & DB table
* Updated API, controller & handler to enforce single execution per policy
* Added checkbox in UI to enable/disable single_active_replication for a
  policy
* Implemented necessary backend & frontend tests
* Prevents parallel runs per policy if single_active_replication is enabled

Signed-off-by: bupd <[email protected]>
1, downgrade the base version to 2.14, and update it whenever we have the release-2.14.0 branch.
2, refresh the base images bses on the latest code.

Signed-off-by: wy65701436 <[email protected]>
Upload log files to github with 5 day retention

Signed-off-by: stonezdj <[email protected]>
dependabot bot and others added 15 commits August 15, 2025 14:52
…rbor#22258)

Bumps [helm.sh/helm/v3](https://github.com/helm/helm) from 3.18.4 to 3.18.5.
- [Release notes](https://github.com/helm/helm/releases)
- [Commits](helm/helm@v3.18.4...v3.18.5)

---
updated-dependencies:
- dependency-name: helm.sh/helm/v3
  dependency-version: 3.18.5
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [actions/checkout](https://github.com/actions/checkout) from 3 to 5.
- [Release notes](https://github.com/actions/checkout/releases)
- [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md)
- [Commits](actions/checkout@v3...v5)

---
updated-dependencies:
- dependency-name: actions/checkout
  dependency-version: '5'
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
… to 1.1.26 in /src (goharbor#22244)

chore(deps): bump github.com/volcengine/volcengine-go-sdk in /src

Bumps [github.com/volcengine/volcengine-go-sdk](https://github.com/volcengine/volcengine-go-sdk) from 1.1.25 to 1.1.26.
- [Release notes](https://github.com/volcengine/volcengine-go-sdk/releases)
- [Commits](volcengine/volcengine-go-sdk@v1.1.25...v1.1.26)

---
updated-dependencies:
- dependency-name: github.com/volcengine/volcengine-go-sdk
  dependency-version: 1.1.26
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
This commit fixes goharbor#22254
It updates the "GetReport" function, such that when the scanner is
unhealthy, and we can't know the the artifact is supported, we will
still try to return the report stored in DB.

Signed-off-by: Daniel Jiang <[email protected]>
use exists instead of in condition

Signed-off-by: stonezdj <[email protected]>
…rc (goharbor#22282)

Bumps [github.com/beego/beego/v2](https://github.com/beego/beego) from 2.3.6 to 2.3.8.
- [Release notes](https://github.com/beego/beego/releases)
- [Changelog](https://github.com/beego/beego/blob/master/CHANGELOG.md)
- [Commits](beego/beego@v2.3.6...v2.3.8)

---
updated-dependencies:
- dependency-name: github.com/beego/beego/v2
  dependency-version: 2.3.8
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…6.2 in /src (goharbor#22283)

chore(deps): bump github.com/prometheus/client_model in /src

Bumps [github.com/prometheus/client_model](https://github.com/prometheus/client_model) from 0.6.1 to 0.6.2.
- [Release notes](https://github.com/prometheus/client_model/releases)
- [Commits](prometheus/client_model@v0.6.1...v0.6.2)

---
updated-dependencies:
- dependency-name: github.com/prometheus/client_model
  dependency-version: 0.6.2
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…6.1 in /src (goharbor#22281)

chore(deps): bump github.com/cloudevents/sdk-go/v2 in /src

Bumps [github.com/cloudevents/sdk-go/v2](https://github.com/cloudevents/sdk-go) from 2.15.2 to 2.16.1.
- [Release notes](https://github.com/cloudevents/sdk-go/releases)
- [Commits](cloudevents/sdk-go@v2.15.2...v2.16.1)

---
updated-dependencies:
- dependency-name: github.com/cloudevents/sdk-go/v2
  dependency-version: 2.16.1
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
… /src (goharbor#22243)

chore(deps): bump github.com/go-openapi/swag in /src

Bumps [github.com/go-openapi/swag](https://github.com/go-openapi/swag) from 0.23.0 to 0.23.1.
- [Commits](go-openapi/swag@v0.23.0...v0.23.1)

---
updated-dependencies:
- dependency-name: github.com/go-openapi/swag
  dependency-version: 0.23.1
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
… to 1.1.29 in /src (goharbor#22284)

chore(deps): bump github.com/volcengine/volcengine-go-sdk in /src

Bumps [github.com/volcengine/volcengine-go-sdk](https://github.com/volcengine/volcengine-go-sdk) from 1.1.26 to 1.1.29.
- [Release notes](https://github.com/volcengine/volcengine-go-sdk/releases)
- [Commits](volcengine/volcengine-go-sdk@v1.1.26...v1.1.29)

---
updated-dependencies:
- dependency-name: github.com/volcengine/volcengine-go-sdk
  dependency-version: 1.1.29
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Describe whats different in next

Signed-off-by: Vadim Bauer <[email protected]>
Signed-off-by: Vadim Bauer <[email protected]>
Signed-off-by: bupd <[email protected]>
@coderabbitai
Copy link

coderabbitai bot commented Aug 28, 2025

📝 Walkthrough

Walkthrough

Adds a Dagger-based local dev/build/publish/sign toolchain; introduces dev configs and scripts. Updates CI workflows and Go/tool versions. Extends API and UI to include vulnerability status and single-active replication. Adds registry adapter whitelist. Enhances parsers, middleware, controllers, migrations, and SQL. Includes Redis Sentinel support patch.

Changes

Cohort / File(s) Change summary
Dagger toolchain and configs
dagger.json, .dagger/go.mod, .dagger/main.go, .dagger/dev.go, .dagger/devServices.go, .dagger/tests.go, .dagger/debug.go, .dagger/consts.go, .dagger/scripts/parseMakefile.go, .dagger/config/*, .dagger/README.md, .dagger/.gitattributes, .dagger/.gitignore
Introduces Dagger module, services orchestration, build/publish/sign pipelines, lint/vuln checks, generated consts, dev configs (env, nginx, registry, jobservice), and scripts for env/debug. Adds repo attributes and ignores, plus README.
CI workflows and logs
.github/workflows/*, tests/ci/api_run.sh
Upgrades actions/checkout to v5; adds artifact upload of integration logs; adds Dagger publish workflow; adjusts CI log packaging.
Version bumps and build base
.buildbaselog, CONTRIBUTING.md, Makefile, src/go.mod, make/harbor.yml.tmpl
Updates Go base image/version to 1.24.6, logs base refresh; bumps dependencies; updates migrator template version to 2.14.0.
Migrations and prepare tooling
make/migrations/postgresql/0170_2.14.0_schema.up.sql, make/photon/prepare/**
Adds DB columns (vulnerability_record.status, replication_policy.single_active_replication). Adds 2.14.0 migration module/template; updates accepted versions and defaults; removes max_job_duration_seconds handling in prepare.
API spec and models (vulnerability status)
api/v2.0/swagger.yaml, src/pkg/scan/vuln/report.go, src/pkg/scan/dao/scan/model.go, src/pkg/scan/postprocessors/report_converters.go, src/pkg/securityhub/dao/security.go, src/server/v2.0/handler/security.go
Adds vulnerability status field across API, models, converters, DAO, and responses; extends filters and error responses; adds new query params.
UI updates (status + replication)
src/portal/src/app/**/*vulnerability*.*, src/portal/src/app/shared/services/interface.ts, src/portal/src/app/base/left-side-nav/replication/**, src/portal/README.md, src/portal/src/i18n/lang/*.json
Shows vulnerability status columns; adds status to interfaces and filters. Adds single_active_replication UI control, wiring, and i18n. Updates replication status rendering for skipped case. Updates portal README.
Replication: single-active execution
src/controller/replication/execution.go, src/controller/replication/execution_test.go, src/controller/replication/model/model.go, src/pkg/replication/model/model.go, src/server/v2.0/handler/replication.go, tests/apitests/python/test_system_permission.py
Adds policy flag to prevent parallel identical replications; counts running executions; marks new exec as skipped via error status; propagates field through models, API handlers, tests.
Registry adapter whitelist
src/common/const.go, src/lib/config/metadata/metadatalist.go, src/controller/registry/controller.go, src/controller/registry/controller_test.go, make/photon/prepare/templates/core/env.jinja
Adds system config/env key for replication adapter whitelist; filters listed adapters by whitelist; adjusts templates; adds tests.
CNIA parser: raw content support
src/controller/artifact/processor/cnai/parser/*
Adds decoding by media-type extension (.tar/.raw); accepts raw for README/LICENSE; updates tests.
Proxy controller cleanup
src/controller/proxy/controller.go
On missing remote manifest, fetches local manifest digest and asynchronously deletes local manifest if found.
Vulnerability middleware behavior
src/server/middleware/vulnerable/vulnerable.go, src/server/middleware/vulnerable/vulnerable_test.go
Moves scannable check into NotFound path; skips check if not scannable and no report; updates tests and adds scenarios.
Audit log permission refactor
src/server/v2.0/handler/auditlog.go, src/server/v2.0/handler/base.go, src/server/v2.0/handler/project.go
Centralizes auth/query building; changes HasProjectPermission to return (bool, error); updates call sites with error handling.
ORM/query and DAO tweaks
src/lib/orm/query.go, src/pkg/repository/dao/dao.go
Adds operator whitelist to query parsing; early-continue after custom filter; switches NonEmptyRepos condition to EXISTS.
Jobservice config/env seconds
src/jobservice/config/config.go, .dagger/config/jobservice/*, make/photon/prepare/templates/jobservice/env.jinja, make/photon/prepare/utils/*
Ensures MAX_JOB_DURATION_SECONDS env is set on Load; adds dev jobservice configs; removes seconds handling from prepare templates/util.
Redis Sentinel support (registry patch)
.dagger/registry/redis.patch
Patches registry to support Redis Sentinel; adds vendor code, config field, and tests in patch context.
Retention metadata cleanup
src/server/v2.0/handler/retention.go
Deletes project metadata key retention_id after retention policy deletion.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Dev as Developer
  participant Dagger as Dagger Pipeline
  participant Harbor as Harbor (Dagger)
  participant Svc as Services

  Dev->>Dagger: dagger call run-dev
  Dagger->>Harbor: DevServer(ctx)
  Harbor->>Svc: Start Postgres ("postgresql")
  Harbor->>Svc: Start Redis ("redis")
  Harbor->>Svc: Start Registry ("registry")
  Harbor->>Svc: Start RegistryCtl ("registryctl")
  Harbor->>Svc: Start Core ("core")
  Harbor->>Svc: Start JobService ("jobservice")
  Harbor->>Svc: Start Portal ("portal")
  Harbor->>Svc: Start Nginx/Proxy ("proxy")
  Harbor-->>Dagger: proxy service
  Dagger-->>Dev: Running dev stack on ports
Loading
sequenceDiagram
  autonumber
  participant API as Replication API
  participant Ctl as Replication Controller
  participant Store as Execution Store
  participant Flow as FlowCtl

  API->>Ctl: Start(policy)
  alt policy.single_active_replication == true
    Ctl->>Store: Count running executions
    Store-->>Ctl: n
  end
  Ctl->>Store: Create execution
  Store-->>Ctl: execID
  alt single-active and n>0
    Ctl->>Store: MarkError(execID, "Execution skipped: active replication...")
    Ctl-->>API: execID
  else
    Ctl->>Flow: Start(execID) (async)
    Ctl-->>API: execID
  end
Loading
sequenceDiagram
  autonumber
  participant MW as Vulnerable Middleware
  participant Scan as Vulnerability Service
  participant Checker as ScanChecker

  MW->>Scan: GetVulnerable(artifact)
  alt NotFound error
    MW->>Checker: IsScannable(artifact)
    alt not scannable
      MW-->>MW: Skip check
      MW-->>Client: allow
    else scannable
      MW-->>Client: PROJECTPOLICYVIOLATION
    end
  else other error
    MW-->>Client: error
  else found
    MW-->>Client: evaluate policy
  end
Loading
sequenceDiagram
  autonumber
  participant API as AuditLog API
  participant Helper as checkPermissionAndBuildQuery
  participant Proj as ProjectCtl
  participant RBAC as HasProjectPermission

  API->>Helper: Build query (qs, sort, page)
  Helper->>Proj: List accessible projects
  loop projects
    Helper->>RBAC: HasProjectPermission(project, Read)
    alt error
      Helper-->>API: error
    else allowed
      Helper-->>Helper: add ProjectID to OrList
    end
  end
  Helper-->>API: q.Query
  API->>Store: Count/List(q)
  API-->>Client: results
Loading

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120+ minutes

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.2.2)

level=warning msg="[runner] Can't run linter goanalysis_metalinter: inspect: failed to load package handler: could not load export data: no export data for "github.com/goharbor/harbor/src/server/v2.0/handler""
level=error msg="Running error: can't run linter goanalysis_metalinter\ninspect: failed to load package handler: could not load export data: no export data for "github.com/goharbor/harbor/src/server/v2.0/handler""

Tip

🔌 Remote MCP (Model Context Protocol) integration is now available!

Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats.

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch dagger

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbit in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbit in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbit gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbit read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbit help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbit ignore or @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbit summary or @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbit or @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Signed-off-by: bupd <[email protected]>
@sonarqubecloud
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
7 Security Hotspots
14.1% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud

Comment on lines +21 to +65
runs-on: ubuntu-latest
outputs:
VERSIONTAG: ${{ steps.vars.outputs.VERSIONTAG }}
DEBUGTAG: ${{ steps.vars.outputs.DEBUGTAG }}
REGISTRY_USER: ${{ steps.vars.outputs.REGISTRY_USER }}
steps:
- name: Checkout
uses: actions/checkout@v4
with:
fetch-depth: 1

- name: Dagger Version
uses: sagikazarmark/[email protected]

- name: Set Variables
id: vars
run: |
if [ "${{ github.event_name }}" = "pull_request" ]; then
# use PR number as version tag
VERSIONTAG="pr-${{ github.event.number }}"
else
VERSIONTAG=$(echo "${GITHUB_REF_NAME}" | tr '/' '-')
fi
DEBUGTAG=$VERSIONTAG-debug
echo "VERSIONTAG=$VERSIONTAG" >> $GITHUB_ENV
echo "DEBUGTAG=$DEBUGTAG" >> $GITHUB_ENV
# docker auth
mkdir -p $HOME/.docker
echo '${{ secrets.DOCKER_AUTH_CONFIG }}' > $HOME/.docker/config.json
REGISTRY_USER=$(jq -r '.auths["${{ env.REGISTRY_DOMAIN }}"].username' $HOME/.docker/config.json)
echo "REGISTRY_USER=$REGISTRY_USER" >> $GITHUB_ENV
# Export registry password
REGISTRY_PASS=$(jq -r '.auths["${{ env.REGISTRY_DOMAIN }}"].password' $HOME/.docker/config.json)
echo "REGISTRY_PASS=$REGISTRY_PASS" >> $GITHUB_ENV
- name: Verify Dagger
uses: dagger/dagger-for-github@v7
with:
version: ${{ steps.dagger_version.outputs.version }}
verb: functions

publish:

Check warning

Code scanning / CodeQL

Workflow does not contain permissions Medium

Actions job or workflow does not limit the permissions of the GITHUB_TOKEN. Consider setting an explicit permissions block, using the following as a minimal starting point: {contents: read}

Copilot Autofix

AI 2 months ago

To resolve the issue, you should add a permissions block to the workflow file .github/workflows/default.yml. Since there is no indication that the jobs require write permissions to repository contents or pull requests, start with the most restrictive set of permissions:
At the root level, add:

permissions:
  contents: read

This will apply to all jobs unless overridden by a job-level permissions block. Place this block near the top of the workflow, right after the name: definition and before the on: block (the earliest possible place it can appear).

No other imports, methods, or definitions are needed, as this is a documentation/configuration change for GitHub Actions.


Suggested changeset 1
.github/workflows/default.yml

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/.github/workflows/default.yml b/.github/workflows/default.yml
--- a/.github/workflows/default.yml
+++ b/.github/workflows/default.yml
@@ -1,4 +1,6 @@
 name: Dagger Build and Publish
+permissions:
+  contents: read
 
 on:
   push:
EOF
@@ -1,4 +1,6 @@
name: Dagger Build and Publish
permissions:
contents: read

on:
push:
Copilot is powered by AI and may make mistakes. Always verify output.
Comment on lines +66 to +144
needs: setup
runs-on: ubuntu-latest
strategy:
matrix:
include:
- component: core
debug: false
- component: core
debug: true
- component: jobservice
debug: false
- component: jobservice
debug: true
- component: registryctl
debug: false
- component: registryctl
debug: true
- component: registry
debug: false
- component: portal
debug: false
- component: nginx
debug: false
- component: trivy-adapter
debug: false
- component: cmd/exporter
debug: false
steps:
- uses: actions/checkout@v4

- name: Set component vars
id: comp
run: |
if [ "${{ matrix.debug }}" = "true" ]; then
export FINAL_IMAGE_TAG="${{ needs.setup.outputs.DEBUGTAG }}"
else
export FINAL_IMAGE_TAG="${{ needs.setup.outputs.VERSIONTAG }}"
fi
echo "FINAL_IMAGE_TAG=$FINAL_IMAGE_TAG" >> $GITHUB_ENV
- name: Verify Dagger
uses: dagger/dagger-for-github@v7
with:
version: latest
verb: functions

- name: Publish and Sign
id: publish
uses: dagger/dagger-for-github@v7
with:
version: latest
verb: call
args: |
publish-and-sign-image \
--pkg ${{ matrix.component }} \
--registry-username=${{ env.REGISTRY_USER }} \
--registry=${{ env.REGISTRY_DOMAIN }} \
--registry-password="${{ env.REGISTRY_PASS }}" \
--image-tags ${{ steps.comp.outputs.FINAL_IMAGE_TAG }} \
--project-name=${{ env.PROJECT_NAME }} \
--debugbin=${{ matrix.debug }}
--github-token=env:GITHUB_TOKEN \
--actions-id-token-request-url=$ACTIONS_ID_TOKEN_REQUEST_URL \
--actions-id-token-request-token=env:ACTIONS_ID_TOKEN_REQUEST_TOKEN
- name: Validate Image Ref
run: |
OUTPUT="${{ steps.publish.outputs.stdout }}"
echo "--- Full Dagger Output ---"
echo "$OUTPUT"
echo "--------------------------"
if echo "$OUTPUT" | grep -qE "^${{ env.REGISTRY_DOMAIN }}/${{ env.PROJECT_NAME }}/[a-z0-9./-]+:${{ steps.comp.outputs.FINAL_IMAGE_TAG }}@sha256:[a-f0-9]{64}$"; then
echo "✅ Success: Published image reference for '${{ matrix.component }}' is valid."
else
echo "❌ Error: Invalid image ref for '${{ matrix.component }}'"
exit 1
fi

Check warning

Code scanning / CodeQL

Workflow does not contain permissions Medium

Actions job or workflow does not limit the permissions of the GITHUB_TOKEN. Consider setting an explicit permissions block, using the following as a minimal starting point: {contents: read}

Copilot Autofix

AI 2 months ago

The recommended fix is to add a permissions key at the top level of the workflow file (.github/workflows/default.yml) to enforce the least privilege principle. By setting permissions: contents: read, you restrict the generated GITHUB_TOKEN to only have read access to the repository contents, which is sufficient for many common workflow tasks. This block should be inserted before the jobs: section, preferably right after the name: and on: blocks and before env:. If any job or step eventually requires additional permissions (like pull-requests: write), such permissions can be granted specifically at the job level.


Suggested changeset 1
.github/workflows/default.yml

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/.github/workflows/default.yml b/.github/workflows/default.yml
--- a/.github/workflows/default.yml
+++ b/.github/workflows/default.yml
@@ -10,6 +10,9 @@
       - "*.md"
       - "assets/**"
 
+permissions:
+  contents: read
+
 env:
   GIT_DEPTH: 1
   REGISTRY_DOMAIN: registry.goharbor.io
EOF
@@ -10,6 +10,9 @@
- "*.md"
- "assets/**"

permissions:
contents: read

env:
GIT_DEPTH: 1
REGISTRY_DOMAIN: registry.goharbor.io
Copilot is powered by AI and may make mistakes. Always verify output.
Copy link

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

40 issues found across 112 files

React with 👍 or 👎 to teach cubic. You can also tag @cubic-dev-ai to give feedback, ask questions, or re-run the review.

--debugbin=${{ matrix.debug }}
--github-token=env:GITHUB_TOKEN \
--actions-id-token-request-url=$ACTIONS_ID_TOKEN_REQUEST_URL \
--actions-id-token-request-token=env:ACTIONS_ID_TOKEN_REQUEST_TOKEN"
Copy link

Choose a reason for hiding this comment

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

Remove the stray trailing quote to avoid a syntax error in the arguments.

Prompt for AI agents
Address the following comment on .github/workflows/default.yml at line 129:

<comment>Remove the stray trailing quote to avoid a syntax error in the arguments.</comment>

<file context>
@@ -0,0 +1,144 @@
+              --debugbin=${{ matrix.debug }}
+              --github-token=env:GITHUB_TOKEN \
+              --actions-id-token-request-url=$ACTIONS_ID_TOKEN_REQUEST_URL \
+              --actions-id-token-request-token=env:ACTIONS_ID_TOKEN_REQUEST_TOKEN&quot;
+
+      - name: Validate Image Ref
</file context>


# Execute the command (passed as $1 arg)
echo "Executing: $1"
eval "$1"
Copy link

Choose a reason for hiding this comment

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

Using eval on the command string is unsafe and unnecessary; prefer exec with sh -c (or exec "$@" with argv) to avoid double interpretation and improve signal handling.

Prompt for AI agents
Address the following comment on .dagger/config/run_env.sh at line 27:

<comment>Using eval on the command string is unsafe and unnecessary; prefer exec with sh -c (or exec &quot;$@&quot; with argv) to avoid double interpretation and improve signal handling.</comment>

<file context>
@@ -0,0 +1,27 @@
+
+# Execute the command (passed as $1 arg)
+echo &quot;Executing: $1&quot;
+eval &quot;$1&quot;
</file context>

version := m.GetVersion(ctx)
postgres := dag.Container().From("goharbor/harbor-db:"+version).
WithExposedPort(5432).
WithEnvVariable("POSTGRES_PASSWORD", "root123").
Copy link

Choose a reason for hiding this comment

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

Hard-coded weak database password for Postgres; use a secret or configuration instead.

Prompt for AI agents
Address the following comment on .dagger/devServices.go at line 101:

<comment>Hard-coded weak database password for Postgres; use a secret or configuration instead.</comment>

<file context>
@@ -0,0 +1,125 @@
+	version := m.GetVersion(ctx)
+	postgres := dag.Container().From(&quot;goharbor/harbor-db:&quot;+version).
+		WithExposedPort(5432).
+		WithEnvVariable(&quot;POSTGRES_PASSWORD&quot;, &quot;root123&quot;).
+		AsService()
+	return postgres
</file context>


# Start the dlv process in the background
# /root/go/bin/dlv exec --headless --listen localhost:$2 $1
dlv --headless=true --listen=0.0.0.0:4001 --accept-multiclient --log-output=debugger,debuglineerr,gdbwire,lldbout,rpc --log=true --continue --api-version=2 exec /core
Copy link

Choose a reason for hiding this comment

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

dlv binds to 0.0.0.0:4001 and ignores provided port/command args, exposing the debug port and reducing configurability.

Prompt for AI agents
Address the following comment on .dagger/config/run_debug.sh at line 36:

<comment>dlv binds to 0.0.0.0:4001 and ignores provided port/command args, exposing the debug port and reducing configurability.</comment>

<file context>
@@ -0,0 +1,37 @@
+
+# Start the dlv process in the background
+# /root/go/bin/dlv exec --headless --listen localhost:$2 $1
+dlv --headless=true --listen=0.0.0.0:4001 --accept-multiclient --log-output=debugger,debuglineerr,gdbwire,lldbout,rpc --log=true --continue --api-version=2 exec /core
+# /root/go/bin/dlv --headless=true --listen=localhost:4001 --accept-multiclient --log-output=debugger,debuglineerr,gdbwire,lldbout,rpc --log=true --api-version=2 attach $pid
</file context>

WithExec([]string{
"cosign", "sign", "--yes", "--recursive",
"--registry-username", registryUsername,
"--registry-password", registryPasswordPlain,
Copy link

Choose a reason for hiding this comment

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

Registry password is passed as a CLI argument, risking secret leakage via process args; use a secret env or credential helper instead.

Prompt for AI agents
Address the following comment on .dagger/main.go at line 155:

<comment>Registry password is passed as a CLI argument, risking secret leakage via process args; use a secret env or credential helper instead.</comment>

<file context>
@@ -0,0 +1,774 @@
+		WithExec([]string{
+			&quot;cosign&quot;, &quot;sign&quot;, &quot;--yes&quot;, &quot;--recursive&quot;,
+			&quot;--registry-username&quot;, registryUsername,
+			&quot;--registry-password&quot;, registryPasswordPlain,
+			imageAddr,
+			&quot;--timeout&quot;, &quot;1m&quot;,
</file context>

)

func (m *Harbor) NginxService(ctx context.Context) *dagger.Service {
nginxConfig := m.OnlyDagger.File("./.dagger/config/proxy/nginx.conf")
Copy link

Choose a reason for hiding this comment

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

Portal service mounts proxy nginx.conf instead of portal/nginx.conf, likely causing misrouting.

Prompt for AI agents
Address the following comment on .dagger/devServices.go at line 9:

<comment>Portal service mounts proxy nginx.conf instead of portal/nginx.conf, likely causing misrouting.</comment>

<file context>
@@ -0,0 +1,125 @@
+)
+
+func (m *Harbor) NginxService(ctx context.Context) *dagger.Service {
+	nginxConfig := m.OnlyDagger.File(&quot;./.dagger/config/proxy/nginx.conf&quot;)
+
+	nginxSrv := m.BuildImage(ctx, DEV_PLATFORM, &quot;nginx&quot;, true).
</file context>
Suggested change
nginxConfig := m.OnlyDagger.File("./.dagger/config/proxy/nginx.conf")
nginxConfig := m.OnlyDagger.File("./.dagger/config/portal/nginx.conf")

}
// delete retention data in project_metadata
if err := r.proMetaMgr.Delete(ctx, p.Scope.Reference, "retention_id"); err != nil {
return r.SendError(ctx, err)
Copy link

Choose a reason for hiding this comment

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

API may return an error after successfully deleting the retention policy if metadata deletion fails, causing partial state and client-visible failure; consider making metadata removal best-effort or transactional with the policy delete.

Prompt for AI agents
Address the following comment on src/server/v2.0/handler/retention.go at line 265:

<comment>API may return an error after successfully deleting the retention policy if metadata deletion fails, causing partial state and client-visible failure; consider making metadata removal best-effort or transactional with the policy delete.</comment>

<file context>
@@ -260,6 +260,10 @@ func (r *retentionAPI) DeleteRetention(ctx context.Context, params operation.Del
 	}
+	// delete retention data in project_metadata
+	if err := r.proMetaMgr.Delete(ctx, p.Scope.Reference, &quot;retention_id&quot;); err != nil {
+		return r.SendError(ctx, err)
+	}
 	return operation.NewDeleteRetentionOK()
</file context>

// WithExec([]string{"bun", "pm", "trust", "--all"}).
WithFile("swagger.yaml", swaggerYaml).
// WithExec([]string{"apt", "update"}).
WithExec([]string{"apt", "install", "unzip"}).
Copy link

Choose a reason for hiding this comment

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

apt install is used without updating package lists and non-interactive -y; this may fail or hang in CI.

Prompt for AI agents
Address the following comment on .dagger/main.go at line 661:

<comment>apt install is used without updating package lists and non-interactive -y; this may fail or hang in CI.</comment>

<file context>
@@ -0,0 +1,774 @@
+		// WithExec([]string{&quot;bun&quot;, &quot;pm&quot;, &quot;trust&quot;, &quot;--all&quot;}).
+		WithFile(&quot;swagger.yaml&quot;, swaggerYaml).
+		// WithExec([]string{&quot;apt&quot;, &quot;update&quot;}).
+		WithExec([]string{&quot;apt&quot;, &quot;install&quot;, &quot;unzip&quot;}).
+		WithExec([]string{&quot;npm&quot;, &quot;install&quot;, &quot;-g&quot;, &quot;bun@&quot; + BUN_VERSION}).
+		WithExec([]string{&quot;bun&quot;, &quot;install&quot;}).
</file context>

registryPassword *dagger.Secret,
imageAddr string,
) (string, error) {
registryPasswordPlain, _ := registryPassword.Plaintext(ctx)
Copy link

Choose a reason for hiding this comment

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

Error from registryPassword.Plaintext(ctx) is ignored; handle and return a clear error.

Prompt for AI agents
Address the following comment on .dagger/main.go at line 135:

<comment>Error from registryPassword.Plaintext(ctx) is ignored; handle and return a clear error.</comment>

<file context>
@@ -0,0 +1,774 @@
+	registryPassword *dagger.Secret,
+	imageAddr string,
+) (string, error) {
+	registryPasswordPlain, _ := registryPassword.Plaintext(ctx)
+
+	cosing_ctr := dag.Container().From(&quot;cgr.dev/chainguard/cosign&quot;)
</file context>

go-version: 1.23.2
id: go
- uses: actions/checkout@v3
- uses: actions/checkout@v5
Copy link

Choose a reason for hiding this comment

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

GitHub Action is not pinned to a specific commit SHA, increasing supply chain risk. Pin to a trusted commit for reproducible and secure builds.

Prompt for AI agents
Address the following comment on .github/workflows/conformance_test.yml at line 31:

<comment>GitHub Action is not pinned to a specific commit SHA, increasing supply chain risk. Pin to a trusted commit for reproducible and secure builds.</comment>

<file context>
@@ -28,7 +28,7 @@ jobs:
           go-version: 1.23.2
         id: go
-      - uses: actions/checkout@v3
+      - uses: actions/checkout@v5
         with:
           path: src/github.com/goharbor/harbor
</file context>

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 58

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (10)
.github/workflows/build-package.yml (1)

24-28: Align Go version in workflows with go.mod
Update all go-version entries in .github/workflows/*.yml from 1.23.2 to 1.24.6 to match the go 1.24.6 directive in src/go.mod.

.github/workflows/conformance_test.yml (1)

26-31: Update workflows’ Go version to match go.mod
Change every go-version: 1.23.2 in .github/workflows/*.yml to 1.24.6, since src/go.mod specifies go 1.24.6.

src/pkg/securityhub/dao/security.go (1)

86-96: Duplicate column in SELECT breaks row mapping.

vr.package is selected twice; this will misalign scans into VulnerabilityItem.

Apply this diff:

-	vulnerabilitySQL = `select  vr.cve_id, vr.cvss_score_v3, vr.package, a.repository_name, a.id artifact_id, a.digest, vr.package, vr.package_version, vr.severity, vr.status, vr.fixed_version, vr.description, vr.urls, a.project_id
+	vulnerabilitySQL = `select  vr.cve_id, vr.cvss_score_v3, vr.package, a.repository_name, a.id artifact_id, a.digest, vr.package_version, vr.severity, vr.status, vr.fixed_version, vr.description, vr.urls, a.project_id
src/controller/replication/model/model.go (1)

127-147: Add missing DB migration for single_active_replication column
The Policy struct in src/pkg/replication/model/model.go includes SingleActiveReplication bool orm:"column(single_active_replication)", but no SQL migration in make/photon/prepare/migrationsadds this column. Create a new 2.14.0 migration to alterreplication_policyand addsingle_active_replication BOOL NOT NULL DEFAULT FALSE`.

.dagger/config/run_debug.sh (1)

1-38: Fix POSIX string indexing and undefined pid

  • Replace ${key:0:1} with a POSIX-compliant check (e.g.
    case "$key" in
      ''|\#*) continue ;;
    esac
    ) to ignore empty lines or comments (SC3057).
  • Either capture and export the child process’s PID before echoing it, or remove the $pid placeholder since it’s never assigned (SC2154).
src/portal/src/i18n/lang/de-de-lang.json (1)

1105-1118: Missing de-DE translation for VULNERABILITY.GRID.COLUMN_STATUS

Add the “Status” column key to align with en-US.

         "GRID": {
             "PLACEHOLDER": "Es wurden keine Scan-Ergebnisse gefunden!",
             "COLUMN_ID": "Schwachstellen",
             "COLUMN_SEVERITY": "Schweregrad",
+            "COLUMN_STATUS": "Status",
             "COLUMN_PACKAGE": "Paket",
src/server/v2.0/handler/project.go (2)

409-419: Data race: concurrent writes to summary without synchronization.

Multiple goroutines mutate the shared summary struct fields. Either run sequentially (cheap, few funcs) or guard with a mutex.

Apply this diff to run sequentially:

-    var wg sync.WaitGroup
-    for _, fn := range fetchSummaries {
-        fn := fn
-
-        wg.Add(1)
-        go func() {
-            defer wg.Done()
-            fn(ctx, p, summary)
-        }()
-    }
-    wg.Wait()
+    for _, fn := range fetchSummaries {
+        fn(ctx, p, summary)
+    }

796-800: Typo in error message (“geater” → “greater”).

User-facing text; fix the spelling.

Apply this diff:

-            return errors.BadRequestError(fmt.Errorf("%d is invalid value of registry_id, it should be geater than 0", *req.RegistryID))
+            return errors.BadRequestError(fmt.Errorf("%d is invalid value of registry_id, it should be greater than 0", *req.RegistryID))
src/server/v2.0/handler/base.go (1)

84-101: Fix: not-found path currently leaks into permission check with projectID=0

On not-found you set p = &project.Project{} and then proceed, causing resource namespace 0 checks instead of returning (false, nil). Return early on IsNotFoundErr and avoid setting a dummy project.

Apply:

-    p, err := baseProjectCtl.GetByName(ctx, projectName)
-    if err != nil {
-        log.Errorf("failed to get project %s: %v", projectName, err)
-        if errors.IsNotFoundErr(err) {
-            p = &project.Project{}
-        } else {
-            return false, err
-        }
-    }
-    if p == nil {
-        log.Warningf("project %s not found", projectName)
-        return false, nil
-    }
-    projectID = p.ProjectID
+    p, err := baseProjectCtl.GetByName(ctx, projectName)
+    if err != nil {
+        if errors.IsNotFoundErr(err) {
+            log.Warningf("project %s not found", projectName)
+            return false, nil
+        }
+        log.Errorf("failed to get project %s: %v", projectName, err)
+        return false, err
+    }
+    projectID = p.ProjectID
src/server/v2.0/handler/replication.go (1)

132-151: Update: validate against existing trigger when Trigger is omitted

If the update omits Trigger, we must validate SingleActiveReplication against the current policy’s trigger; otherwise event_based + single_active can slip through.

Apply:

 func (r *replicationAPI) UpdateReplicationPolicy(ctx context.Context, params operation.UpdateReplicationPolicyParams) middleware.Responder {
@@
-    policy := &repctlmodel.Policy{
+    policy := &repctlmodel.Policy{
         ID:                params.ID,
@@
     if params.Policy.Trigger != nil {
         policy.Trigger = &model.Trigger{
             Type: params.Policy.Trigger.Type,
         }
@@
     }
@@
-    if params.Policy.SingleActiveReplication != nil {
-        // Validate and assign SingleActiveReplication only for non-event_based triggers
-        if params.Policy.Trigger != nil && params.Policy.Trigger.Type == model.TriggerTypeEventBased && *params.Policy.SingleActiveReplication {
-            return r.SendError(ctx, fmt.Errorf("single active replication is not allowed for event_based triggers"))
-        }
-        policy.SingleActiveReplication = *params.Policy.SingleActiveReplication
-    }
+    if params.Policy.SingleActiveReplication != nil {
+        // Determine effective trigger type (new or existing)
+        effTrigger := ""
+        if params.Policy.Trigger != nil {
+            effTrigger = params.Policy.Trigger.Type
+        } else {
+            // fetch existing to validate when trigger not provided
+            existing, err := r.ctl.GetPolicy(ctx, params.ID)
+            if err != nil {
+                return r.SendError(ctx, err)
+            }
+            if existing.Trigger != nil {
+                effTrigger = existing.Trigger.Type
+            }
+        }
+        if effTrigger == model.TriggerTypeEventBased && *params.Policy.SingleActiveReplication {
+            return r.SendError(ctx, errors.BadRequestError(nil).
+                WithMessage("single_active_replication is not allowed for event_based triggers"))
+        }
+        policy.SingleActiveReplication = *params.Policy.SingleActiveReplication
+    }

Also applies to: 171-180, 192-199

Comment on lines +1 to +5
/dagger.gen.go
/internal/dagger
/internal/querybuilder
/internal/telemetry
/.env
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Ignore the real registry passwd to avoid committing secrets.

Add the concrete htpasswd path to this ignore list.

Apply:

 /dagger.gen.go
 /internal/dagger
 /internal/querybuilder
 /internal/telemetry
 /.env
+ /config/registry/passwd
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
/dagger.gen.go
/internal/dagger
/internal/querybuilder
/internal/telemetry
/.env
/dagger.gen.go
/internal/dagger
/internal/querybuilder
/internal/telemetry
/.env
/config/registry/passwd
🤖 Prompt for AI Agents
In .dagger/.gitignore around lines 1-5, the real registry htpasswd file is not
being ignored; add a concrete ignore entry for it. Edit .dagger/.gitignore and
add the line "/.dagger/registry.htpasswd" (or the exact filename you use for the
registry htpasswd) so the real registry password file is not committed.

Comment on lines +1 to +4
CORE_SECRET=D1fWkYKg6OQgISeP
REGISTRY_URL=http://registry:5000
JOBSERVICE_SECRET=gVQJX0AlVjstElN8
CORE_URL=http://core:8080
Copy link

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Remove committed secrets; inject via Dagger/CI secrets and rotate immediately.

CORE_SECRET, JOBSERVICE_SECRET, and REGISTRY_CREDENTIAL_PASSWORD are hardcoded. Even for local dev, this is a leakage vector and will trip gitleaks. Replace with placeholders and source real values from Dagger secrets or your CI secret store. Rotate any environments where these values were used.

Apply this diff to redact and document:

-CORE_SECRET=D1fWkYKg6OQgISeP
+## Inject via Dagger secret or env at runtime (e.g., dagger -e CORE_SECRET=...).
+CORE_SECRET=REPLACE_ME

-JOBSERVICE_SECRET=gVQJX0AlVjstElN8
+## Inject via Dagger secret or env at runtime.
+JOBSERVICE_SECRET=REPLACE_ME
...
-REGISTRY_CREDENTIAL_PASSWORD=TRJUhYbJgSjXZWAj3oLEet3ugJ3nAOk3
+## Inject via Dagger secret or env at runtime.
+REGISTRY_CREDENTIAL_PASSWORD=REPLACE_ME

To confirm no secrets remain:

If you want, I can provide a follow-up patch to:

  • move this file to env.example with placeholders,
  • load real values via .dagger/run_env.sh using dagger.Secret().AsEnvVariable(...).

Also applies to: 10-15


🏁 Script executed:

#!/bin/bash
# Fail on high-entropy strings resembling secrets (quick sweep).
rg -nP '(?i)(secret|passwd|password|token|apikey|key)\s*[:=]\s*[A-Za-z0-9+/=]{16,}' -C2

Length of output: 15970


Remove hardcoded secrets in .dagger/config/jobservice/env
The file still contains live values for CORE_SECRET and JOBSERVICE_SECRET (the previous scan missed hidden files by default). Replace them with placeholders, inject via Dagger/CI secrets at runtime, and rotate any environments where these were used. Update your secret-scan to include hidden files (e.g. rg --hidden -nP …) to confirm no high-entropy values remain. REGISTRY_URL and CORE_URL are service endpoints, not secrets. Consider renaming this to env.example and sourcing real values in your .dagger/run_env.sh.

.dagger/config/jobservice/env
-CORE_SECRET=D1fWkYKg6OQgISeP
+CORE_SECRET=REPLACE_ME
-JOBSERVICE_SECRET=gVQJX0AlVjstElN8
+JOBSERVICE_SECRET=REPLACE_ME
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
CORE_SECRET=D1fWkYKg6OQgISeP
REGISTRY_URL=http://registry:5000
JOBSERVICE_SECRET=gVQJX0AlVjstElN8
CORE_URL=http://core:8080
CORE_SECRET=REPLACE_ME
REGISTRY_URL=http://registry:5000
JOBSERVICE_SECRET=REPLACE_ME
CORE_URL=http://core:8080
🧰 Tools
🪛 Gitleaks (8.27.2)

1-1: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


3-3: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)

🤖 Prompt for AI Agents
.dagger/config/jobservice/env lines 1-4 contain hardcoded secrets (CORE_SECRET
and JOBSERVICE_SECRET); replace those two values with placeholder variables
(e.g. CORE_SECRET_PLACEHOLDER, JOBSERVICE_SECRET_PLACEHOLDER) and remove any
real secret material, ensure REGISTRY_URL and CORE_URL remain as service
endpoints or placeholders if desired; wire the real secrets into the pipeline by
reading them from Dagger/CI secret injection at runtime (update
.dagger/run_env.sh to source injected secrets), rotate any environment
credentials that used the leaked values, rename this file to
.dagger/config/jobservice/env.example (or add a copy) so it no longer stores
real secrets, and update your secret-scan command to include hidden files (for
example use ripgrep with --hidden) to verify no high-entropy values remain.

@@ -0,0 +1 @@
harbor_registry_user:$2y$05$p.Xb0IvvIIjCYGKEyyUwBeuUzKZsPFhWcngVKZLkc95Gurr52RN9u
Copy link

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Do not commit credential hashes; generate at runtime or use a template + gitignore.

Even for dev, committing an htpasswd bcrypt leak can be reused accidentally. Rotate this credential, and switch to a generated file or a checked-in template.

Suggested follow-ups:

  • Rename this to passwd.example and generate the real file at runtime (htpasswd -Bbn "$REGISTRY_CREDENTIAL_USERNAME" "$REGISTRY_CREDENTIAL_PASSWORD" > /etc/registry/passwd), or
  • Add .dagger/config/registry/passwd to .dagger/.gitignore (see separate comment) and keep only the example.

Quick repo scan to ensure the leaked hash/user isn’t referenced elsewhere:


🏁 Script executed:

#!/bin/bash
rg -n -C2 -S $'harbor_registry_user|/etc/registry/passwd|REGISTRY_CREDENTIAL_PASSWORD' .dagger || true

Length of output: 1669


Remove all committed registry credentials and migrate to templated/generate-at-runtime approach.
Plaintext credentials REGISTRY_CREDENTIAL_USERNAME/REGISTRY_CREDENTIAL_PASSWORD in:

  • .dagger/config/registryctl/env
  • .dagger/config/jobservice/env
  • .dagger/config/core/env
    and the bcrypt hash in .dagger/config/registry/passwd must be removed, credentials rotated, and these files replaced with .example templates (e.g. passwd.example) and added to .gitignore. Generate the real credentials at runtime (e.g. htpasswd -Bbn "$REGISTRY_CREDENTIAL_USERNAME" "$REGISTRY_CREDENTIAL_PASSWORD" > /etc/registry/passwd").
🤖 Prompt for AI Agents
In .dagger/config/registry/passwd around lines 1 to 1, a committed bcrypt
password hash and other files (.dagger/config/registryctl/env,
.dagger/config/jobservice/env, .dagger/config/core/env) contain hardcoded
registry credentials; remove the hash and any plaintext credentials, rotate
those secrets out of the repo, and replace each file with a corresponding
.example template (e.g. passwd.example, registryctl.env.example,
jobservice.env.example, core.env.example) that contains only variable
names/placeholders; add the real filenames (passwd and the three env files) to
.gitignore, update CI/dev setup docs to generate credentials at runtime (for
example, using htpasswd to create the passwd file from
REGISTRY_CREDENTIAL_USERNAME and REGISTRY_CREDENTIAL_PASSWORD injected at
runtime), and ensure deployment scripts/containers read credentials from secure
runtime sources (env vars / secret manager) rather than version control.

Comment on lines +1 to +14
CORE_SECRET=D1fWkYKg6OQgISeP
REGISTRY_URL=http://registry:5000
JOBSERVICE_SECRET=gVQJX0AlVjstElN8
CORE_URL=http://core:8080
REGISTRY_CONTROLLER_URL=http://registryctl:8080
JOBSERVICE_WEBHOOK_JOB_MAX_RETRY=3
JOBSERVICE_WEBHOOK_JOB_HTTP_CLIENT_TIMEOUT=3


HTTP_PROXY=
HTTPS_PROXY=
NO_PROXY=db,localhost,redis,log,exporter,postgresql,trivy-adapter,portal,.internal,registry,127.0.0.1,jobservice,nginx,.local,core,registryctl
REGISTRY_CREDENTIAL_USERNAME=harbor_registry_user
REGISTRY_CREDENTIAL_PASSWORD=TRJUhYbJgSjXZWAj3oLEet3ugJ3nAOk3
Copy link

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Hardcoded secrets committed — rotate immediately and replace with placeholders/secrets store.

CORE_SECRET, JOBSERVICE_SECRET, and REGISTRY_CREDENTIAL_PASSWORD appear to be real secrets. This is high risk even for dev. Rotate/revoke now, purge from history, and switch to Dagger/CI secret injection.

Apply this diff to stop committing live secrets (use env expansion with safe defaults), then manage real values via Dagger’s secret APIs or CI secret store:

-CORE_SECRET=D1fWkYKg6OQgISeP
+CORE_SECRET=${CORE_SECRET:-CHANGEME}
 REGISTRY_URL=http://registry:5000
-JOBSERVICE_SECRET=gVQJX0AlVjstElN8
+JOBSERVICE_SECRET=${JOBSERVICE_SECRET:-CHANGEME}
 CORE_URL=http://core:8080
 REGISTRY_CONTROLLER_URL=http://registryctl:8080
 JOBSERVICE_WEBHOOK_JOB_MAX_RETRY=3
 JOBSERVICE_WEBHOOK_JOB_HTTP_CLIENT_TIMEOUT=3

 HTTP_PROXY=
 HTTPS_PROXY=
 NO_PROXY=db,localhost,redis,log,exporter,postgresql,trivy-adapter,portal,.internal,registry,127.0.0.1,jobservice,nginx,.local,core,registryctl
 REGISTRY_CREDENTIAL_USERNAME=harbor_registry_user
-REGISTRY_CREDENTIAL_PASSWORD=TRJUhYbJgSjXZWAj3oLEet3ugJ3nAOk3
+REGISTRY_CREDENTIAL_PASSWORD=${REGISTRY_CREDENTIAL_PASSWORD:-CHANGEME}

Also:

  • Commit an env.template and gitignore the real env file.
  • Rotate the exposed creds in any affected systems.

Run to locate any other committed secrets:


🏁 Script executed:

#!/bin/bash
rg -nP '(?i)(core_secret|jobservice_secret|registry_credential_password)\s*=' -C2

Length of output: 2253


Remove hardcoded secrets from .dagger/config/registryctl/env
Rotate and revoke CORE_SECRET, JOBSERVICE_SECRET, and REGISTRY_CREDENTIAL_PASSWORD immediately; purge them from history. Replace with environment‐variable defaults and manage real values via Dagger’s secret APIs or your CI secret store. Commit an env.template (with these placeholders) and gitignore the real env file.

-CORE_SECRET=D1fWkYKg6OQgISeP
+CORE_SECRET=${CORE_SECRET:-CHANGEME}
 REGISTRY_URL=http://registry:5000
-JOBSERVICE_SECRET=gVQJX0AlVjstElN8
+JOBSERVICE_SECRET=${JOBSERVICE_SECRET:-CHANGEME}
 CORE_URL=http://core:8080
 REGISTRY_CONTROLLER_URL=http://registryctl:8080
 JOBSERVICE_WEBHOOK_JOB_MAX_RETRY=3
 JOBSERVICE_WEBHOOK_JOB_HTTP_CLIENT_TIMEOUT=3

 HTTP_PROXY=
 HTTPS_PROXY=
 NO_PROXY=db,localhost,redis,log,exporter,postgresql,trivy-adapter,portal,.internal,registry,127.0.0.1,jobservice,nginx,.local,core,registryctl
 REGISTRY_CREDENTIAL_USERNAME=harbor_registry_user
-REGISTRY_CREDENTIAL_PASSWORD=TRJUhYbJgSjXZWAj3oLEet3ugJ3nAOk3
+REGISTRY_CREDENTIAL_PASSWORD=${REGISTRY_CREDENTIAL_PASSWORD:-CHANGEME}

Other templates under make/photon/prepare/templates/** already use Jinja placeholders for these values—no additional hardcoded secrets detected.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
CORE_SECRET=D1fWkYKg6OQgISeP
REGISTRY_URL=http://registry:5000
JOBSERVICE_SECRET=gVQJX0AlVjstElN8
CORE_URL=http://core:8080
REGISTRY_CONTROLLER_URL=http://registryctl:8080
JOBSERVICE_WEBHOOK_JOB_MAX_RETRY=3
JOBSERVICE_WEBHOOK_JOB_HTTP_CLIENT_TIMEOUT=3
HTTP_PROXY=
HTTPS_PROXY=
NO_PROXY=db,localhost,redis,log,exporter,postgresql,trivy-adapter,portal,.internal,registry,127.0.0.1,jobservice,nginx,.local,core,registryctl
REGISTRY_CREDENTIAL_USERNAME=harbor_registry_user
REGISTRY_CREDENTIAL_PASSWORD=TRJUhYbJgSjXZWAj3oLEet3ugJ3nAOk3
CORE_SECRET=${CORE_SECRET:-CHANGEME}
REGISTRY_URL=http://registry:5000
JOBSERVICE_SECRET=${JOBSERVICE_SECRET:-CHANGEME}
CORE_URL=http://core:8080
REGISTRY_CONTROLLER_URL=http://registryctl:8080
JOBSERVICE_WEBHOOK_JOB_MAX_RETRY=3
JOBSERVICE_WEBHOOK_JOB_HTTP_CLIENT_TIMEOUT=3
HTTP_PROXY=
HTTPS_PROXY=
NO_PROXY=db,localhost,redis,log,exporter,postgresql,trivy-adapter,portal,.internal,registry,127.0.0.1,jobservice,nginx,.local,core,registryctl
REGISTRY_CREDENTIAL_USERNAME=harbor_registry_user
REGISTRY_CREDENTIAL_PASSWORD=${REGISTRY_CREDENTIAL_PASSWORD:-CHANGEME}
🧰 Tools
🪛 Gitleaks (8.27.2)

1-1: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


3-3: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


14-14: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)

Comment on lines +14 to +16
if [ -z "$key" ] || [ "${key:0:1}" = "#" ]; then
continue
fi
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Bashism in /bin/sh; fix comment detection

${key:0:1} is not POSIX sh. Use a case on the whole key.

-  if [ -z "$key" ] || [ "${key:0:1}" = "#" ]; then
+  case "$key" in
+    ""|\#*) continue ;;
+  esac

Committable suggestion skipped: line range outside the PR's diff.

🧰 Tools
🪛 Shellcheck (0.10.0)

[warning] 14-14: In POSIX sh, string indexing is undefined.

(SC3057)

🤖 Prompt for AI Agents
.dagger/config/run_debug.sh around lines 14-16: the script uses the non-POSIX
parameter expansion ${key:0:1} to detect comments; replace that check with a
POSIX-compliant case statement that matches keys beginning with '#' (or
otherwise test the whole $key for a leading hash) and continue when matched,
ensuring the script remains compatible with /bin/sh.

"ALLOWED_CHARACTERS": "İzin verilen özel karakterler",
"TOTAL": "Toplam",
"OVERRIDE": "Geçersiz Kıl",
"SINGLE_ACTIVE_REPLICATION": "Single active replication",
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Localize the label (currently English).

Use a natural Turkish label.

Apply this diff:

-        "SINGLE_ACTIVE_REPLICATION": "Single active replication",
+        "SINGLE_ACTIVE_REPLICATION": "Tek etkin çoğaltma",
🤖 Prompt for AI Agents
In src/portal/src/i18n/lang/tr-tr-lang.json around line 583, the value for
"SINGLE_ACTIVE_REPLICATION" is still in English; replace the English label with
a natural Turkish label (e.g., "Tek aktif replikasyon") and keep the JSON
formatting intact (quotes around the value and the trailing comma).

"PULL_BASED": "把资源由远端仓库拉取到本地Harbor。",
"DESTINATION_NAMESPACE": "指定目标名称空间。如果不填,资源会被放到和源相同的名称空间下。",
"OVERRIDE": "如果存在具有相同名称的资源,请指定是否覆盖目标上的资源。",
"SINGLE_ACTIVE_REPLICATION": "Specify whether to skip execution until the previous active execution finishes, avoiding the execution of the same replication rules multiple times in parallel.",
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

本地化新增的提示文本(目前为英文)。

请将字符串翻译为中文以保持一致性。

应用如下 diff:

-        "SINGLE_ACTIVE_REPLICATION": "Specify whether to skip execution until the previous active execution finishes, avoiding the execution of the same replication rules multiple times in parallel.",
+        "SINGLE_ACTIVE_REPLICATION": "指定在先前的活动执行完成之前是否跳过执行,以避免同一复制规则并行多次执行。",
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
"SINGLE_ACTIVE_REPLICATION": "Specify whether to skip execution until the previous active execution finishes, avoiding the execution of the same replication rules multiple times in parallel.",
"SINGLE_ACTIVE_REPLICATION": "指定在先前的活动执行完成之前是否跳过执行,以避免同一复制规则并行多次执行。",
🤖 Prompt for AI Agents
In src/portal/src/i18n/lang/zh-cn-lang.json around line 79, the value for
"SINGLE_ACTIVE_REPLICATION" is still in English; replace it with a Chinese
translation. Change the string value to a Chinese sentence such as
"指定是否在上一个正在运行的执行完成之前跳过本次执行,以避免相同的复制规则被并行多次执行。", ensuring proper JSON escaping
and keeping the same key.

"ALLOWED_CHARACTERS": "允许的特殊字符",
"TOTAL": "总数",
"OVERRIDE": "覆盖",
"SINGLE_ACTIVE_REPLICATION": "Single active replication",
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

本地化标签(目前为英文)。

应用如下 diff:

-        "SINGLE_ACTIVE_REPLICATION": "Single active replication",
+        "SINGLE_ACTIVE_REPLICATION": "单个活动复制",
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
"SINGLE_ACTIVE_REPLICATION": "Single active replication",
"SINGLE_ACTIVE_REPLICATION": "单个活动复制",
🤖 Prompt for AI Agents
In src/portal/src/i18n/lang/zh-cn-lang.json around line 580, the translation key
"SINGLE_ACTIVE_REPLICATION" currently has an English value ("Single active
replication"); update this to the proper Chinese localization by replacing the
value with an appropriate Chinese string (e.g., "单活复制" or another agreed
translation), keeping JSON syntax and quotation marks intact and ensuring
trailing commas remain correct for the file context.

Comment on lines +99 to +101
msg := fmt.Sprintf(`current image without vulnerability scanning cannot be pulled due to configured policy in 'Prevent images with vulnerability severity of "%s" or higher from running.' `+
`To continue with pull, please contact your project administrator for help.`, projectSeverity)
return errors.New(nil).WithCode(errors.PROJECTPOLICYVIOLATION).WithMessage(msg)
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Bug: errors.New(nil) likely wrong type; pass message to New instead.

errors.New in this repo accepts a string (see NotFoundError usage). Passing nil risks compile-time failure and loses the message set via WithMessage.

Apply:

-			return errors.New(nil).WithCode(errors.PROJECTPOLICYVIOLATION).WithMessage(msg)
+			return errors.New(msg).WithCode(errors.PROJECTPOLICYVIOLATION)

And similarly below:

-			return errors.New(nil).WithCode(errors.PROJECTPOLICYVIOLATION).WithMessage(msg)
+			return errors.New(msg).WithCode(errors.PROJECTPOLICYVIOLATION)
-			return errors.New(nil).WithCode(errors.PROJECTPOLICYVIOLATION).WithMessage(msg)
+			return errors.New(msg).WithCode(errors.PROJECTPOLICYVIOLATION)

Also applies to: 117-121, 129-133

🤖 Prompt for AI Agents
In src/server/middleware/vulnerable/vulnerable.go around lines 99-101, 117-121,
and 129-133, the code calls errors.New(nil).WithCode(...).WithMessage(msg) which
passes nil to errors.New; change these to pass the message into errors.New
directly (e.g., errors.New(msg).WithCode(...)) so the error is constructed with
the message instead of using nil and then calling WithMessage.

Comment on lines +263 to +266
// delete retention data in project_metadata
if err := r.proMetaMgr.Delete(ctx, p.Scope.Reference, "retention_id"); err != nil {
return r.SendError(ctx, err)
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Avoid inconsistent state: make metadata cleanup atomic with policy delete (or compensate).

If DeleteRetention succeeds but proMetaMgr.Delete fails, the client gets an error even though the policy is gone, and the stale retention_id may block future creates. Do the metadata removal first (ignore NotFound), then delete the policy and attempt a best-effort rollback of metadata if the policy delete fails.

Apply this diff within the block:

- if err = r.retentionCtl.DeleteRetention(ctx, params.ID); err != nil {
-   return r.SendError(ctx, err)
- }
- // delete retention data in project_metadata
- if err := r.proMetaMgr.Delete(ctx, p.Scope.Reference, "retention_id"); err != nil {
-   return r.SendError(ctx, err)
- }
+ // 1) Remove project metadata first so we never block future creates with a stale key.
+ if err := r.proMetaMgr.Delete(ctx, p.Scope.Reference, "retention_id"); err != nil {
+   if !errors.IsNotFoundErr(err) {
+     return r.SendError(ctx, err)
+   }
+ }
+ // 2) Delete the policy; if it fails, best-effort restore the metadata for consistency.
+ if err := r.retentionCtl.DeleteRetention(ctx, params.ID); err != nil {
+   _ = r.proMetaMgr.Add(ctx, p.Scope.Reference, map[string]string{"retention_id": strconv.FormatInt(p.ID, 10)})
+   return r.SendError(ctx, err)
+ }

Longer-term: consider moving both operations into the retention controller and executing them in a single DB transaction for true atomicity.

🤖 Prompt for AI Agents
In src/server/v2.0/handler/retention.go around lines 263-266, avoid the current
ordering that deletes the policy then the metadata; instead read and save the
current retention_id from project metadata, attempt to delete the metadata first
(treat NotFound as success), then call DeleteRetention; if DeleteRetention
fails, attempt a best-effort rollback by restoring the saved retention_id into
project metadata and return the original DeleteRetention error; if rollback
restore fails, log the restore failure and still return the original error so
the client knows the policy delete failed. Ensure operations handle idempotency
and surface the correct error to the client.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review continued from previous batch...

Comment on lines +10 to +15
POSTGRESQL_HOST=postgresql
POSTGRESQL_PORT=5432
POSTGRESQL_USERNAME=postgres
POSTGRESQL_PASSWORD=root123
POSTGRESQL_DATABASE=registry
POSTGRESQL_SSLMODE=disable
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Also avoid committing DB credentials.

POSTGRESQL_PASSWORD=root123 should be injected, even in dev.

Apply:

-POSTGRESQL_PASSWORD=root123
+POSTGRESQL_PASSWORD=${POSTGRESQL_PASSWORD:-devpassword}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
POSTGRESQL_HOST=postgresql
POSTGRESQL_PORT=5432
POSTGRESQL_USERNAME=postgres
POSTGRESQL_PASSWORD=root123
POSTGRESQL_DATABASE=registry
POSTGRESQL_SSLMODE=disable
POSTGRESQL_HOST=postgresql
POSTGRESQL_PORT=5432
POSTGRESQL_USERNAME=postgres
POSTGRESQL_PASSWORD=${POSTGRESQL_PASSWORD:-devpassword}
POSTGRESQL_DATABASE=registry
POSTGRESQL_SSLMODE=disable
🤖 Prompt for AI Agents
In .dagger/config/core/env around lines 10 to 15, the file currently contains a
committed plaintext DB password (POSTGRESQL_PASSWORD=root123); remove this
hardcoded credential and change the config to expect an injected secret instead
(e.g., reference an environment variable or secret placeholder), add a
.env.example or README entry showing the variable name, and ensure the real
password is provided at runtime via the CI/CD/secret manager or dagger secret
injection so no credentials are stored in the repo.

Comment on lines +25 to +27
CORE_SECRET=D1fWkYKg6OQgISeP
JOBSERVICE_SECRET=gVQJX0AlVjstElN8
WITH_TRIVY=False
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Hardcoded secrets committed to VCS (critical).

CORE_SECRET, JOBSERVICE_SECRET, and REGISTRY_CREDENTIAL_PASSWORD are real-looking secrets. Even for dev, keep them out of Git; load via Dagger Secrets, env injection, or a local .env not tracked.

Suggested minimal hardening (placeholders + env expansion):

-CORE_SECRET=D1fWkYKg6OQgISeP
-JOBSERVICE_SECRET=gVQJX0AlVjstElN8
+CORE_SECRET=${CORE_SECRET:?set CORE_SECRET via dev secrets}
+JOBSERVICE_SECRET=${JOBSERVICE_SECRET:?set JOBSERVICE_SECRET via dev secrets}
@@
-REGISTRY_CREDENTIAL_PASSWORD=TRJUhYbJgSjXZWAj3oLEet3ugJ3nAOk3
+REGISTRY_CREDENTIAL_PASSWORD=${REGISTRY_CREDENTIAL_PASSWORD:?set REGISTRY_CREDENTIAL_PASSWORD}

Follow-up:

  • Move this file to .dagger/config/core/env.example and .gitignore the real env.
  • Wire Dagger to pass secrets (e.g., WithSecretFile/WithEnvVariable).

I can provide a Dagger snippet to load these from the host or a secret store.

Also applies to: 37-37

🧰 Tools
🪛 Gitleaks (8.27.2)

25-25: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


26-26: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)

🤖 Prompt for AI Agents
.dagger/config/core/env lines 25-27 (and line 37) contain hardcoded secrets;
remove these real secret values and replace them with non-sensitive placeholders
(e.g., CORE_SECRET=${CORE_SECRET:-placeholder}) and entries referencing
environment expansion only, move the current file to
.dagger/config/core/env.example, add .dagger/config/core/env to .gitignore, and
update Dagger pipeline to inject the real secrets at runtime via Dagger secrets
or host env (e.g., WithSecretFile/WithEnvVariable) so that no real secrets are
committed to VCS.

Comment on lines +35 to +50
func (m *Harbor) JobService(ctx context.Context) *dagger.Service {
jobSrvConfig := m.OnlyDagger.File("./.dagger/config/jobservice/config.yml")
envFile := m.OnlyDagger.File("./.dagger/config/jobservice/env")
run_script := m.OnlyDagger.File("./.dagger/config/run_env.sh")

jobSrv := m.BuildImage(ctx, DEV_PLATFORM, "jobservice", true).
WithMountedFile("/etc/jobservice/config.yml", jobSrvConfig).
WithMountedDirectory("/var/log/jobs", m.OnlyDagger.Directory("./.dagger/config/jobservice")).
WithMountedFile("/envFile", envFile).
WithMountedFile("/run_script", run_script).
WithExec([]string{"chmod", "+x", "/run_script"}).
WithExposedPort(8080).
WithEntrypoint([]string{"/run_script", "/jobservice -c /etc/jobservice/config.yml"}).
AsService()
return jobSrv
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Entrypoint args are passed as one string; the binary won’t exec.

Each argv element must be split; same pattern appears elsewhere.

-    WithEntrypoint([]string{"/run_script", "/jobservice -c /etc/jobservice/config.yml"}).
+    WithEntrypoint([]string{"/run_script", "/jobservice", "-c", "/etc/jobservice/config.yml"}).

Also consider using a cache volume for “/var/log/jobs” instead of mounting the config dir over it.

-    WithMountedDirectory("/var/log/jobs", m.OnlyDagger.Directory("./.dagger/config/jobservice")).
+    WithMountedCache("/var/log/jobs", dag.CacheVolume("jobservice-logs")).
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
func (m *Harbor) JobService(ctx context.Context) *dagger.Service {
jobSrvConfig := m.OnlyDagger.File("./.dagger/config/jobservice/config.yml")
envFile := m.OnlyDagger.File("./.dagger/config/jobservice/env")
run_script := m.OnlyDagger.File("./.dagger/config/run_env.sh")
jobSrv := m.BuildImage(ctx, DEV_PLATFORM, "jobservice", true).
WithMountedFile("/etc/jobservice/config.yml", jobSrvConfig).
WithMountedDirectory("/var/log/jobs", m.OnlyDagger.Directory("./.dagger/config/jobservice")).
WithMountedFile("/envFile", envFile).
WithMountedFile("/run_script", run_script).
WithExec([]string{"chmod", "+x", "/run_script"}).
WithExposedPort(8080).
WithEntrypoint([]string{"/run_script", "/jobservice -c /etc/jobservice/config.yml"}).
AsService()
return jobSrv
}
func (m *Harbor) JobService(ctx context.Context) *dagger.Service {
jobSrvConfig := m.OnlyDagger.File("./.dagger/config/jobservice/config.yml")
envFile := m.OnlyDagger.File("./.dagger/config/jobservice/env")
run_script := m.OnlyDagger.File("./.dagger/config/run_env.sh")
jobSrv := m.BuildImage(ctx, DEV_PLATFORM, "jobservice", true).
WithMountedFile("/etc/jobservice/config.yml", jobSrvConfig).
WithMountedCache("/var/log/jobs", dag.CacheVolume("jobservice-logs")).
WithMountedFile("/envFile", envFile).
WithMountedFile("/run_script", run_script).
WithExec([]string{"chmod", "+x", "/run_script"}).
WithExposedPort(8080).
WithEntrypoint([]string{
"/run_script",
"/jobservice",
"-c",
"/etc/jobservice/config.yml",
}).
AsService()
return jobSrv
}
🤖 Prompt for AI Agents
In .dagger/devServices.go around lines 35 to 50, the WithEntrypoint call is
passing the entire command as a single string so the binary will not be exec'd;
change the entrypoint to pass each argv element separately (first element is the
executable, subsequent elements are its args) so dagger can exec the binary
correctly, e.g. split "/run_script" and "/jobservice -c
/etc/jobservice/config.yml" into separate elements. Also avoid mounting the
config directory over /var/log/jobs — replace the mounted directory with a cache
volume (or a dedicated directory) for /var/log/jobs so logs are persisted
without shadowing your config files.

Comment on lines +81 to +95
regConfigDir := m.OnlyDagger.Directory("./.dagger/config/registry")
regCtlConfig := m.OnlyDagger.File("./.dagger/config/registryctl/config.yml")
envFile := m.OnlyDagger.File("./.dagger/config/jobservice/env")
run_script := m.OnlyDagger.File("./.dagger/config/run_env.sh")

regCtl := m.BuildImage(ctx, DEV_PLATFORM, "registryctl", true).
WithMountedDirectory("/etc/registry", regConfigDir).
WithMountedFile("/etc/registryctl/config.yml", regCtlConfig).
WithMountedFile("/envFile", envFile).
WithMountedFile("/run_script", run_script).
WithEntrypoint([]string{"/run_script", "/registryctl -c /etc/registryctl/config.yml"}).
AsService()

return regCtl
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Wrong env file and same entrypoint-args bug.

Uses jobservice/env instead of registryctl/env; and entrypoint packs args into one string.

-  envFile := m.OnlyDagger.File("./.dagger/config/jobservice/env")
+  envFile := m.OnlyDagger.File("./.dagger/config/registryctl/env")
 ...
-    WithEntrypoint([]string{"/run_script", "/registryctl -c /etc/registryctl/config.yml"}).
+    WithEntrypoint([]string{"/run_script", "/registryctl", "-c", "/etc/registryctl/config.yml"}).
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
regConfigDir := m.OnlyDagger.Directory("./.dagger/config/registry")
regCtlConfig := m.OnlyDagger.File("./.dagger/config/registryctl/config.yml")
envFile := m.OnlyDagger.File("./.dagger/config/jobservice/env")
run_script := m.OnlyDagger.File("./.dagger/config/run_env.sh")
regCtl := m.BuildImage(ctx, DEV_PLATFORM, "registryctl", true).
WithMountedDirectory("/etc/registry", regConfigDir).
WithMountedFile("/etc/registryctl/config.yml", regCtlConfig).
WithMountedFile("/envFile", envFile).
WithMountedFile("/run_script", run_script).
WithEntrypoint([]string{"/run_script", "/registryctl -c /etc/registryctl/config.yml"}).
AsService()
return regCtl
}
regConfigDir := m.OnlyDagger.Directory("./.dagger/config/registry")
regCtlConfig := m.OnlyDagger.File("./.dagger/config/registryctl/config.yml")
envFile := m.OnlyDagger.File("./.dagger/config/registryctl/env")
run_script := m.OnlyDagger.File("./.dagger/config/run_env.sh")
regCtl := m.BuildImage(ctx, DEV_PLATFORM, "registryctl", true).
WithMountedDirectory("/etc/registry", regConfigDir).
WithMountedFile("/etc/registryctl/config.yml", regCtlConfig).
WithMountedFile("/envFile", envFile).
WithMountedFile("/run_script", run_script).
WithEntrypoint([]string{"/run_script", "/registryctl", "-c", "/etc/registryctl/config.yml"}).
AsService()
return regCtl
}
🤖 Prompt for AI Agents
.dagger/devServices.go around lines 81 to 95: the code mounts the wrong env file
(./.dagger/config/jobservice/env) and constructs the entrypoint with all args as
a single string; change envFile to use ./.dagger/config/registryctl/env (or the
correct registryctl env file path) when creating m.OnlyDagger.File, and change
WithEntrypoint to pass command and args as separate slice elements (e.g. split
"/registryctl -c /etc/registryctl/config.yml" into "/registryctl", "-c",
"/etc/registryctl/config.yml") so the runtime receives separate argv entries
rather than one combined string.

Comment on lines +135 to +136
registryPasswordPlain, _ := registryPassword.Plaintext(ctx)

Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Security concern: plaintext password exposure

The code retrieves the registry password as plaintext and then passes it as a command-line argument, which could be visible in process listings.

Consider using environment variables or stdin for passing sensitive credentials:

-	registryPasswordPlain, _ := registryPassword.Plaintext(ctx)
-
 	cosign_ctr := dag.Container().From("cgr.dev/chainguard/cosign")
 
 	// ... existing code ...
 
 	return cosign_ctr.WithSecretVariable("REGISTRY_PASSWORD", registryPassword).
 		WithExec([]string{"cosign", "env"}).
 		WithExec([]string{
 			"cosign", "sign", "--yes", "--recursive",
 			"--registry-username", registryUsername,
-			"--registry-password", registryPasswordPlain,
+			"--registry-password", "env:REGISTRY_PASSWORD",
 			imageAddr,
 			"--timeout", "1m",
 		}).Stdout(ctx)
🤖 Prompt for AI Agents
In .dagger/main.go around lines 135-136, the code calls
registryPassword.Plaintext(ctx) and passes the result as a command-line
argument, exposing the credential in process listings; replace this pattern by
avoiding plaintext CLI args — keep the secret as a Dagger secret or environment
variable and inject it into the container via a secret mount or an env var (or
pass via stdin) rather than building it into the command string; specifically,
do not call Plaintext() to inline the secret, instead attach registryPassword as
a secret to the step (or set it in the container's environment from the Dagger
secret) and modify the command invocation to read the credential from the
mounted secret file or from stdin/env.

Comment on lines +96 to +105
- name: Set component vars
id: comp
run: |
if [ "${{ matrix.debug }}" = "true" ]; then
export FINAL_IMAGE_TAG="${{ needs.setup.outputs.DEBUGTAG }}"
else
export FINAL_IMAGE_TAG="${{ needs.setup.outputs.VERSIONTAG }}"
fi
echo "FINAL_IMAGE_TAG=$FINAL_IMAGE_TAG" >> $GITHUB_ENV
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Set and use FINAL_IMAGE_TAG consistently (outputs vs env)

You write to GITHUB_ENV but later read from steps.comp.outputs. Use env in downstream steps.

Apply:

       - name: Set component vars
         id: comp
         run: |
           if [ "${{ matrix.debug }}" = "true" ]; then
             export FINAL_IMAGE_TAG="${{ needs.setup.outputs.DEBUGTAG }}"
           else
             export FINAL_IMAGE_TAG="${{ needs.setup.outputs.VERSIONTAG }}"
           fi
-          echo "FINAL_IMAGE_TAG=$FINAL_IMAGE_TAG" >> $GITHUB_ENV
+          echo "FINAL_IMAGE_TAG=$FINAL_IMAGE_TAG" >> "$GITHUB_ENV"

And below, replace usages:

-              --image-tags ${{ steps.comp.outputs.FINAL_IMAGE_TAG }} \
+              --image-tags ${{ env.FINAL_IMAGE_TAG }} \

and

-          if echo "$OUTPUT" | grep -qE "^${{ env.REGISTRY_DOMAIN }}/${{ env.PROJECT_NAME }}/[a-z0-9./-]+:${{ steps.comp.outputs.FINAL_IMAGE_TAG }}@sha256:[a-f0-9]{64}$"; then
+          if echo "$OUTPUT" | grep -qE "^${{ env.REGISTRY_DOMAIN }}/${{ env.PROJECT_NAME }}/[a-z0-9./-]+:${{ env.FINAL_IMAGE_TAG }}@sha256:[a-f0-9]{64}$"; then
🧰 Tools
🪛 actionlint (1.7.7)

98-98: shellcheck reported issue in this script: SC2086:info:6:44: Double quote to prevent globbing and word splitting

(shellcheck)

Comment on lines +227 to 239
allAdapters, err := c.regMgr.ListRegistryProviderTypes(ctx)
if err != nil {
return []string{}, err
}
whitelistedAdapters := getWhitelistedAdapters(ctx)
var filtered []string
for _, t := range allAdapters {
if _, ok := whitelistedAdapters[t]; ok {
filtered = append(filtered, t)
}
}
return filtered, nil
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Bug: Empty adapter list when no whitelist is configured. Default should be “all.”

If getWhitelistedAdapters returns nil, current code returns an empty slice, hiding all adapters in UI/API.

Apply:

  func (c *controller) ListRegistryProviderTypes(ctx context.Context) ([]string, error) {
  	allAdapters, err := c.regMgr.ListRegistryProviderTypes(ctx)
  	if err != nil {
  		return []string{}, err
  	}
  	whitelistedAdapters := getWhitelistedAdapters(ctx)
+  if whitelistedAdapters == nil {
+    // No whitelist configured → expose all adapters
+    return allAdapters, nil
+  }
  	var filtered []string
  	for _, t := range allAdapters {
  		if _, ok := whitelistedAdapters[t]; ok {
  			filtered = append(filtered, t)
  		}
  	}
  	return filtered, nil
  }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
allAdapters, err := c.regMgr.ListRegistryProviderTypes(ctx)
if err != nil {
return []string{}, err
}
whitelistedAdapters := getWhitelistedAdapters(ctx)
var filtered []string
for _, t := range allAdapters {
if _, ok := whitelistedAdapters[t]; ok {
filtered = append(filtered, t)
}
}
return filtered, nil
}
func (c *controller) ListRegistryProviderTypes(ctx context.Context) ([]string, error) {
allAdapters, err := c.regMgr.ListRegistryProviderTypes(ctx)
if err != nil {
return []string{}, err
}
whitelistedAdapters := getWhitelistedAdapters(ctx)
if whitelistedAdapters == nil {
// No whitelist configured → expose all adapters
return allAdapters, nil
}
var filtered []string
for _, t := range allAdapters {
if _, ok := whitelistedAdapters[t]; ok {
filtered = append(filtered, t)
}
}
return filtered, nil
}
🤖 Prompt for AI Agents
In src/controller/registry/controller.go around lines 227 to 239, the code
filters allAdapters by whitelistedAdapters but if getWhitelistedAdapters(ctx)
returns nil it treats that as an empty whitelist and returns no adapters; change
the logic to treat a nil whitelist as "allow all" by checking if
whitelistedAdapters == nil (or len==0 only if intended) and in that case return
allAdapters directly; otherwise proceed to filter only those present in the
whitelist and return the filtered list.

Comment on lines +52 to 59
func (a *auditlogAPI) checkPermissionAndBuildQuery(ctx context.Context, qs *string, sort *string, page *int64, pageSize *int64) (*q.Query, error) {
secCtx, ok := security.FromContext(ctx)
if !ok {
return a.SendError(ctx, errors.UnauthorizedError(errors.New("security context not found")))
return nil, errors.UnauthorizedError(errors.New("security context not found"))
}
if !secCtx.IsAuthenticated() {
return a.SendError(ctx, errors.UnauthorizedError(nil).WithMessage(secCtx.GetUsername()))
return nil, errors.UnauthorizedError(nil).WithMessage(secCtx.GetUsername())
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Don’t leak username and use a clear unauthenticated message.

Returning UnauthorizedError with the username as message is odd and can leak info. Prefer an explicit reason.

Apply:

-	if !secCtx.IsAuthenticated() {
-		return nil, errors.UnauthorizedError(nil).WithMessage(secCtx.GetUsername())
-	}
+	if !secCtx.IsAuthenticated() {
+		return nil, errors.UnauthorizedError(errors.New("user not authenticated"))
+	}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
func (a *auditlogAPI) checkPermissionAndBuildQuery(ctx context.Context, qs *string, sort *string, page *int64, pageSize *int64) (*q.Query, error) {
secCtx, ok := security.FromContext(ctx)
if !ok {
return a.SendError(ctx, errors.UnauthorizedError(errors.New("security context not found")))
return nil, errors.UnauthorizedError(errors.New("security context not found"))
}
if !secCtx.IsAuthenticated() {
return a.SendError(ctx, errors.UnauthorizedError(nil).WithMessage(secCtx.GetUsername()))
return nil, errors.UnauthorizedError(nil).WithMessage(secCtx.GetUsername())
}
func (a *auditlogAPI) checkPermissionAndBuildQuery(ctx context.Context, qs *string, sort *string, page *int64, pageSize *int64) (*q.Query, error) {
secCtx, ok := security.FromContext(ctx)
if !ok {
return nil, errors.UnauthorizedError(errors.New("security context not found"))
}
if !secCtx.IsAuthenticated() {
return nil, errors.UnauthorizedError(errors.New("user not authenticated"))
}
🤖 Prompt for AI Agents
In src/server/v2.0/handler/auditlog.go around lines 52 to 59, the
UnauthorizedError is currently returned with the user's username which leaks
PII; change the error to use a generic unauthenticated message instead. Replace
the return that calls WithMessage(secCtx.GetUsername()) with a call that
supplies a non-identifying string (e.g., "user not authenticated" or
"unauthenticated") so the API responds clearly without exposing the username.

Comment on lines +116 to +123
if params.Policy.SingleActiveReplication != nil {
// Validate and assign SingleActiveReplication only for non-event_based triggers
if params.Policy.Trigger != nil && params.Policy.Trigger.Type == model.TriggerTypeEventBased && *params.Policy.SingleActiveReplication {
return r.SendError(ctx, fmt.Errorf("single active replication is not allowed for event_based triggers"))
}
policy.SingleActiveReplication = *params.Policy.SingleActiveReplication
}

Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Return typed 400 error instead of fmt error for invalid SingleActiveReplication on event_based

Use errors.BadRequestError to ensure HTTP 400 and consistent error shape.

Apply:

-    if params.Policy.Trigger != nil && params.Policy.Trigger.Type == model.TriggerTypeEventBased && *params.Policy.SingleActiveReplication {
-        return r.SendError(ctx, fmt.Errorf("single active replication is not allowed for event_based triggers"))
-    }
+    if params.Policy.Trigger != nil && params.Policy.Trigger.Type == model.TriggerTypeEventBased && *params.Policy.SingleActiveReplication {
+        return r.SendError(ctx, errors.BadRequestError(nil).
+            WithMessage("single_active_replication is not allowed for event_based triggers"))
+    }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if params.Policy.SingleActiveReplication != nil {
// Validate and assign SingleActiveReplication only for non-event_based triggers
if params.Policy.Trigger != nil && params.Policy.Trigger.Type == model.TriggerTypeEventBased && *params.Policy.SingleActiveReplication {
return r.SendError(ctx, fmt.Errorf("single active replication is not allowed for event_based triggers"))
}
policy.SingleActiveReplication = *params.Policy.SingleActiveReplication
}
if params.Policy.SingleActiveReplication != nil {
// Validate and assign SingleActiveReplication only for non-event_based triggers
if params.Policy.Trigger != nil && params.Policy.Trigger.Type == model.TriggerTypeEventBased && *params.Policy.SingleActiveReplication {
return r.SendError(ctx, errors.BadRequestError(nil).
WithMessage("single_active_replication is not allowed for event_based triggers"))
}
policy.SingleActiveReplication = *params.Policy.SingleActiveReplication
}
🤖 Prompt for AI Agents
In src/server/v2.0/handler/replication.go around lines 116 to 123, the handler
currently returns a generic fmt.Errorf when SingleActiveReplication is
disallowed for event_based triggers; replace that with a typed 400 error by
returning errors.BadRequestError (or the repo's BadRequestError constructor)
with the same message, e.g. use errors.BadRequestError("single active
replication is not allowed for event_based triggers"); also add the necessary
import for the errors package if missing so the code compiles and keeps
consistent HTTP 400 response shape.

Comment on lines +192 to +199
if params.Policy.SingleActiveReplication != nil {
// Validate and assign SingleActiveReplication only for non-event_based triggers
if params.Policy.Trigger != nil && params.Policy.Trigger.Type == model.TriggerTypeEventBased && *params.Policy.SingleActiveReplication {
return r.SendError(ctx, fmt.Errorf("single active replication is not allowed for event_based triggers"))
}
policy.SingleActiveReplication = *params.Policy.SingleActiveReplication
}

Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Same 400 mapping in Update path

Apply:

-    if params.Policy.Trigger != nil && params.Policy.Trigger.Type == model.TriggerTypeEventBased && *params.Policy.SingleActiveReplication {
-        return r.SendError(ctx, fmt.Errorf("single active replication is not allowed for event_based triggers"))
-    }
+    if params.Policy.Trigger != nil && params.Policy.Trigger.Type == model.TriggerTypeEventBased && *params.Policy.SingleActiveReplication {
+        return r.SendError(ctx, errors.BadRequestError(nil).
+            WithMessage("single_active_replication is not allowed for event_based triggers"))
+    }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if params.Policy.SingleActiveReplication != nil {
// Validate and assign SingleActiveReplication only for non-event_based triggers
if params.Policy.Trigger != nil && params.Policy.Trigger.Type == model.TriggerTypeEventBased && *params.Policy.SingleActiveReplication {
return r.SendError(ctx, fmt.Errorf("single active replication is not allowed for event_based triggers"))
}
policy.SingleActiveReplication = *params.Policy.SingleActiveReplication
}
if params.Policy.SingleActiveReplication != nil {
// Validate and assign SingleActiveReplication only for non-event_based triggers
if params.Policy.Trigger != nil && params.Policy.Trigger.Type == model.TriggerTypeEventBased && *params.Policy.SingleActiveReplication {
return r.SendError(ctx, errors.BadRequestError(nil).
WithMessage("single_active_replication is not allowed for event_based triggers"))
}
policy.SingleActiveReplication = *params.Policy.SingleActiveReplication
}
🤖 Prompt for AI Agents
In src/server/v2.0/handler/replication.go around lines 192 to 199, the
validation error for SingleActiveReplication with event_based triggers is
currently returned via a generic error which does not map to HTTP 400; change
the return to produce the same Bad Request (400) response used in the create
path (i.e., return a typed/utility bad-request error or call the handler's
SendBadRequest/SendErrorWithCode so the client receives a 400), preserving the
existing message text and flow and ensuring Update path validation uses
identical 400 mapping as Create.

@Vad1mo Vad1mo merged commit 41ac785 into next Sep 3, 2025
21 of 38 checks passed
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.