Repository navigation
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3326 +/- ##
============================================
- Coverage 54.29% 52.81% -1.48%
+ Complexity 2616 2185 -431
============================================
Files 770 762 -8
Lines 67952 67689 -263
Branches 9213 9192 -21
============================================
- Hits 36892 35748 -1144
- Misses 27482 28430 +948
+ Partials 3578 3511 -67 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
imbajin
left a comment
imbajin
left a comment
There was a problem hiding this comment.
The head, base and complete two-file patch are identical to fork #300. Independent cross-check confirms the existing source-review evidence applies: 8/10 for the original full change at 189ca23, and 9/10 for the a35c43e follow-up delta. These are separate scopes, not a new full-PR 9/10. The writer remains limited to two named master repositories and submits only into its own repository. No new source blocker found. Verify the first continuous Java 17 master submission, manifest completeness and real dependency-review comparison independently in each repository; fork CI and manual snapshot claims do not replace those gates.
bitflicker64
left a comment
bitflicker64
left a comment
There was a problem hiding this comment.
Blocking: yes. Summary: The submit job depends on an action that the ASF allow list does not include, so the workflow would fail at startup in apache/hugegraph and the ASF baseline would not be produced. The rest of the workflow checks out: the three pinned SHAs match their tags, every input exists in the pinned action, and the validate job's tests pass at this head. Evidence: gh api -X GET repos/apache/infrastructure-actions/contents/approved_patterns.yml and actions.yml grepped for advanced-security and maven-dependency; gh api -X GET repos/<action>/git/ref/tags/<tag> for the three pins; action.yml of the submission action at a64327a7; python3 -m unittest discover -s .github/scripts -p 'test_ci_policy.py' and -s install-dist/scripts/dependency -p 'test_*.py' at a35c43e (both OK).
bitflicker64
left a comment
bitflicker64
left a comment
There was a problem hiding this comment.
Blocking: no. Summary: the read-only generator and the checkout-free writer hold up against the code at this head, and the serializer matches the pinned upstream parser on the points I compared; the one finding is that a partial re-run of submit can never pass, which the docs do not say. The workflow has not run end to end in either repository, so runner and API behaviour (artifact download with the job token, snapshot acceptance, dependency-review comparison) is still unverified. Evidence: read the full diff at 02ad423 (4 files, +817); python3 -m unittest discover for test_ci_policy.py, test_dependency_snapshot.py (9 tests) and install-dist/scripts/dependency (7 tests), all OK; gh api -X GET repos/actions/<name>/git/ref/tags/<tag> confirms the checkout v7.0.1, setup-java v6.0.1 and upload-artifact v4.6.2 pins; ran depgraph-maven-plugin:4.0.3:graph over the 38-module reactor locally and dependency-snapshot.py on the result (38 manifests, 5054 resolved entries, 1.7 MB), then applied the writer's manifest, package URL and direct-dependency checks to it with no rejection; compared manifest() with src/depgraph.ts at actions/maven-dependency-submission-action 1f666f7a.
imbajin
left a comment
imbajin
left a comment
There was a problem hiding this comment.
Blocking: no. Summary: The new graph test does not exercise an indirect-only dependency, so it would not catch an all-direct relationship regression. Evidence: the fixture at .github/scripts/test_dependency_snapshot.py:175 makes every package direct.
- submit canonical master snapshots in a separate job - limit write permission to trusted push and dispatch - pin privileged actions and verify the event revision - document repository scope and baseline acceptance
- allow the designated org master producer - keep fork and pull request events read-only - document isolated baseline acceptance checks
- use the pinned GitHub-maintained upstream fork - preserve master-only writes and freshness guards - document the permitted publisher and source reference
- build and serialize Maven graphs in a read-only job - validate a bounded JSON ZIP in a fresh data-only writer - preserve dependency identity and master freshness - document boundaries and add archive redirect regressions
- document all-job reruns or fresh master dispatch - test dependency paths containing only indirect edges - verify direct edges take precedence over indirect paths
e506519 to
d78ad6c
Compare
- select dependency checks for producer and workflow inputs - keep fixture tests in the existing entry job - verify mixed changes retain runtime coverage
bitflicker64
left a comment
bitflicker64
left a comment
There was a problem hiding this comment.
Blocking: yes. Summary: The read-only generator, the checkout-free writer and the serializer hold up against the code at this head, and both earlier review threads (re-run recovery, indirect-only fixture) are addressed. One gap remains: a master commit that stops being the tip before its run finishes never gets a baseline and cannot be backfilled, so PRs based on it still fail dependency review. Evidence: full diff at 2fa78c1 (6 files, +860/-1); python3 -m unittest discover -s .github/scripts for test_ci_policy.py (22 tests) and test_dependency_snapshot.py (11 tests), both OK; manifest() compared with src/depgraph.ts and src/snapshot-generator.ts at actions/maven-dependency-submission-action 1f666f7a; checkout, setup-java and upload-artifact pins match their tags; gh api -X GET repos/apache/hugegraph/dependency-graph/compare/92af2f89...2fa78c13 (5058 additions, per-module POM manifests); master commit times and open-PR base SHAs from the GitHub API; the dependency-review log of run 38464100215. The workflow has not run in either repository, so artifact download, snapshot acceptance and the base/head comparison are unverified.
| EXPECTED_SHA: ${{ github.sha }} | ||
| run: | | ||
| test "$(git rev-parse HEAD)" = "$EXPECTED_SHA" | ||
| test "$(git ls-remote origin refs/heads/master | cut -f1)" = "$EXPECTED_SHA" |
There was a problem hiding this comment.
Important: a master commit that is no longer the branch tip when this check runs never gets a baseline, and nothing can add one later, so pull requests based on that commit keep failing dependency review.
Evidence: this step, the repeat at line 100 and the writer's Stale master before submission check (line 307) all fail the run when refs/heads/master is not github.sha. workflow_dispatch only runs for the current tip, and the concurrency group (line 28) keeps one pending run, so a superseded or replaced commit has no recovery path. Master moves faster than one run: #3337 and #3335 merged 46 seconds apart (20:01:23Z and 20:02:09Z on 2026-10-13), four commits landed between 08:56:45Z and 08:58:58Z on 2026-10-12, and mvn install alone took 3m48s in this PR's dependency-check job. dependency-review compares exact SHAs: on this head it logged "snapshots compared for the base SHA (0) and the head SHA (1) do not match", listed every dependency as added (5058 entries from the compare API) and failed on vulnerable packages. Open PRs sit on such commits: #3334 has base c1e7aba77, which was the tip for under 8 minutes before b84454fb0.
Requested change: submit the snapshot for the event SHA even after master advances. The run, artifact, SHA and ref identity checks already bind it to that commit, and the concurrency group already serializes submissions. If the tip-only rule has to stay, add a way to backfill a named master SHA and state in docs/ci.md that a PR whose base commit was skipped has no baseline until it is updated onto a submitted commit.
imbajin
left a comment
imbajin
left a comment
There was a problem hiding this comment.
Blocking: yes. Summary: The writer accepts schema-valid but incomplete dependency graphs from the Maven producer, allowing dependencies to be omitted from the submitted baseline. Evidence: exact-head static review of .github/workflows/submit-dependencies.yml:89-106, 294-306; run 38464100215 logged zero base snapshots versus one head snapshot.
| isinstance(package['dependencies'], list) and | ||
| all(isinstance(child, str) and child in resolved for child in package['dependencies']), | ||
| 'Package schema') | ||
| require(not resolved or any(p['relationship'] == 'direct' for p in resolved.values()), |
There was a problem hiding this comment.
contents: write job (lines 120-132). Please independently verify each graph's completeness against a trusted dependency inventory before submission, or otherwise ensure incomplete producer output cannot reach the token-bearing writer.
3 participants
Dependency review needs a trustworthy master inventory. Previously, Maven installation and dependency graph generation ran in a job with the snapshot write token. Both now run in a read-only job that emits JSON; a fresh writer validates and submits that data without checkout or Maven.
The writer validates run, artifact, source SHA and ref identity, then reads a bounded single snapshot.json ZIP member in memory without extraction. GitHub Authorization stays on the API request and is absent from the signed storage download. Repository, event, master freshness and snapshot identity guards remain enforced.
Dependency snapshot producer and workflow changes select dependency checks; snapshot fixtures run in the existing entry job. CI classifier edits retain conservative mainline validation. The updated routing and fixtures passed local tests and independent review.
Local parser equivalence, archive and redirect controls, workflow checks and three independent reviews passed. Actual ASF and org master full installation, snapshot submission and dependency-review comparison remain OPEN until the workflow runs after merge. Existing security checks remain enforced.
Updated docs/ci.md with the generation and writer boundaries and acceptance requirements. Doc - Done.
Conceptual overview
只读依赖基线生成与独立受限 writer，实际基线验收仍待合并后完成。
Conceptual illustration; not test or release-acceptance evidence.