Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
149 changes: 99 additions & 50 deletions .github/scripts/check-licenses.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,76 +16,125 @@
import json
import sys
import re
from pathlib import Path

def load_data(json_file):
path = Path(json_file).resolve()
if not path.is_relative_to(Path.cwd().resolve()):
print(f"Refusing to read outside the working directory: {json_file}", file=sys.stderr)
Comment thread
anchildress1 marked this conversation as resolved.
sys.exit(1)
try:
with open(json_file, 'r') as f:
with open(path, 'r') as f:
return json.load(f)
except Exception as e:
print(f"Error reading {json_file}: {e}", file=sys.stderr)
sys.exit(1)

def check_licenses(data, allowed):
bad = []
def _component_license(component):
licenses = []
for entry in component.get('licenses', []):
if not isinstance(entry, dict):
continue
license_info = entry.get('license') or {}
value = entry.get('expression') or license_info.get('id') or license_info.get('name')
if value:
licenses.append(TROVE_CLASSIFIERS.get(value, value))
# Every declared entry applies to the component, so all of them must pass.
return ' AND '.join(f'({value})' for value in licenses) or None


def _license_entries(data):
if isinstance(data, list):
# Python format: list of dicts
for item in data:
name = item.get('Name')
lic = item.get('License')
if not license_allowed(lic, allowed):
bad.append((name, lic))
yield item.get('Name'), item.get('License')
elif isinstance(data, dict) and 'components' in data:
for component in data['components']:
yield component.get('name'), _component_license(component)
elif isinstance(data, dict):
# Check for CycloneDX SBOM format
if 'components' in data:
for component in data['components']:
name = component.get('name')
lic_entries = component.get('licenses', [])
# Extract license names/ids from nested structure
lics = []
for l in lic_entries:
if isinstance(l, dict):
lic_obj = l.get('license', {})
lic_id = lic_obj.get('id') or lic_obj.get('name')
if lic_id:
lics.append(lic_id)
lic = lics[0] if len(lics) == 1 else lics if lics else None
if not license_allowed(lic, allowed):
bad.append((name, lic))
else:
# Node format: dict of dicts
for pkg, info in data.items():
lic = info.get('licenses')
if not license_allowed(lic, allowed):
bad.append((pkg, lic))
for package, info in data.items():
yield package, info.get('licenses')
else:
print("Unknown JSON format", file=sys.stderr)
sys.exit(1)
return bad


def license_allowed(lic, allowed):
"""Return True if license value `lic` matches an allowed id exactly.
def check_licenses(data, allowed):
return [(name, lic) for name, lic in _license_entries(data) if not license_allowed(lic, allowed)]


class LicenseExpression:
"""SPDX subset: ids, AND, OR, parentheses; AND binds tighter. WITH, `+`, and anything else fail closed."""

def __init__(self, expression, allowed):
self.tokens = re.findall(r"\(|\)|[A-Za-z0-9.-]+", expression)
self.valid_tokens = bool(self.tokens) and "".join(self.tokens).lower() == re.sub(
Comment thread
anchildress1 marked this conversation as resolved.
r"\s+", "", expression
).lower()
self.allowed_ids = {item.lower() for item in allowed if item}
self.position = 0

def is_allowed(self):
if not self.valid_tokens:
return False
try:
result = self._or()
except ValueError:
return False
return result and self.position == len(self.tokens)

def _or(self):
result = self._and()
while self._current() == 'OR':
self.position += 1
next_result = self._and()
result = result or next_result
return result

def _and(self):
result = self._atom()
while self._current() == 'AND':
self.position += 1
next_result = self._atom()
result = result and next_result
return result

def _atom(self):
token = self._current()
if token is None or token in {'AND', 'OR', ')'}:
raise ValueError('expected license')
self.position += 1
if token == '(':
result = self._or()
if self._current() != ')':
raise ValueError('unclosed license group')
self.position += 1
return result
return token.lower() in self.allowed_ids

def _current(self):
if self.position < len(self.tokens):
return self.tokens[self.position].upper()
return None


# cyclonedx-py reports PyPI trove classifiers verbatim; they are not SPDX expressions.
TROVE_CLASSIFIERS = {
'License :: OSI Approved :: Apache Software License': 'Apache',
'License :: OSI Approved :: BSD License': 'BSD',
'License :: OSI Approved :: ISC License (ISCL)': 'ISC',
'License :: OSI Approved :: MIT License': 'MIT',
'License :: OSI Approved :: Python Software Foundation License': 'Python',
}

- `lic` can be None, a string, or a list/iterable.
- `allowed` is a set of allowed identifiers (case-insensitive).
- Matching is whole-token only: substring matches like 'mit' in
'limited' passed the old check, and UNKNOWN fails closed.
"""

