Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 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
130 changes: 81 additions & 49 deletions .github/scripts/check-licenses.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,73 +19,105 @@

def load_data(json_file):
try:
with open(json_file, 'r') as f:

Check failure on line 22 in .github/scripts/check-licenses.py

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Path Traversal via faulty LLM-supplied CLI arguments

See more on https://sonarcloud.io/project/issues?id=anchildress1_rai-lint&issues=AaDu-q7Q1B6jItHtrcdL&open=AaDu-q7Q1B6jItHtrcdL&pullRequest=140
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(value)
if len(licenses) == 1:
return licenses[0]
return licenses or None
Comment thread
anchildress1 marked this conversation as resolved.
Outdated


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:
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

- `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)
return LicenseExpression(str(lic).strip(), allowed).is_allowed()

def main():
if len(sys.argv) != 2:
Expand Down
21 changes: 20 additions & 1 deletion .github/scripts/test_check_licenses.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,14 @@
("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'
Expand All @@ -32,7 +40,18 @@ 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),
]
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}"
print(f"ok - {len(CASES) + len(formats)} license checker cases pass")


if __name__ == "__main__":
Expand Down
12 changes: 6 additions & 6 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,7 +188,7 @@ 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 && github.event.pull_request.user.login != 'dependabot[bot]'
Comment thread
anchildress1 marked this conversation as resolved.
Outdated
Comment thread
anchildress1 marked this conversation as resolved.
Outdated
permissions:
contents: read
steps:
Expand Down Expand Up @@ -219,7 +219,7 @@ jobs:
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 +250,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
9 changes: 0 additions & 9 deletions commitlint.config.js
Original file line number Diff line number Diff line change
@@ -1,14 +1,5 @@
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
2 changes: 1 addition & 1 deletion docs/architecture.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,4 +37,4 @@ Each pattern matches a complete line of the form `Key: Name <contact>`:

The Node plugin (`packages/node-commitlint/src/rules/`) and the Python plugin (`packages/python-gitlint/gitlint_rai/rules.py`) build their patterns from the same key list and pattern template. `rai-signed-off-by` uses the same anchored-line strategy with a fixed `Signed-off-by` key. Parity tests in the Python suite (`test_pattern_parity_with_node_plugin`, `test_signoff_pattern_parity_with_node_plugin`) fail if the two sources drift.

One known nuance: the inputs differ slightly. Node validates the raw commit message, while gitlint strips comment lines and scissors content (`git commit -v` diffs) before rules run. A footer inside that stripped region counts for commitlint but not for gitlint.
The inputs differ slightly: gitlint strips comment lines and scissors content before running the Python rules. The Node rules ignore content after Git's scissors marker before checking the raw message, so discarded lines cannot satisfy either footer rule.
6 changes: 5 additions & 1 deletion packages/node-commitlint/src/rules/rai-footer-exists.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ const AI_ATTRIBUTION_PATTERN = new RegExp(
'i',
);

const SCISSORS_MARKER = /^[^\r\n] ------------------------ >8 ------------------------\r?$/m;

const VIOLATION_MESSAGE =
'Commit message must include AI attribution footer:\n' +
' 1. "Authored-by: [Human] <contact>" - Human only, no AI\n' +
Expand All @@ -35,7 +37,9 @@ const VIOLATION_MESSAGE =
' - "Generated-by: GitHub Copilot <copilot@github.com>"';

const raiFooterExists: Rule = (parsed) => {
const hasValidFooter = AI_ATTRIBUTION_PATTERN.test(parsed.raw ?? '');
const message = parsed.raw ?? '';
const marker = message.search(SCISSORS_MARKER);
const hasValidFooter = AI_ATTRIBUTION_PATTERN.test(marker === -1 ? message : message.slice(0, marker));
return hasValidFooter ? [true, ''] : [false, VIOLATION_MESSAGE];
};

Expand Down
6 changes: 5 additions & 1 deletion packages/node-commitlint/src/rules/rai-signed-off-by.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import type { Rule } from '@commitlint/types';
const SIGNED_OFF_BY_PATTERN =
/(?:^|\n)Signed-off-by:[ \t]+[^ \t<\r\n][^<\r\n]*(?<=[ \t])<[^>\r\n]+>\r?(?:\n|$)/i;

const SCISSORS_MARKER = /^[^\r\n] ------------------------ >8 ------------------------\r?$/m;

const VIOLATION_MESSAGE =
'Commit message must include a Signed-off-by footer:\n' +
' "Signed-off-by: Your Name <your.email@example.com>"\n' +
Expand All @@ -14,7 +16,9 @@ const VIOLATION_MESSAGE =
'`git commit -s` (or `--signoff`).';

const raiSignedOffBy: Rule = (parsed) => {
const hasSignOff = SIGNED_OFF_BY_PATTERN.test(parsed.raw ?? '');
const message = parsed.raw ?? '';
const marker = message.search(SCISSORS_MARKER);
const hasSignOff = SIGNED_OFF_BY_PATTERN.test(marker === -1 ? message : message.slice(0, marker));
return hasSignOff ? [true, ''] : [false, VIOLATION_MESSAGE];
};

Expand Down
26 changes: 26 additions & 0 deletions 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 { 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,29 @@ describe('commitlint integration', () => {
expect(result.errors[0].name).toBe('rai-signed-off-by');
expect(result.errors[0].message).toContain('Signed-off-by');
});

it('rejects both footers when they appear only after Git scissors', async () => {
const result = await lintMessage(
'feat: add a thing\n\n# ------------------------ >8 ------------------------\nGenerated-by: AI <ai@example.com>\nSigned-off-by: Jane Doe <jane@example.com>',
);
expect(result.valid).toBe(false);
expect(result.errors.map((error) => error.name)).toEqual([
'rai-footer-exists',
'rai-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.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);
});

});
17 changes: 17 additions & 0 deletions packages/node-commitlint/tests/rai-footer-exists.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
expect(isValid).toBe(true);
});

it('should pass with attribution anywhere in the message', () => {

Check warning on line 24 in packages/node-commitlint/tests/rai-footer-exists.test.ts

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Replace these 3 tests with a single Parameterized one.

See more on https://sonarcloud.io/project/issues?id=anchildress1_rai-lint&issues=AaDu-q4O1B6jItHtrcdK&open=AaDu-q4O1B6jItHtrcdK&pullRequest=140
const [isValid] = validate(
'feat: add feature\n\nGenerated-by: AI <ai@example.com>\n\nMore detail after.',
);
Expand All @@ -35,6 +35,23 @@
expect(isValid).toBe(true);
});

it.each(['#', ';'])(
'rejects attribution only after a Git scissors marker with %s comment character',
(commentChar) => {
const [isValid] = validate(
`feat: add feature\n\n${commentChar} ------------------------ >8 ------------------------\nGenerated-by: AI <ai@example.com>`,
);
expect(isValid).toBe(false);
},
);

it('accepts attribution before a CRLF scissors marker', () => {
const [isValid] = validate(
'feat: add feature\r\n\r\nGenerated-by: AI <ai@example.com>\r\n# ------------------------ >8 ------------------------\r\n',
);
expect(isValid).toBe(true);
});

it('should fail without AI attribution footer', () => {
const [isValid, message] = validate('feat: add new feature\n\nSome other footer');
expect(isValid).toBe(false);
Expand Down
14 changes: 14 additions & 0 deletions packages/node-commitlint/tests/rai-signed-off-by.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,20 @@ describe('rai-signed-off-by', () => {
expect(isValid).toBe(true);
});

it('rejects sign-off only after a Git scissors marker', () => {
const [isValid] = validate(
'feat: add feature\n\n# ------------------------ >8 ------------------------\nSigned-off-by: Jane Doe <jane@example.com>',
);
expect(isValid).toBe(false);
});

it('accepts sign-off before a Git scissors marker', () => {
const [isValid] = validate(
'feat: add feature\n\nSigned-off-by: Jane Doe <jane@example.com>\n# ------------------------ >8 ------------------------\n',
);
expect(isValid).toBe(true);
});

it('should fail without a Signed-off-by footer', () => {
const [isValid, message] = validate('feat: add feature\n\nSome other footer');
expect(isValid).toBe(false);
Expand Down
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