Add License Scanning and Reporting for Endpoints Changes on Main.#412
Add License Scanning and Reporting for Endpoints Changes on Main.#412arav-agarwal2 wants to merge 2 commits into
Conversation
|
MLCommons CLA bot All contributors have signed the MLCommons CLA ✍️ ✅ |
There was a problem hiding this comment.
Code Review
This pull request adds a .github/scanoss.json configuration file to define scanning skip patterns (such as tests, vendor, and third-party directories) and specify BOM inclusions and exclusions. There are no review comments, and I have no feedback to provide.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
arekay-nv
left a comment
There was a problem hiding this comment.
Review-council: SCANOSS license-scan workflow
Three independent reviewers (Codex gpt-5.5, Grok 4.5, Claude) looked at this PR. The workflow is well-built and security-conscious — correct push-to-main trigger (not pull_request_target), permissions: contents: read, and defensive shell (handles the all-zeros first-push SHA, empty deltas, and a missing key). No critical/high issues: ${{ }} interpolation into run: is limited to commit SHAs + env.REPORTS_DIR (not injectable), and the data.items() results walk matches SCANOSS's output shape.
Findings below are reliability/hardening, most-severe first. The top one (flagged by all three reviewers) is the only thing I'd treat as merge-worthy — a failed scan currently reports as "clean." The rest are optional. Nothing here blocks the PR.
| [ -s "$OUT" ] || echo '{}' > "$OUT" | ||
| echo "SCAN_RC=$SCAN_RC" >> "$GITHUB_ENV" |
There was a problem hiding this comment.
Silent "clean" on scan failure — flagged by all three reviewers (Codex P2, Grok high, Claude). SCAN_RC is captured and exported to $GITHUB_ENV, but no later step reads it. On a failed scan (bad/expired key, quota exhausted, API or CLI error) the [ -s "$OUT" ] || echo '{}' fallback leaves results.json as {}, so the Summary prints "no components detected in this delta" — indistinguishable from a genuinely clean scan. For a license/BOM check, a failed run that looks compliant is the worst failure mode. Keep it non-blocking, but make failure visible:
| [ -s "$OUT" ] || echo '{}' > "$OUT" | |
| echo "SCAN_RC=$SCAN_RC" >> "$GITHUB_ENV" | |
| [ -s "$OUT" ] || echo '{}' > "$OUT" | |
| if [ "$SCAN_RC" -ne 0 ]; then | |
| echo "::warning title=SCANOSS scan failed::scanoss-py exited $SCAN_RC; the report below may be incomplete." | |
| echo "SCAN_FAILED=1" >> "$GITHUB_ENV" | |
| fi | |
| echo "SCAN_RC=$SCAN_RC" >> "$GITHUB_ENV" |
Then have the report builder print a "SCAN_FAILED=1, so the Summary can't be misread as clean.
| SCAN_RC=0 | ||
| if [ -s changed_files.txt ]; then | ||
| KEY_ARG=() | ||
| [ -n "${SCANOSS_API_KEY:-}" ] && KEY_ARG=(--key "${SCANOSS_API_KEY}") |
There was a problem hiding this comment.
API key on the process argv (Grok, Claude). KEY_ARG=(--key "${SCANOSS_API_KEY}") puts the secret in argv, readable via /proc/<pid>/cmdline for the scan's lifetime. Recent scanoss-py reads SCANOSS_API_KEY from the environment (already exported in this step's env:), so you can drop the --key arg and the KEY_ARG array entirely. If you keep --key, confirm scanoss never echoes its argv. Exposure is bounded (ephemeral single-tenant runner), hence low severity.
| python-version: "3.11" | ||
|
|
||
| - name: Install scanoss-py | ||
| run: pip install scanoss |
There was a problem hiding this comment.
Pin scanoss (Grok, Claude). pip install scanoss installs whatever PyPI serves at run time — non-reproducible, and a breaking/compromised release silently changes scan/convert/inspect behavior (and the scanoss.winnowing import the quota probe needs). Pin to the version you validate against, e.g. pip install scanoss==<x.y.z>, ideally via a constraints/requirements file.
|
|
||
| steps: | ||
| - name: Checkout code | ||
| uses: actions/checkout@v4 |
There was a problem hiding this comment.
Optional: SHA-pin the actions. actions/checkout@v4 (also setup-python@v5, upload-artifact@v4) are floating major tags. For a workflow that handles a secret on main, pinning to full commit SHAs (version in a trailing comment) stops a moved tag from changing runner behavior without a diff. Low priority — all three are first-party GitHub actions.
| from scanoss.winnowing import Winnowing | ||
| wfp = Winnowing().wfp_for_contents(".github/scanoss.json", False, open(".github/scanoss.json", "rb").read()) | ||
| rid = str(uuid.uuid4()) | ||
| r = requests.post( |
There was a problem hiding this comment.
Quota probe spends the quota it measures (Grok, Claude). This POSTs a winnowed wfp to /scan/direct every run only to read the X-Ratelimit-* headers — consuming one file-scan against the same per-key quota it reports, and adding a requests + scanoss.winnowing dependency for a Summary-only cosmetic. Consider dropping it, or reading the rate-limit headers off the main scan's HTTP response instead of issuing a separate billable request.
| { | ||
| "purl": "pkg:github/mlcommons/endpoints" | ||
| }, | ||
| { | ||
| "purl": "pkg:github/nvidia/tensorrt-edge-llm" | ||
| }, | ||
| { | ||
| "purl": "pkg:github/nvlabs/cosmos-policy" | ||
| } | ||
| ], | ||
| "exclude": [ |
There was a problem hiding this comment.
Confirm the BOM include scope (Grok, Claude). include lists pkg:github/nvidia/tensorrt-edge-llm and pkg:github/nvlabs/cosmos-policy alongside this repo. Declaring other orgs' repos as BOM members tells SCANOSS their matches are declared, which can mask undeclared/copyleft findings in this tree. If mlcommons/endpoints doesn't vendor those, restrict include to this repo:
| { | |
| "purl": "pkg:github/mlcommons/endpoints" | |
| }, | |
| { | |
| "purl": "pkg:github/nvidia/tensorrt-edge-llm" | |
| }, | |
| { | |
| "purl": "pkg:github/nvlabs/cosmos-policy" | |
| } | |
| ], | |
| "exclude": [ | |
| "include": [ | |
| { | |
| "purl": "pkg:github/mlcommons/endpoints" | |
| } | |
| ], |
Keep the extra entries if the cross-repo declaration is intentional.
What does this PR do?
As part of the rollout for our AI coding policy, we'd like to add a small licensing checker to this codebase. It shouldn't block anything - it just reports changes on an action report we can review asynchronously.
Type of change
Related issues
Testing
Checklist