Conversation
Reviewer's GuideThe container now invokes the installed File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="docker-image/Dockerfiles/Dockerfile" line_range="131-133" />
<code_context>
-# Copy trustify-da-javascript-client executable script from the builder stage
-COPY --from=builder /trustify-da.sh /trustify-da.sh
+# Use the trustify-da CLI directly as the entrypoint; any subcommand (stack,
+# remediate, sbom, image, license, ...) can be passed as `docker run <image> <command> [args]`
+ENTRYPOINT ["trustify-da"]
</code_context>
<issue_to_address>
**issue (broader_impact):** The direct CLI entrypoint removes the wrapper's automatic Yarn-version detection. A Yarn Classic project with `package.json` and `yarn.lock`, no `packageManager` field, and no manually supplied `TRUSTIFY_DA_YARN_PATH` is analyzed with the image's default `/usr/local/corepack/bin/yarn` instead of `/usr/local/bin/yarn-classic`, so dependency extraction runs with the wrong Yarn version and can fail or produce an incorrect dependency set.
**Triggers:** When analyzing Yarn Classic projects that do not declare `packageManager` and do not explicitly set `TRUSTIFY_DA_YARN_PATH`.
**Suggested fix:** Move the wrapper's manifest-based Yarn selection into the CLI/provider path, or retain a thin entrypoint wrapper that sets `TRUSTIFY_DA_YARN_PATH` before invoking `trustify-da`.
</issue_to_address>Sourcery assessment
Approval pending. 1 finding to address first.
Blocking findings: docker-image/Dockerfiles/Dockerfile:133
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #628 +/- ##
==========================================
+ Coverage 92.02% 92.07% +0.04%
==========================================
Files 45 45
Lines 10602 10665 +63
Branches 1921 1935 +14
==========================================
+ Hits 9757 9820 +63
Misses 845 845
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
|
[sdlc-workflow/verify-pr] Re: @sourcery-ai review — Classified as code change request — sub-task TC-6104 created to address this feedback (the Yarn Classic/Berry auto-detection regression). Same finding as inline comment on |
Verification Report for TC-5614 (commit ded6d3b)
Overall: FAILIssues requiring attention:
Security is clean and CI is green. This report is informational — a human reviewer decides whether to merge. This comment was AI-generated by sdlc-workflow/verify-pr v0.13.9. |
PR guacsec#628 removed the entrypoint wrapper that auto-detected Yarn variant from manifest signals. This restores that logic in the provider itself. When TRUSTIFY_DA_YARN_PATH is not explicitly set, the provider now: 1. Checks packageManager field in package.json (most authoritative) 2. Falls back to .yarnrc.yml presence (berry indicator) 3. Defaults to classic for bare v1 yarn.lock Only runs for Yarn projects (package.json + sibling yarn.lock), preventing accidental cross-ecosystem behavior for npm/pnpm/Bun projects. Implements TC-6104 Assisted-by: Claude Code
PR guacsec#628 removed the entrypoint wrapper that auto-detected Yarn variant from manifest signals. This restores that logic in the provider itself. When TRUSTIFY_DA_YARN_PATH is not explicitly set, the provider now: 1. Checks packageManager field in package.json (most authoritative) 2. Falls back to .yarnrc.yml presence (berry indicator) 3. Defaults to classic for bare v1 yarn.lock Only runs for Yarn projects (package.json + sibling yarn.lock), preventing accidental cross-ecosystem behavior for npm/pnpm/Bun projects. Implements TC-6104 Assisted-by: Claude Code
Sourcery withdrew this approval because the latest commits introduced blocking findings.
Remove trustify-da.sh shell wrapper from container image. The CLI already handles subcommand routing via yargs (stack, remediate, image, sbom, license, etc.), making the 120-line wrapper redundant. This enables all subcommands to work directly: docker run <image> stack <manifest> docker run <image> remediate <path> --dry-run docker run <image> sbom <manifest> Changes: - Remove COPY of trustify-da.sh and associated chmod from Dockerfile - Set ENTRYPOINT ["trustify-da"] to invoke CLI directly - Add Usage section to README.md with examples for all subcommands - Document TRUSTIFY_DA_YARN_PATH env var for Yarn version selection Implements TC-5614 Assisted-by: Claude Code
PR guacsec#628 removed the entrypoint wrapper that auto-detected Yarn variant from manifest signals. This restores that logic in the provider itself. When TRUSTIFY_DA_YARN_PATH is not explicitly set, the provider now: 1. Checks packageManager field in package.json (most authoritative) 2. Falls back to .yarnrc.yml presence (berry indicator) 3. Defaults to classic for bare v1 yarn.lock Only runs for Yarn projects (package.json + sibling yarn.lock), preventing accidental cross-ecosystem behavior for npm/pnpm/Bun projects. Implements TC-6104 Assisted-by: Claude Code
- restore Yarn Classic/Berry auto-detection without masking user overrides - remove obsolete GitHub Action entrypoint override - document CLI-focused image behavior and dry-run exit code
|
These yarn changes should be done in a separate PR in my opinion 😄 Ill review both PRs when theyre separate |
Summary
trustify-da.shshell wrapper from the container imageENTRYPOINT ["trustify-da"]to invoke the CLI directlyThe CLI already handles subcommand routing via yargs (stack, remediate, image, sbom, license, etc.), making the 120-line shell wrapper redundant. This enables all CLI subcommands to work directly through the container entrypoint.
The Yarn auto-detection logic (previously in the removed wrapper) is now implemented in
javascript_yarn.js. WhenTRUSTIFY_DA_YARN_PATHis not explicitly set, the provider auto-detects the Yarn variant from manifest signals (packageManager field, .yarnrc.yml presence, or defaults to classic for bare v1 yarn.lock). Detection only runs for Yarn projects (package.json + sibling yarn.lock), preventing cross-ecosystem behavior.Test plan
docker run <image> --helpshows CLI usagedocker run <image> stack --helpshows stack subcommand helpImplements TC-5614
Implements TC-6104
🤖 Generated with Claude Code
Summary by Sourcery
Use the Trustify CLI directly as the container entrypoint while preserving automatic Yarn variant selection and documenting container usage.
New Features:
Enhancements:
Build:
trustify-dadirectly as its entrypoint and omit the wrapper and implicit Yarn path.Documentation:
Tests: