Skip to content

Commit 302c851

Browse files
mxaminclaude
andcommitted
Read code, not comments, in the shadow audit (#356)
The audit matched raw source lines, so a comment naming PyXmlSec_LxmlShadowBegin or ->_c_node — the comments that explain those very crossings — counted as the code itself. A binding could lose its Begin call and still pass the invariant because a comment beside it mentioned one, and a raw access inside an error string would have been reported as a real one. Comments and string/char literal bodies are now blanked (lines and columns preserved) before either rule is applied. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent 4e9e56b commit 302c851

1 file changed

Lines changed: 53 additions & 1 deletion

File tree

‎tests/test_shadow_audit.py‎

Lines changed: 53 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@
1111
2. raw node access (``->_c_node`` / ``->_c_doc``) appears only inside the functions listed in
1212
``RAW_ACCESS_ALLOWED``.
1313
14+
Comments and string literals are blanked first, so only real code counts for either rule.
15+
1416
Adding a function to either list is a deliberate design decision; see developer.md.
1517
"""
1618

@@ -52,11 +54,45 @@
5254
IS_ACTIVE = 'PyXmlSec_LxmlShadowIsActive()'
5355

5456

57+
def _blank_comments_and_literals(source: str) -> str:
58+
"""Blanks the body of every comment and string/char literal, keeping lines and columns.
59+
60+
The checks below match raw text, so without this a comment explaining ``->_c_node`` or
61+
naming a ``PyXmlSec_LxmlShadowBegin*`` helper would read as the code itself — a binding
62+
could lose its guard and still pass because of the comment describing it.
63+
"""
64+
out: list[str] = []
65+
i, n = 0, len(source)
66+
while i < n:
67+
pair = source[i : i + 2]
68+
if pair in ('//', '/*'):
69+
end = source.find('\n', i) if pair == '//' else source.find('*/', i + 2) + 2
70+
if end < 2: # unterminated: the rest of the file is comment
71+
end = n
72+
out.append(''.join('\n' if c == '\n' else ' ' for c in source[i:end]))
73+
i = end
74+
elif source[i] in '"\'':
75+
quote = source[i]
76+
out.append(quote)
77+
i += 1
78+
while i < n and source[i] != quote:
79+
step = 2 if source[i] == '\\' and i + 1 < n else 1
80+
out.append(''.join('\n' if c == '\n' else ' ' for c in source[i : i + step]))
81+
i += step
82+
if i < n:
83+
out.append(quote)
84+
i += 1
85+
else:
86+
out.append(source[i])
87+
i += 1
88+
return ''.join(out)
89+
90+
5591
def _functions(source: str) -> Iterator[tuple[str, list[str]]]:
5692
"""Yields (name, [lines]) for every PyXmlSec_* function defined in the C source."""
5793
name = None
5894
body: list[str] = []
59-
for line in source.splitlines():
95+
for line in _blank_comments_and_literals(source).splitlines():
6096
match = FUNCTION_DEF.match(line)
6197
if match and not line.rstrip().endswith(';'):
6298
if name is not None:
@@ -134,6 +170,22 @@ def test_checker_flags_a_raw_binding(self) -> None:
134170
self.assertIn('never calls a PyXmlSec_LxmlShadowBegin*', found[0])
135171
self.assertIn('outside the allowed functions', found[1])
136172

173+
def test_checker_reads_code_not_comments(self) -> None:
174+
# the guard named in a comment does not guard anything, and a mention in a literal is not access
175+
bad = (
176+
'static PyObject* PyXmlSec_Bad(PyObject* self, PyObject* args) {\n'
177+
' PyXmlSec_LxmlElementPtr node = NULL;\n'
178+
' if (!PyArg_ParseTuple(args, "O&:bad", PyXmlSec_LxmlElementConverter, &node)) return NULL;\n'
179+
' // PyXmlSec_LxmlShadowBegin(&shadow, node, "cannot copy.") belongs here\n'
180+
' /* and node->_c_node would then be shadow.root */\n'
181+
' PyErr_SetString(PyXmlSec_Error, "node->_c_node is not for xmlsec.");\n'
182+
' return NULL;\n'
183+
'}\n'
184+
)
185+
found = violations(bad, 'bad.c')
186+
self.assertEqual(1, len(found), found)
187+
self.assertIn('never calls a PyXmlSec_LxmlShadowBegin*', found[0])
188+
137189
def test_checker_flags_raw_access_before_the_switch(self) -> None:
138190
bad = (
139191
'static PyObject* PyXmlSec_TreeAddIds(PyObject* self, PyObject* args) {\n'

0 commit comments

Comments
 (0)