License review -fixes - #3
Open
manojdev-ai wants to merge 17 commits into
Open
Conversation
manojdev-ai
commented
Aug 2, 2026
manojdev-ai
commented
Contributor
- 2b5f20f Apache-2.0 licence text + NOTICE findings 1, 4
- 0474f9c Pin dependencies and container versions finding 7
- 6eedd94 Third-party inventory + SBOM tooling findings 8, 6
- 10f382d Model licences + HF_TOKEN finding 5
- 4fdc2b8 Self-hosted fonts and Chart.js finding 9
- 332cd77 SHA-pinned installers + RELEASE.md finding 2
- d5297ce CI enforcement + docs findings 3, 10 + all
Addresses licence review findings 1 (blocker) and 4.
LICENSE contained the 17-line notice intended for source-file headers
(740 bytes), not the licence. Sections 3 (patent grant) and 5
(contributions) were absent. Incorporation by reference likely preserves
the terms legally, but automated scanners cannot detect it: GitHub's
licence API returned {"key":"other","spdx_id":"NOASSERTION"}, and
enterprise tooling (FOSSA, Black Duck, Snyk) treats NOASSERTION as
unknown-licence and blocks ingestion. The repo was quietly failing
corporate review with no signal back to us.
Replaced with the full 11,344-byte text, copyright filled in at the
appendix.
Added NOTICE — optional under Apache-2.0 but the conventional home for
the copyright line. It also names the three dependencies with
obligations that survive redistribution, and states plainly that the
Apache grant does not extend to model weights.
Addresses licence review finding 7. Every entry in requirements.txt used >=, and the benchmark scripts pinned vllm/vllm-openai:latest and rocm/vllm:latest. Two installs on different days resolve different transitive sets, potentially with different licences, and nothing records which set was actually used. Python deps: requirements.in intended ranges, hand-edited requirements.txt pinned resolution, 52 packages, generated requirements.lock.txt same with hashes Regenerate both with `make deps-lock`. Containers: benchmarks/versions.env is now the single source of truth, sourced by all ten benchmark scripts. It supports digest pins and falls back to tags with a visible warning. The digests themselves still need resolving against the registry — left empty rather than guessed. Each run now prints its own provenance (image, host, date, git SHA) before starting. The reproducibility half of this matters more than the licence half for a product that publishes benchmark numbers. If we cannot say which vLLM build produced a figure, the figure is not defensible.
Addresses licence review findings 8 and 6 (blocker). Finding 8 — repo dependencies. THIRD-PARTY-NOTICES.md inventories all 52 packages in the pinned set, generated by scripts/generate_third_party_notices.py from PyPI metadata. Three carry obligations that survive redistribution of a binary artifact: paramiko LGPL-2.1 psycopg2-binary LGPL-3.0-or-later (OpenSSL exception) certifi MPL-2.0 Each has a written compliance position rather than just a licence string. The review scoped this as medium on the assumption that users build locally and we convey no copies; since we do distribute prebuilt images, it is a blocker and the notices have to travel with them. Finding 6 — workload images. scripts/generate_sbom.sh scans llminference, jupyternotebook and benchmark-client with syft, resolving each image's digest first, and emits SPDX JSON plus a roll-up report that flags GPL/AGPL/proprietary/NOASSERTION packages by name. The scan itself needs registry credentials and is not run here. Two questions in the generated report need counsel, not a scanner: whether the NVIDIA CUDA EULA's redistribution conditions permit our distribution model, and how the Debian base layer's GPL source offer is passed along. benchmark-client is not a workload_type, so its tag is mapped explicitly to its paired llminference version; the script hard-fails on any image it cannot resolve rather than defaulting to a tag that may not exist.
Addresses licence review finding 5.
Two of four catalogued models are gated behind Meta's manual approval,
and benchmarks/*.sh add two more. Nothing said so, and a user could
reasonably read our Apache-2.0 grant as covering the weights. It does
not.
Worse, there was no HF_TOKEN path anywhere in the codebase — only
HF_HOME — so gated models could never have downloaded fresh. They
worked only from a pre-warmed cache, which makes the failure look
intermittent.
MODEL-LICENSES.md per-model terms, gate status, access instructions,
the Llama Community Licence provisions that
actually bite, and the DeepSeek-R1-Distill mixed
lineage question
catalog.json license / gated / license_url per model
check_model_access.py reports gate status for every model referenced
by catalog.json and benchmarks/; --token verifies
the token has real access rather than just
resolving the model card
The token is read from ~/.aistudio/env on the GPU node and forwarded
into the workload container using docker's passthrough form (-e VAR with
no value), so the secret never appears in the command string, the API,
or a run manifest. Unset, the flags collapse and behaviour is unchanged.
Deliberately NOT ~/.bashrc. Debian and Ubuntu ship a ~/.bashrc that
returns early for non-interactive shells, and every command the worker
runs over SSH is non-interactive — an export appended there would be
set when an operator logs in by hand and unset for every benchmark.
`make check-node-env NODE=<host>` exercises the same non-interactive
path the worker uses, so it cannot pass while the real thing fails.
Unresolved: gs://aistudio-datasets/sharegpt.json has provenance that
cannot be cleared by inspecting the file. Options and a recommendation
are in MODEL-LICENSES.md; it needs a product decision.
Note the new catalog.json keys are not yet read by anything — /models is
served from a separate hardcoded dict in app/catalog.py. Surfacing a
gate warning before a run starts is the follow-up that turns this from
documentation into a guardrail.
Addresses licence review finding 9, plus a second instance it did not
name.
demo-ui/index.html loaded Inter and JetBrains Mono from
fonts.googleapis.com and Chart.js from jsdelivr. Both are problems for
the same two reasons:
Privacy Requesting fonts from Google discloses the visitor's IP to
a third party. LG Muenchen I, 3 O 17493/20 (20 Jan 2022)
held that to be a GDPR breach absent consent.
Function CDN assets fail outright in air-gapped clusters, which is
the target deployment. This was a functional bug in the
environment we actually ship into that happened to also be
a privacy problem.
scripts/vendor_frontend_assets.sh fetches all three from npm, copies
their licence texts alongside, generates the @font-face CSS and writes
demo-ui/vendor/NOTICE — so the OFL-1.1 notice obligation created by
self-hosting is discharged by the same script that creates it. Latin
subset, only the weights the page uses.
The binaries are gitignored and fetched at setup time (`make setup`
depends on it, both installers run it) to keep 460 KB out of the
history. The generated CSS, NOTICE and licence texts ARE committed —
those are compliance artifacts and must exist in the repository rather
than be produced by a step someone might skip.
This is weaker than committing the binaries and worth being precise
about. GDPR is fully resolved either way: a one-time fetch on the
operator's machine at install time discloses nothing about visitors.
Air-gap is resolved for the GPU nodes, which never serve the dashboard,
but not for an air-gapped server host — that case needs
demo-ui/vendor/ copied from a connected machine, and the script says so
when it fails.
Degradation is quiet where it can be: every font-family already falls
back to a system stack, so a missing .woff2 costs appearance only. A
missing Chart.js cannot degrade, so the page shows a banner naming the
command to fix it rather than throwing a bare ReferenceError.
Addresses licence review finding 2. The v1.0.0 release asset was 1,340 bytes containing only install.sh and install.ps1, byte-identical to the repo copies. It held no source: the scripts fetch the code from git at run time, so the v1.0.0 artifact did not contain v1.0.0. Nothing was archived. GitHub already attaches complete source archives to the same page for free, and those do include LICENSE — the manual upload was strictly worse than the thing next to it. The install chain also had no integrity verification: the tag was lightweight and unsigned, install.sh checked out a tag NAME, and no checksums were published. A tag is movable — anyone with write access can repoint it and every subsequent install silently gets different code. Both installers now pin a commit SHA. On top of that they check whether the tag still points at that commit, which is the one thing git will not warn about and the actual attack a mutable tag permits. Verifying HEAD == COMMIT after `git checkout COMMIT` would prove nothing, since git resolves commits by content address. Tag signatures are verified when present and reported either way, non-fatally — most users will not have imported the signing key. install.ps1 also gains an explicit frontend vendoring step; install.sh gets it for free via `make setup`. RELEASE.md documents the corrected process: annotated signed tags, no hand-built tarball, GitHub's archives plus a signed SHA256SUMS, and a verification checklist. Still to do: generate a CoreSpan signing key, re-cut v1.0.0, and update COMMIT in both installers to the full post-remediation SHA. Until then they point at the pre-remediation tree.
Makes the rest of this branch self-enforcing. Every check here exists
because something in the 31 July review got past a human once; the
point is that it cannot happen quietly a second time.
.github/workflows/compliance.yml
LICENSE size and required sections
GitHub licence API returns Apache-2.0 (finding 1's real failure mode
was machine detection, not human reading)
NOTICE / THIRD-PARTY-NOTICES / MODEL-LICENSES / vendor NOTICE present
no unpinned requirement, and requirements.txt matches a fresh
compile of requirements.in
THIRD-PARTY-NOTICES.md matches the pinned set
no third-party origin in demo-ui/
committed licence texts match a clean regeneration, and every font
fonts.css references is actually produced
model gate status, weekly and non-blocking — the Hub rate-limits and
that should not block a merge
workload image SBOMs, disabled pending registry credentials
Makefile gains compliance, deps-lock, third-party, vendor-assets, sbom,
check-models and check-node-env. `make compliance` runs locally
whatever CI runs, so the feedback loop does not require a push.
The unpinned-dependency guard is written to match on any range
operator rather than by package-name character class: requirements.in
contains uvicorn[standard]>=0.30.0, and a class that mishandles the
brackets would let exactly the most likely mistake through.
README gains a Licensing section pointing at the four inventories, and
corrects the framing the review flagged in finding 3 — workload image
access is now documented as public and verifiable by the reader, with
~/gcr.json demoted from a blanket node requirement to something needed
only for private tags.
docs/LICENCE-REVIEW-RESPONSE.md is the finding-by-finding response for
product: verdict, fix, what is still open and who owns it. Findings 3,
6 and 10 need decisions outside engineering.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Fixes the "Committed licence texts match a clean regeneration" failure (exit 126) on PR #3. Exit 126 is "found but not executable". This repo has core.fileMode=false because it is developed on Windows, so chmod +x in a working tree never reaches the index and every shebang file in the repo — all 24, not just the ones this branch added — was committed 100644. The runner checked out vendor_frontend_assets.sh non-executable and ./scripts/... died immediately. Fixed with `git update-index --chmod=+x` across all of them. This is pre-existing: `./benchmarks/t4/tinyllama-tp1.sh` as documented in the README would have failed the same way on any fresh clone from a case-sensitive checkout. benchmarks/versions.env stays 644 — it is sourced, not executed. Added a CI check so it cannot recur: any tracked file with a shebang must be executable, with the update-index invocation in the error message since chmod alone will not fix it here. Two more failures were waiting in the same workflow, neither caused by this branch, both of which would have turned an unrelated PR red: requirements check — compiled into an empty scratch file, so uv resolved everything to latest instead of preserving the committed pins. Any release by any of the 52 packages would have failed the diff. Now seeds the scratch file with requirements.txt first, which asserts what we actually mean: requirements.txt is a valid resolution of requirements.in, not the newest possible one. third-party inventory check — a PyPI hiccup produced LOOKUP-FAILED entries, which then differed from the committed file and failed the build. An outage is not evidence that the inventory is stale. Now retries with backoff, and reports without failing if packages remain unreachable. Adding retries introduced a worse problem — 52 packages times three attempts against a dead endpoint hung for roughly half an hour before giving up. Added a circuit breaker after 3 consecutive failures and dropped the per-request timeout to 15s. Measured against a blackholed proxy: 9 seconds to a clean exit, down from >30 minutes.
manojdev-ai
requested review from
KarthikAbburi,
TejaMuraharirao and
rsingh-alt
August 3, 2026 11:11
…th. Now we can show warning about HF_Token
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.