From 7a93dd59d99d8e0ee3469ba65f60b6f977032d48 Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Wed, 22 Apr 2026 15:25:10 +0200 Subject: [PATCH] cfengine format: Don't reformat policy files with syntax errors Co-authored-by: Claude Opus 4.6 (1M context) Signed-off-by: Ole Herman Schumacher Elgesem --- src/cfengine_cli/commands.py | 13 +++++++++- src/cfengine_cli/format.py | 18 ++++++++++---- src/cfengine_cli/lint.py | 42 +++++++++++++++++++++++++++++++++ tests/shell/004-format-check.sh | 26 ++++++++++++++++++++ 4 files changed, 94 insertions(+), 5 deletions(-) diff --git a/src/cfengine_cli/commands.py b/src/cfengine_cli/commands.py index ec3aec9..f7f45d8 100644 --- a/src/cfengine_cli/commands.py +++ b/src/cfengine_cli/commands.py @@ -4,7 +4,7 @@ import json from cfengine_cli.profile import profile_cfengine, generate_callstack from cfengine_cli.dev import dispatch_dev_subcommand -from cfengine_cli.lint import lint_args +from cfengine_cli.lint import lint_args, PolicySyntaxError from cfengine_cli.shell import user_command from cfengine_cli.paths import bin from cfengine_cli.version import cfengine_cli_version_string @@ -51,6 +51,9 @@ def deploy() -> int: def _format_filename(filename: str, line_length: int, check: bool) -> int: + """Format a single file. + + Raises PolicySyntaxError for .cf files with syntax errors.""" if filename.startswith("./."): return 0 if filename.endswith(".json"): @@ -70,6 +73,14 @@ def _format_dirname(directory: str, line_length: int, check: bool) -> int: def format(names, line_length, check) -> int: + try: + return _format_inner(names, line_length, check) + except PolicySyntaxError as e: + print(f"Error: {e}") + return 1 + + +def _format_inner(names, line_length, check) -> int: if not names: return _format_dirname(".", line_length, check) if len(names) == 1 and names[0] == "-": diff --git a/src/cfengine_cli/format.py b/src/cfengine_cli/format.py index a111fce..46744b7 100644 --- a/src/cfengine_cli/format.py +++ b/src/cfengine_cli/format.py @@ -7,6 +7,7 @@ import tree_sitter_cfengine as tscfengine from tree_sitter import Language, Parser, Node from cfbs.pretty import pretty_file, pretty_check_file +from cfengine_cli.lint import check_policy_syntax # Node types that increase indentation by 2 when entered INDENTED_TYPES = { @@ -778,19 +779,23 @@ def format_policy_file(filename: str, line_length: int, check: bool) -> int: """Format a .cf policy file in place, writing only if content changed. Returns 0 in case of successful reformat or no reformat needed. - Returns 1 when check is True and reformat is needed.""" + Returns 1 when check is True and reformat is needed. + Raises PolicySyntaxError when the file has syntax errors.""" assert filename.endswith(".cf") PY_LANGUAGE = Language(tscfengine.language()) parser = Parser(PY_LANGUAGE) - fmt = Formatter() with open(filename, "rb") as f: original_data = f.read() tree = parser.parse(original_data) root_node = tree.root_node assert root_node.type == "source_file" + + check_policy_syntax(tree, filename) + + fmt = Formatter() autoformat(root_node, fmt, line_length) new_data = fmt.buffer + "\n" @@ -808,16 +813,21 @@ def format_policy_file(filename: str, line_length: int, check: bool) -> int: def format_policy_fin_fout( fin: IO[str], fout: IO[str], line_length: int, check: bool ) -> int: - """Format CFEngine policy read from fin, writing the result to fout.""" + """Format CFEngine policy read from fin, writing the result to fout. + + Raises PolicySyntaxError when the input has syntax errors.""" PY_LANGUAGE = Language(tscfengine.language()) parser = Parser(PY_LANGUAGE) - fmt = Formatter() original_data = fin.read().encode("utf-8") tree = parser.parse(original_data) root_node = tree.root_node assert root_node.type == "source_file" + + check_policy_syntax(tree, "") + + fmt = Formatter() autoformat(root_node, fmt, line_length) new_data = fmt.buffer + "\n" diff --git a/src/cfengine_cli/lint.py b/src/cfengine_cli/lint.py index 84aadd6..9c3944c 100644 --- a/src/cfengine_cli/lint.py +++ b/src/cfengine_cli/lint.py @@ -1035,6 +1035,48 @@ def _lint_json_selector(file: str) -> int: return _lint_json_plain(file) +# --------------------------------------------------------------------------- +# Syntax error detection (used by both linter and formatter) +# --------------------------------------------------------------------------- + + +def _find_first_error(node: Node) -> Node | None: + """Find the first ERROR node in the tree, or None if the tree is valid.""" + if node.type == "ERROR": + return node + for child in node.children: + found = _find_first_error(child) + if found: + return found + return None + + +class PolicySyntaxError(Exception): + """Raised when a policy file has syntax errors and cannot be formatted.""" + + def __init__(self, filename: str, line: int, column: int): + self.filename = filename + self.line = line + self.column = column + super().__init__(f"Syntax error in '{filename}' at {filename}:{line}:{column}") + + +def check_policy_syntax(tree: Tree, filename: str) -> None: + """Check a parsed tree for syntax errors. + + Raises PolicySyntaxError if an ERROR node is found. + + Only checks for ERROR nodes, not MISSING nodes — missing tokens like + semicolons are handled gracefully by the formatter.""" + root_node = tree.root_node + error_node = _find_first_error(root_node) + if not error_node: + return + line = error_node.range.start_point[0] + 1 + column = error_node.range.start_point[1] + 1 + raise PolicySyntaxError(filename, line, column) + + # Interface: These are the functions we want to be called from outside # They create State() and should not be called recursively inside lint.py diff --git a/tests/shell/004-format-check.sh b/tests/shell/004-format-check.sh index f05acc4..4b2ee54 100755 --- a/tests/shell/004-format-check.sh +++ b/tests/shell/004-format-check.sh @@ -39,3 +39,29 @@ if cfengine format --check "$tmpdir/bad2.cf"; then fi # Verify the file was NOT modified diff "$tmpdir/bad2_orig.cf" "$tmpdir/bad2.cf" + +# Case 5: format a file with syntax errors -> exit 1, file NOT modified +printf 'bundle agent { invalid syntax\n' > "$tmpdir/syntax_error.cf" +cp "$tmpdir/syntax_error.cf" "$tmpdir/syntax_error_orig.cf" +if cfengine format "$tmpdir/syntax_error.cf"; then + echo "FAIL: expected exit code 1 for file with syntax errors" + exit 1 +fi +# Verify the file was NOT modified +diff "$tmpdir/syntax_error_orig.cf" "$tmpdir/syntax_error.cf" + +# Case 6: format from stdin with syntax errors -> exit 1 +if printf 'bundle agent { invalid syntax\n' | cfengine format -; then + echo "FAIL: expected exit code 1 for stdin with syntax errors" + exit 1 +fi + +# Case 7: --check on file with syntax errors -> exit 1, file NOT modified +printf 'bundle agent { invalid syntax\n' > "$tmpdir/syntax_error2.cf" +cp "$tmpdir/syntax_error2.cf" "$tmpdir/syntax_error2_orig.cf" +if cfengine format --check "$tmpdir/syntax_error2.cf"; then + echo "FAIL: expected exit code 1 for --check on file with syntax errors" + exit 1 +fi +# Verify the file was NOT modified +diff "$tmpdir/syntax_error2_orig.cf" "$tmpdir/syntax_error2.cf"