def license_allowed(lic, allowed):
"""Accept a license value only when a permitted choice satisfies every required license."""
Comment thread
anchildress1 marked this conversation as resolved.
if lic is None:
return False

allowed_lc = {a.lower() for a in allowed if a}

if isinstance(lic, (list, tuple)):
items = [str(x).lower() for x in lic if x]
else:
items = [str(lic).lower()]

for item in items:
tokens = re.split(r"[^a-z0-9.-]+", item)
if any(a in tokens for a in allowed_lc):
return True
return False
return any(license_allowed(item, allowed) for item in lic)
value = str(lic).strip()
return LicenseExpression(TROVE_CLASSIFIERS.get(value, value), allowed).is_allowed()

def main():
if len(sys.argv) != 2:
Expand Down
49 changes: 44 additions & 5 deletions .github/scripts/test_check_licenses.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,25 +2,38 @@
"""Regression tests for check-licenses.py. Run: python3 test_check_licenses.py"""

import importlib.util
import subprocess
import sys
import tempfile
from pathlib import Path

spec = importlib.util.spec_from_file_location(
"check_licenses", Path(__file__).parent / "check-licenses.py"
)
SCRIPT = Path(__file__).parent / "check-licenses.py"
spec = importlib.util.spec_from_file_location("check_licenses", SCRIPT)
check_licenses = importlib.util.module_from_spec(spec)
spec.loader.exec_module(check_licenses)

ALLOWED = {"MIT", "Apache-2.0", "BSD-3-Clause", "ISC", "LicenseRef-PolyForm-Shield-1.0.0"}
ALLOWED = {"MIT", "Apache-2.0", "Apache", "BSD-3-Clause", "BSD", "ISC", "LicenseRef-PolyForm-Shield-1.0.0"}

