From b2ee8cb1b9fe73cbbdb7400e543a8f6512c1ab97 Mon Sep 17 00:00:00 2001 From: Mihir Kandoi Date: Sun, 21 Jun 2026 17:10:34 +0530 Subject: [PATCH] ci(postgres): fix semgrep + two review findings in the checker - semgrep: annotate the source-reading open() with # nosemgrep for the frappe-security-file-traversal rule (dev-only lint tool; path comes from pre-commit, not user input). - bool-scan: only inspect the field *value* arg (db_set args[1]/dict args[0]; set_value args[3]/dict args[2]) so a positional update_modified=False (e.g. db_set('f', 0, False)) no longer false-positives. - # pg-ok: also honour the annotation on a multi-line call's closing paren line (scan one line past the node's end). --- .github/helper/postgres_compat.py | 29 ++++++++++++++++++++--------- 1 file changed, 20 insertions(+), 9 deletions(-) diff --git a/.github/helper/postgres_compat.py b/.github/helper/postgres_compat.py index e5fc0578c7a..a02e82eb953 100755 --- a/.github/helper/postgres_compat.py +++ b/.github/helper/postgres_compat.py @@ -92,10 +92,10 @@ class Visitor(ast.NodeVisitor): def _ignored(self, node: ast.AST) -> bool: start = getattr(node, "lineno", 1) end = getattr(node, "end_lineno", start) or start - # honour `# pg-ok` anywhere on the node's line span, or on the line just above - # (the enclosing call, e.g. `frappe.db.sql( # pg-ok`). + # honour `# pg-ok` anywhere on the node's line span, the line just above (the enclosing + # call, e.g. `frappe.db.sql( # pg-ok`), or the line just below (a multi-line call's `) # pg-ok`). lo = max(0, start - 2) - return any(IGNORE in self.lines[i] for i in range(lo, min(end, len(self.lines)))) + return any(IGNORE in self.lines[i] for i in range(lo, min(end + 1, len(self.lines)))) def _flag(self, node: ast.AST, msg: str) -> None: if not self._ignored(node): @@ -134,15 +134,25 @@ class Visitor(ast.NodeVisitor): if name == "get" and node.args and isinstance(node.args[0], ast.Constant) and node.args[0].value in MYSQL_RESULT_KEYS: self._flag(node, f'"{node.args[0].value}" is a MySQL SHOW INDEX result key -> use frappe.db.has_index()/get_column_index()') - # set_value(..., True) / db_set("field", True) on a Check (int) column + # set_value(..., True) / db_set("field", True) on a Check (int) column. + # Only the field *value* arg carries bool->smallint risk — NOT trailing flags like + # update_modified. db_set(field, value, update_modified, ...) -> value at args[1] (or a dict + # at args[0]); set_value(dt, dn, field, value, ...) -> value at args[3] (or a dict at args[2]). if name in SET_BOOL_FUNCS: - for a in node.args: + value_idx, dict_idx = (1, 0) if name == "db_set" else (3, 2) + dict_arg = ( + node.args[dict_idx] + if len(node.args) > dict_idx and isinstance(node.args[dict_idx], ast.Dict) + else None + ) + if dict_arg is not None: + for v in dict_arg.values: + if isinstance(v, ast.Constant) and isinstance(v.value, bool): + self._flag(node, f"{name}(...) sets an int/Check column with a bool in a dict -> pass 1/0 (Postgres rejects bool->smallint)") + elif len(node.args) > value_idx: + a = node.args[value_idx] if isinstance(a, ast.Constant) and isinstance(a.value, bool): self._flag(node, f"{name}(..., {a.value}) sets an int/Check column with a bool -> pass 1/0 (Postgres rejects bool->smallint)") - elif isinstance(a, ast.Dict): - for v in a.values: - if isinstance(v, ast.Constant) and isinstance(v.value, bool): - self._flag(node, f"{name}(...) sets an int/Check column with a bool in a dict -> pass 1/0 (Postgres rejects bool->smallint)") self.generic_visit(node) @@ -155,6 +165,7 @@ class Visitor(ast.NodeVisitor): def check_file(path: str) -> list[str]: try: + # nosemgrep: frappe-semgrep-rules.rules.security.frappe-security-file-traversal -- dev-only lint tool; `path` is a source file supplied by pre-commit, not user input src = open(path, encoding="utf-8").read() except (OSError, UnicodeDecodeError): return []