diff --git a/README.md b/README.md index 2764e9f..9889831 100644 --- a/README.md +++ b/README.md @@ -80,15 +80,35 @@ Available checks: * `signed-off` - `Signed-off-by:` trailer exists * `signature` - Verify GPG or SSH signature +### Scope validation + +By default any scope is accepted and scope is optional. Use `--scopes` to +restrict allowed values and `--require-scope` to enforce that a scope is always +present: + +```bash +# only allow known scopes +commit-guard --scopes auth,api,db + +# require a scope +commit-guard --require-scope + +# combine both +commit-guard --scopes auth,api --require-scope +``` + ### Configuration file Place `.commit-guard.toml` in your project root (or any parent directory) to -set defaults for `enable` and `disable`. commit-guard searches upward from the -working directory and uses the first file found. +set defaults for `enable`, `disable`, `scopes`, and `require-scope`. +commit-guard searches upward from the working directory and uses the first file +found. ```toml # .commit-guard.toml disable = ["signature", "body"] +scopes = ["auth", "api", "db"] +require-scope = true ``` ```toml @@ -96,8 +116,8 @@ disable = ["signature", "body"] enable = ["subject", "imperative"] ``` -CLI flags (`--enable`, `--disable`) take full precedence and ignore the config -file entirely when provided. +CLI flags (`--enable`, `--disable`, `--scopes`, `--require-scope`) take full +precedence and ignore config file values when provided. ### Checking a range of commits diff --git a/src/git_commit_guard/__init__.py b/src/git_commit_guard/__init__.py index f184eee..08d906a 100644 --- a/src/git_commit_guard/__init__.py +++ b/src/git_commit_guard/__init__.py @@ -119,7 +119,7 @@ def _strip_comments(message): ) -def check_subject(line, result): +def check_subject(line, result, allowed_scopes=frozenset(), *, require_scope=False): m = SUBJECT_RE.match(line) if not m: result.error(f"subject does not match 'type(scope): description': {line}") @@ -128,6 +128,12 @@ def check_subject(line, result): if m.group("type") not in TYPES: result.error(f"unknown type: {m.group('type')}") + scope = m.group("scope") + if require_scope and scope is None: + result.error("scope is required") + if allowed_scopes and scope is not None and scope not in allowed_scopes: + result.error(f"unknown scope: {scope}") + desc = m.group("desc") if desc[0].isupper(): result.error("description must not start with uppercase") @@ -213,6 +219,42 @@ class Args: rev: str | None message: str enabled: frozenset + allowed_scopes: frozenset + require_scope: bool + + +def _resolve_enabled(args, config, parser): + if args.enable or args.disable: + enabled = ( + frozenset(_parse_checks(parser, args.enable)) if args.enable else ALL_CHECKS + ) + if args.disable: + enabled = enabled - frozenset(_parse_checks(parser, args.disable)) + elif config.get("enable"): + enabled = frozenset(_parse_config_checks(config, "enable")) + elif config.get("disable"): + enabled = ALL_CHECKS - frozenset(_parse_config_checks(config, "disable")) + else: + enabled = ALL_CHECKS + return enabled + + +def _resolve_scopes(args, config): + if args.scopes: + allowed_scopes = frozenset(s.strip() for s in args.scopes.split(",")) + elif config.get("scopes"): + allowed_scopes = frozenset(config["scopes"]) + else: + allowed_scopes = frozenset() + + if args.require_scope: + require_scope = True + elif "require-scope" in config: + require_scope = config["require-scope"] + else: + require_scope = False + + return allowed_scopes, require_scope def _parse_checks(parser, value): @@ -237,21 +279,21 @@ def _parse_args(): metavar="CHECK[,CHECK,...]", help=f"skip these checks ({checks_list})", ) + parser.add_argument( + "--scopes", + metavar="SCOPE[,SCOPE,...]", + help="allowed scope values (any scope accepted if not set)", + ) + parser.add_argument( + "--require-scope", + action="store_true", + default=False, + help="require a scope in the subject line", + ) args = parser.parse_args() config = _load_config() - - if args.enable or args.disable: - enabled = ( - frozenset(_parse_checks(parser, args.enable)) if args.enable else ALL_CHECKS - ) - if args.disable: - enabled = enabled - frozenset(_parse_checks(parser, args.disable)) - elif config.get("enable"): - enabled = frozenset(_parse_config_checks(config, "enable")) - elif config.get("disable"): - enabled = ALL_CHECKS - frozenset(_parse_config_checks(config, "disable")) - else: - enabled = ALL_CHECKS + enabled = _resolve_enabled(args, config, parser) + allowed_scopes, require_scope = _resolve_scopes(args, config) if args.message_file: rev = None @@ -266,7 +308,13 @@ def _parse_args(): rev = "HEAD" message = _strip_comments(_get_message(rev)) - return Args(rev=rev, message=message, enabled=enabled) + return Args( + rev=rev, + message=message, + enabled=enabled, + allowed_scopes=allowed_scopes, + require_scope=require_scope, + ) def _report(result): @@ -290,7 +338,9 @@ def main(): desc = None if Check.SUBJECT in args.enabled: - desc = check_subject(lines[0], result) + desc = check_subject( + lines[0], result, args.allowed_scopes, require_scope=args.require_scope + ) if Check.IMPERATIVE in args.enabled: if desc is None: m = SUBJECT_RE.match(lines[0]) diff --git a/tests/test_git_commit_guard.py b/tests/test_git_commit_guard.py index 83a66df..56c7dfd 100644 --- a/tests/test_git_commit_guard.py +++ b/tests/test_git_commit_guard.py @@ -117,6 +117,36 @@ def test_subject_at_max_length(self): check_subject("fix: " + "a" * 67, r) # exactly 72 chars assert r.ok + def test_scope_in_allowlist_passes(self): + r = Result() + check_subject("fix(auth): add token", r, allowed_scopes=frozenset(["auth"])) + assert r.ok + + def test_scope_not_in_allowlist_fails(self): + r = Result() + check_subject("fix(api): add token", r, allowed_scopes=frozenset(["auth"])) + assert not r.ok + + def test_no_scope_with_allowlist_passes(self): + r = Result() + check_subject("fix: add token", r, allowed_scopes=frozenset(["auth"])) + assert r.ok + + def test_require_scope_without_scope_fails(self): + r = Result() + check_subject("fix: add token", r, require_scope=True) + assert not r.ok + + def test_require_scope_with_scope_passes(self): + r = Result() + check_subject("fix(auth): add token", r, require_scope=True) + assert r.ok + + def test_empty_allowlist_accepts_any_scope(self): + r = Result() + check_subject("fix(anything): add token", r, allowed_scopes=frozenset()) + assert r.ok + @pytest.mark.parametrize( "type_", [ @@ -522,6 +552,73 @@ def test_config_enable_applied(self, tmp_path): ): assert main() == 0 + def test_scopes_flag_valid(self, tmp_path): + f = tmp_path / "msg" + f.write_text("fix(auth): add token\n\nbody\n\nSigned-off-by: A User ") + argv = [ + "cg", + "--message-file", + str(f), + "--disable", + "signature", + "--scopes", + "auth,api", + ] + with patch("sys.argv", argv): + assert main() == 0 + + def test_scopes_flag_invalid(self, tmp_path): + f = tmp_path / "msg" + f.write_text("fix(db): add thing\n\nbody\n\nSigned-off-by: A User ") + argv = [ + "cg", + "--message-file", + str(f), + "--disable", + "signature", + "--scopes", + "auth,api", + ] + with patch("sys.argv", argv): + assert main() == 1 + + def test_require_scope_flag(self, tmp_path): + f = tmp_path / "msg" + f.write_text(_VALID_MSG) + argv = [ + "cg", + "--message-file", + str(f), + "--disable", + "signature", + "--require-scope", + ] + with patch("sys.argv", argv): + assert main() == 1 + + def test_scopes_from_config(self, tmp_path): + f = tmp_path / "msg" + f.write_text("fix(db): add thing\n\nbody\n\nSigned-off-by: A User ") + argv = ["cg", "--message-file", str(f), "--disable", "signature"] + with ( + patch("sys.argv", argv), + patch("git_commit_guard._load_config", return_value={"scopes": ["auth"]}), + ): + assert main() == 1 + + def test_require_scope_from_config(self, tmp_path): + f = tmp_path / "msg" + f.write_text(_VALID_MSG) + argv = ["cg", "--message-file", str(f), "--disable", "signature"] + with ( + patch("sys.argv", argv), + patch( + "git_commit_guard._load_config", + return_value={"require-scope": True}, + ), + ): + assert main() == 1 + def test_cli_overrides_config(self, tmp_path): f = tmp_path / "msg" f.write_text(_VALID_MSG)