CASES = [
("MIT", True),
("BSD-3-Clause", True),
("(MIT OR Apache-2.0)", True),
("GPL-3.0 OR MIT", True),
("MIT AND BSD-3-Clause", True),
("GPL-3.0 AND MIT", False),
("GPL-3.0 OR MIT AND GPL-3.0", False),
("MIT AND (GPL-3.0 OR BSD-3-Clause)", True),
("(MIT OR GPL-3.0) AND GPL-3.0", False),
("MIT OR", False),
("MIT WITH Classpath-exception-2.0", False),
("LicenseRef-PolyForm-Shield-1.0.0", True),
(["GPL-3.0", "MIT"], True),
# substring matches must not pass: 'mit' in 'limited'
("Limited Proprietary License", False),
("mitigated-license", False),
("License :: OSI Approved :: Apache Software License", True),
("License :: OSI Approved :: BSD License", True),
("License :: OSI Approved :: GNU General Public License v3 (GPLv3)", False),
("GPL-3.0", False),
# unknown/missing licenses fail closed
("UNKNOWN", False),
Expand All @@ -32,7 +45,33 @@ def main():
for lic, want in CASES:
got = check_licenses.license_allowed(lic, ALLOWED)
assert got == want, f"license_allowed({lic!r}) = {got}, expected {want}"
print(f"ok - {len(CASES)} license checker cases pass")
formats = [
({"pkg": {"licenses": "GPL-3.0 AND MIT"}}, True),
({"pkg": {"licenses": "MIT OR GPL-3.0"}}, False),
([{"Name": "pkg", "License": "GPL-3.0 AND MIT"}], True),
({"components": [{"name": "pkg", "licenses": [{"license": {"id": "MIT"}}]}]}, False),
({"components": [{"name": "pkg", "licenses": [{"expression": "MIT OR GPL-3.0"}]}]}, False),
({"components": [{"name": "pkg", "licenses": [{"expression": "MIT AND GPL-3.0"}]}]}, True),
({"components": [{"name": "pkg", "licenses": [{"license": {"id": "MIT"}}, {"license": {"id": "GPL-3.0"}}]}]}, True),
({"components": [{"name": "pkg", "licenses": [
{"license": {"id": "BSD-3-Clause"}},
{"license": {"name": "License :: OSI Approved :: BSD License"}},
]}]}, False),
]
for data, want_bad in formats:
got_bad = bool(check_licenses.check_licenses(data, ALLOWED))
assert got_bad == want_bad, f"check_licenses({data!r}) bad={got_bad}, expected {want_bad}"
with tempfile.TemporaryDirectory() as tmp:
root = Path(tmp)
(root / "licenses.json").write_text('{"pkg": {"licenses": "MIT"}}')
(root / "work").mkdir()
for cwd, arg, want_code in [(root, "licenses.json", 0), (root / "work", "../licenses.json", 1)]:
result = subprocess.run(
[sys.executable, str(SCRIPT), arg], cwd=cwd, capture_output=True, text=True
)
assert result.returncode == want_code, f"{arg} from {cwd}: exit {result.returncode}"
assert "outside the working directory" in result.stderr, result.stderr
print(f"ok - {len(CASES) + len(formats) + 2} license checker cases pass")


if __name__ == "__main__":
Expand Down
42 changes: 35 additions & 7 deletions .github/workflows/test-and-build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ jobs:
name: Node ${{ matrix.node-version }}
runs-on: ubuntu-latest
timeout-minutes: 9
if: (github.event_name == 'workflow_dispatch' || github.event.pull_request.draft == false) && !startsWith(github.head_ref, 'release-please--')
if: github.event_name != 'pull_request' || github.event.pull_request.draft == false
Comment thread
anchildress1 marked this conversation as resolved.
permissions:
contents: read
strategy:
Expand Down Expand Up @@ -73,7 +73,7 @@ jobs:
name: Bundle Analysis
runs-on: ubuntu-latest
timeout-minutes: 6
if: (github.event_name == 'workflow_dispatch' || github.event.pull_request.draft == false) && !startsWith(github.head_ref, 'release-please--')
if: github.event_name != 'pull_request' || github.event.pull_request.draft == false
Comment thread
anchildress1 marked this conversation as resolved.
permissions:
contents: read

Expand Down Expand Up @@ -121,7 +121,7 @@ jobs:
name: Python ${{ matrix.python-version }}
runs-on: ubuntu-latest
timeout-minutes: 9
if: (github.event_name == 'workflow_dispatch' || github.event.pull_request.draft == false) && !startsWith(github.head_ref, 'release-please--')
if: github.event_name != 'pull_request' || github.event.pull_request.draft == false
Comment thread
anchildress1 marked this conversation as resolved.
permissions:
contents: read
strategy:
Expand Down Expand Up @@ -188,9 +188,10 @@ jobs:
name: Commitlint
runs-on: ubuntu-latest
timeout-minutes: 5
if: github.event_name == 'pull_request' && github.event.pull_request.draft == false && !startsWith(github.head_ref, 'release-please--')
if: github.event_name == 'pull_request' && github.event.pull_request.draft == false
permissions:
contents: read
pull-requests: read
steps:
- uses: actions/checkout@v7.0.1
with:
Expand All @@ -213,13 +214,40 @@ jobs:
# origin/<base> scopes the lint to branch-unique commits; base.sha
# ranges re-lint history merged in from the base branch
- name: Lint PR commit messages
run: npx commitlint --from origin/${{ github.event.pull_request.base.ref }} --to ${{ github.event.pull_request.head.sha }}
env:
GH_TOKEN: ${{ github.token }}
PR_AUTHOR: ${{ github.event.pull_request.user.login }}
PR_NUMBER: ${{ github.event.pull_request.number }}
HEAD_REF: ${{ github.head_ref }}
BASE_REF: ${{ github.event.pull_request.base.ref }}
HEAD_SHA: ${{ github.event.pull_request.head.sha }}
run: |
set -euo pipefail
bot_author=""
if [[ "$PR_AUTHOR" == "dependabot[bot]" ]] || \
[[ "$PR_AUTHOR" == "github-actions[bot]" && "$HEAD_REF" == release-please--* ]]; then
bot_author="$PR_AUTHOR"
fi

if [[ -z "$bot_author" ]]; then
node_modules/.bin/commitlint --from "origin/$BASE_REF" --to "$HEAD_SHA"
exit 0
fi

# The PR commits API caps at 250 entries, so it only allowlists; the git range decides coverage.
verified_bot_shas=$(gh api --paginate "repos/$GITHUB_REPOSITORY/pulls/$PR_NUMBER/commits" |
jq -r --arg bot "$bot_author" \
'.[] | select(.author.login == $bot and .commit.verification.verified == true) | .sha')
Comment thread
anchildress1 marked this conversation as resolved.
git rev-list "origin/$BASE_REF..$HEAD_SHA" | while read -r sha; do
grep -qxF "$sha" <<< "$verified_bot_shas" && continue
git show -s --format=%B "$sha" | node_modules/.bin/commitlint
done

dependency-review:
name: Dependency Review
runs-on: ubuntu-latest
timeout-minutes: 6
if: github.event_name == 'pull_request' && github.event.pull_request.draft == false && !startsWith(github.head_ref, 'release-please--')
if: github.event_name == 'pull_request' && github.event.pull_request.draft == false
Comment thread
anchildress1 marked this conversation as resolved.
permissions:
contents: read
pull-requests: write
Expand Down Expand Up @@ -250,7 +278,7 @@ jobs:
runs-on: ubuntu-latest
timeout-minutes: 9
needs: [node-tests, python-tests]
if: (github.event_name == 'workflow_dispatch' || github.event.pull_request.draft == false) && !startsWith(github.head_ref, 'release-please--')
if: github.event_name != 'pull_request' || github.event.pull_request.draft == false
Comment thread
anchildress1 marked this conversation as resolved.
permissions:
contents: read
pull-requests: write
Expand Down
10 changes: 1 addition & 9 deletions commitlint.config.js
Original file line number Diff line number Diff line change
@@ -1,14 +1,6 @@
/** @satisfies {import('@commitlint/types').UserConfig} */
export default {
extends: ['@commitlint/config-conventional'],
// Dependabot titles regularly exceed header-max-length and carry no
// RAI footer; its commits are machine-generated and not lintable.
// Both conditions are required so a human can't skip linting by
// pasting the trailer into an unrelated commit message.
ignores: [
(message) =>
/^build\(deps(-dev)?\): bump /.test(message) &&
message.includes('Signed-off-by: dependabot[bot] <support@github.com>'),
],
rules: {
'header-max-length': [2, 'always', 72],
'footer-max-line-length': [2, 'always', 100],
Expand Down
16 changes: 15 additions & 1 deletion packages/node-commitlint/tests/integration.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import lint from '@commitlint/lint';
import { RuleConfigSeverity } from '@commitlint/types';
import { RuleConfigSeverity, type UserConfig } from '@commitlint/types';
import { describe, it, expect } from 'vitest';
import repositoryConfig from '../../../commitlint.config.js';
import plugin from '../src/index';

// Runs the rules through the real commitlint pipeline so a broken export
Expand Down Expand Up @@ -43,4 +44,17 @@ describe('commitlint integration', () => {
expect(result.errors[0].name).toBe('rai-signed-off-by');
expect(result.errors[0].message).toContain('Signed-off-by');
});

it('rejects a forged Dependabot message under the repository policy', async () => {
Comment thread
anchildress1 marked this conversation as resolved.
const result = await lint(
'build(deps): bump example\n\nSigned-off-by: dependabot[bot] <support@github.com>',
repositoryConfig.rules,
{
ignores: (repositoryConfig as UserConfig).ignores,
plugins: { 'commitlint-plugin-rai': plugin },
},
);
Comment thread
anchildress1 marked this conversation as resolved.
expect(result.valid).toBe(false);
expect(result.errors.some((error) => error.name === 'rai-footer-exists')).toBe(true);
});
});
5 changes: 0 additions & 5 deletions packages/python-gitlint/MANIFEST.in
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,6 @@ include CHANGELOG.md
recursive-include gitlint_rai *.py py.typed
recursive-include gitlint_rai/sbom *.json

Comment thread
anchildress1 marked this conversation as resolved.
global-exclude *.pyc
global-exclude __pycache__
global-exclude .DS_Store
global-exclude .gitignore
exclude uv.lock
prune tests
prune reports
prune gitlint_rai.egg-info
Expand Down
Loading
Loading