Repository navigation
CLI-1510 Generate a release report from the Harbor benchmark (PoC) - #1241
jonathan-balli-sonarsource wants to merge 2 commits into
Conversation
✅ Deploy Preview for sonarqube-cli canceled.
|
|
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
| - name: Build the release report | ||
| id: release-report | ||
| if: ${{ !cancelled() && hashFiles('harbor-jobs/**') != '' }} | ||
| shell: bash | ||
| env: | ||
| CLI_VERSION: ${{ inputs.cli-version }} | ||
| ITERATION: ${{ inputs.iteration }} | ||
| run: | | ||
| iteration=() | ||
| [ -n "${ITERATION}" ] && iteration=(--iteration "${ITERATION}") | ||
| mkdir release-report | ||
| bun benchmarks/harbor/report/to-json.ts \ | ||
| --cli-version "${CLI_VERSION}" --run-id "${GITHUB_RUN_ID}" --commit "${GITHUB_SHA}" \ | ||
| --date "$(date -u +%Y-%m-%dT%H:%M:%SZ)" ${iteration[@]+"${iteration[@]}"} \ | ||
| harbor-jobs > "release-report/${GITHUB_RUN_ID}.json" |
There was a problem hiding this comment.
💡 Security: Release JSON and dashboard are published without the secrets scan
The README sets the rule "Nothing is published before it is scanned". It also says the report job scans its report text, even though that text is built only from result.json fields and numbers taken from artifacts that were already scanned. The new release JSON comes from the same fields: task names, agent and model names, config.json model names and trial folder names. Yet it is uploaded as the release-report artifact and pushed to a durable external repository, and dashboard.html is uploaded too, without ci/scan-secrets.py running on either. This bypasses the scan the README describes, and the data store may become public (see open points). Fix: run python3 -I benchmarks/harbor/ci/scan-secrets.py release-report before the upload, the same way report.txt is scanned.
Was this helpful? React with 👍 / 👎
There was a problem hiding this comment.
Not changed. The release JSON is built only from task, agent and model names and from numbers in result.json and trajectory.json final_metrics, never from message text, so I see no way for a secret to reach it. The artifacts it reads were already scanned and redacted by the benchmark jobs. I would rather not add a scan step to a PoC for this.
There was a problem hiding this comment.
Checked benchmarks/harbor/report/to-json.ts. The output has no message text: the only free-form strings are task, agent and model, and everything else is a number or a fixed status. The leak risk is lower than the finding suggests.
One gap remains. On the crash path (crashedTrialRecord, lines 100-109), task comes from the trial folder name and agent/model from config.json. Those files were not read from result.json, and the agent may be able to write them. The README says the artifacts were "already scanned and redacted", but a value in a field that no scan flagged would still be published to the external repository and the dashboard.
If you accept that small risk for the PoC, the finding can stay as a known limitation. Running python3 -I benchmarks/harbor/ci/scan-secrets.py release-report before the upload is a one-line step if you change your mind. I made no code changes.
…d build the dashboard in the workflow
3044cac to
1e72d52
Compare
CI failed: The Harbor release report job ran exit 1 after the report was generated and uploaded, likely because one macOS integrate--happy claude-sonnet trial failed (1/2 passed); the exit condition is not shown in the log, and the link to this PR is unclear.OverviewOne unique error template across 5 analyzed logs from the Report job. Report generation and artifact upload succeeded, but the job ended with an explicit FailuresReport job exits 1 after successful report generation (confidence: medium)
Summary
Code Review 👍 Approved with suggestions 3 closed / 4 findings🟡 Medium risk · The opt-in workflow writes benchmark metadata to an external repository with a token, so misconfiguration could retain or expose data beyond artifact expiry. Adds a Harbor benchmark release report: per-run JSON from trial results, a standalone HTML dashboard, and git-branch and local-dir storage adapters wired into a new workflow with a store job. Three earlier issues were resolved (plain 💡 Security: Release JSON and dashboard are published without the secrets scan📄 .github/workflows/harbor-benchmark.yml:165-176 📄 .github/workflows/harbor-benchmark.yml:182-196 📄 .github/workflows/harbor-benchmark.yml:251-263 📄 benchmarks/harbor/README.md:22 The README sets the rule "Nothing is published before it is scanned". It also says the report job scans its report text, even though that text is built only from ✅ 3 closed✅ Bug: New release-report artifact breaks to-json.ts on a plain
|
| Auto-apply | Compact |
|
|
Was this helpful? React with 👍 / 👎 | Gitar
|
Why
The benchmark artifacts expire after 90 days, and the ones of a run started from a PR branch are deleted when the PR is closed (
pr-cleanup.yml). To follow the CLI over time, each run must be kept in a small, durable form. This is a draft PoC for CLI-1510, opened only to run the workflow and the CI from this branch. It is not for review and should not be merged.What
report/to-json.ts <runDir>writes one JSON per run on the standard output: the release (CLI version, run id, commit, date, optional iteration) and one line per trial (OS, task, agent, model, status, agent time, steps, tokens, cost). It reads theharbor-jobsdirectory of the report job (merged artifacts) or a run downloaded withgh run download, and stops with a non-zero code on anything unexpected. The OS is the name of the job directory.report/dashboard.ts <dataDir>anddashboard-template.htmlbuild a standalone HTML dashboard (no network, no dependency) from a directory of those JSON.report/adapters/git-branch.shandlocal-dir.share the storage:push <file>adds a JSON,pull <dir>gets them all back.git-branch.shkeeps them on the orphan branchbenchmark-dataof a git repository, and another storage only needs another adapter, so the two scripts above do not depend on where the data lives.cli-version,iteration,store-report). The report job always builds the JSON and uploads it as therelease-reportartifact. A newstorejob runs only whenstore-reportis checked: it pushes the JSON, pulls all of them and uploadsdashboard.html.Testing
bun run typecheckand Prettier pass.to-json.tswas run on three real runs (48, 40 and 40 trials) in both directory forms, with the same trials each time, and on the error cases (no folder, mixed forms, a job without trial, wrong number of arguments). The adapters were tested against a local bare repository (no branch, first push, same file twice, modified file, unreachable repository) and against a private test repository on GitHub. The steps of the report job were replayed locally on a real run: 40 trials, push, pull and a dashboard. Not tested yet: a run of the workflow on GitHub (the token, the write access of thestorejob and the boolean input).Open points
The storage is a private test repository (
jonathan-balli-sonarsource/harbor-benchmark-data-poc), written in the workflow and reached with the repository secretHB_BENCHMARK_DATA_TOKEN(a fine-grained token limited to that repository). Both are temporary. Whether a public repository is acceptable for these numbers, where to keep them, and what triggers the report (release.ymlor a schedule) are still to be decided.