Skip to content

Commit 55bc48c

Browse files
committed
fix(redaction): measure literal spans over the snippet, not per line
A literal can open on one line and close on another. Spans were computed per line, so a marker on a literal's second line read as a comment and the rest of that line was starred -- dropping the closing quote, after which the literal pass no longer matched and the opening line's value survived. Spans are now measured once over the whole snippet and consulted by absolute position. Two things fall out of that. A snippet is a slice of a file, so a literal can also never close. The quoted value branch deferred to the literal pass, which never matches an unterminated literal, so `password = "hunter2` was left untouched. It now defers only when the quote opens a span the pass can find, and masks the value whole otherwise. A multi-line literal body was measured as one value, so the head-and-tail reveal exposed the start of its first line. Each line of such a body is now masked whole, with the line breaks kept so the snippet still shows where the literal begins and ends.
1 parent 63df31e commit 55bc48c

2 files changed

Lines changed: 92 additions & 46 deletions

File tree

‎socket_basics/core/utils/redaction.py‎

Lines changed: 65 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -197,78 +197,93 @@ def scrub_tokens(text: Any) -> str:
197197
return scrubbed
198198

199199

200-
def _split_assignment(line: str) -> 'tuple[str, str, str] | None':
201-
"""Split a line at the assignment operator that binds, if it has one.
200+
def _split_assignment(code: str, offset: int, in_literal) -> 'tuple[str, str, str] | None':
201+
"""Split a statement at the assignment operator that binds, if it has one.
202202
203203
Returns ``(head, value, trailing_whitespace)``, where ``head`` runs through
204204
the operator and any space after it. Operators covered by a string literal
205205
are skipped, and the last of the rest wins.
206206
"""
207-
spans = [m.span() for m in _STRING_LITERAL.finditer(line)]
208207
chosen = None
209-
for match in _ASSIGNMENT_OPERATOR.finditer(line):
210-
if any(start <= match.start() < end for start, end in spans):
208+
for match in _ASSIGNMENT_OPERATOR.finditer(code):
209+
if in_literal(offset + match.start()):
211210
continue
212211
chosen = match
213212
if chosen is None:
214213
return None
215214

216-
rest = line[chosen.end():]
215+
rest = code[chosen.end():]
217216
value = rest.lstrip()
218217
if not value:
219218
return None
220-
head = line[:chosen.end()] + rest[:len(rest) - len(value)]
219+
head = code[:chosen.end()] + rest[:len(rest) - len(value)]
221220
stripped = value.rstrip()
222221
return head, stripped, value[len(stripped):]
223222

224223

225-
def _split_statements(code: str) -> 'list[str]':
224+
def _split_statements(code: str, offset: int, in_literal) -> 'list[tuple[str, int]]':
226225
"""Split code on statement separators, keeping each separator as a piece.
227226
228227
One operator binds per statement, so a line carrying more than one has to be
229228
handled a statement at a time: in ``a = hunter2; password = x`` the last
230229
operator is the second, which would leave the first value in the head.
231-
A separator inside a string literal is part of the value.
232230
"""
233-
spans = [m.span() for m in _STRING_LITERAL.finditer(code)]
234-
pieces: 'list[str]' = []
231+
pieces: 'list[tuple[str, int]]' = []
235232
start = 0
236233
for separator in _STATEMENT_SEPARATOR.finditer(code):
237-
if any(span_start <= separator.start() < span_end
238-
for span_start, span_end in spans):
234+
if in_literal(offset + separator.start()):
239235
continue
240-
pieces.append(code[start:separator.start()])
241-
pieces.append(separator.group(0))
236+
pieces.append((code[start:separator.start()], offset + start))
237+
pieces.append((separator.group(0), offset + separator.start()))
242238
start = separator.end()
243-
pieces.append(code[start:])
239+
pieces.append((code[start:], offset + start))
244240
return pieces
245241

246242

247-
def _mask_statement(code: str) -> str:
243+
def _mask_statement(code: str, offset: int, in_literal) -> str:
248244
"""Mask the assigned value in a single statement."""
249-
split = _split_assignment(code)
245+
split = _split_assignment(code, offset, in_literal)
250246
if not split:
251247
return code
252248
head, value, trailing = split
253-
# A quoted value is the literal pass's job, and a value that opens with a
254-
# call is code rather than a credential: ``request.form.get('password')`` is
255-
# the finding, and starring it leaves nothing to act on.
256-
if value.startswith(('"', "'", '`')) or _CALL_EXPRESSION.match(value):
249+
250+
# A value that opens with a call is code rather than a credential:
251+
# ``request.form.get('password')`` is the finding, and starring it leaves
252+
# nothing to act on. A quoted value is the literal pass's job -- but only
253+
# where the quote opens a literal that pass can find. A snippet cut mid
254+
# string has an opening quote and no closing one, so nothing matches and
255+
# the value would survive untouched.
256+
opens_literal = value.startswith(('"', "'", '`')) and in_literal(offset + len(head))
257+
if opens_literal or _CALL_EXPRESSION.match(value):
257258
return code
259+
258260
# Anything else is masked whole. Where the value ends is unknowable here,
259261
# and measuring it together with what follows would reveal the head of a
260262
# short credential.
261263
return f"{head}{'*' * len(value)}{trailing}"
262264

263265

264-
def _mask_code(code: str) -> str:
266+
def _mask_code(code: str, offset: int, in_literal) -> str:
265267
"""Mask the assigned value in every statement on one line of code."""
266268
return ''.join(
267-
piece if piece == ';' else _mask_statement(piece)
268-
for piece in _split_statements(code)
269+
piece if piece == ';' else _mask_statement(piece, piece_offset, in_literal)
270+
for piece, piece_offset in _split_statements(code, offset, in_literal)
269271
)
270272

271273

274+
def _split_comment(line: str, offset: int, in_literal) -> 'tuple[str, str, str]':
275+
"""Split a line into code, comment marker and comment text.
276+
277+
The marker is kept so the masked line still reads as commented. A marker
278+
inside a string literal is part of the value, not a comment.
279+
"""
280+
for marker in _COMMENT_MARKER.finditer(line):
281+
if in_literal(offset + marker.start()):
282+
continue
283+
return line[:marker.start()], marker.group(0), line[marker.end():]
284+
return line, '', ''
285+
286+
272287
def redact_literals(text: Any) -> str:
273288
"""Mask the body of every string literal, keeping the surrounding syntax.
274289
@@ -290,37 +305,41 @@ def _mask_literal(match: 're.Match[str]') -> str:
290305
if not body:
291306
return match.group(0)
292307
quote = match.group('quote')
293-
return f'{quote}{mask_value(body)}{quote}'
308+
if '\n' in body:
309+
# A multi-line body is a block of content rather than one opaque
310+
# value, so the head-and-tail reveal would expose real text -- the
311+
# first line of it. Mask every line and keep the line breaks, so
312+
# the snippet still shows where the literal starts and ends.
313+
masked = '\n'.join('*' * len(segment) for segment in body.split('\n'))
314+
else:
315+
masked = mask_value(body)
316+
return f'{quote}{masked}{quote}'
317+
318+
# Literal spans are measured over the whole snippet, not line by line. A
319+
# literal can span lines, and a ``#`` or ``--`` on its second line is part
320+
# of the value; reading it as a comment and starring the rest of that line
321+
# can drop the closing quote, after which the literal pass no longer matches
322+
# and the opening line's credential survives.
323+
spans = [match.span() for match in _STRING_LITERAL.finditer(text)]
324+
325+
def in_literal(position: int) -> bool:
326+
return any(start <= position < end for start, end in spans)
294327

295-
# Mask unquoted assignments line by line before processing literals. A
296-
# quoted value is left for the literal pass, while an unquoted value is
297-
# masked even if another line or a trailing comment contains quoted text.
298328
masked_lines = []
329+
offset = 0
299330
for line in text.split('\n'):
300331
# The comment comes off first. Everything below reasons about where the
301332
# value ends, and a comment can hold anything the value can -- including
302333
# an operator later in the line than the real one, which would bind and
303334
# leave the credential sitting in the head.
304-
code, marker, comment = _split_comment(line)
305-
masked_lines.append(f"{_mask_code(code)}{marker}{'*' * len(comment)}")
335+
code, marker, comment = _split_comment(line, offset, in_literal)
336+
masked_lines.append(
337+
f"{_mask_code(code, offset, in_literal)}{marker}{'*' * len(comment)}"
338+
)
339+
offset += len(line) + 1
306340
return _STRING_LITERAL.sub(_mask_literal, '\n'.join(masked_lines))
307341

308342

309-
def _split_comment(line: str) -> 'tuple[str, str, str]':
310-
"""Split a line into code, comment marker and comment text.
311-
312-
The marker is kept so the masked line still reads as commented. A marker
313-
inside a string literal is part of the value, not a comment, so literal
314-
spans are skipped. Returns empty marker and text when there is no comment.
315-
"""
316-
spans = [m.span() for m in _STRING_LITERAL.finditer(line)]
317-
for marker in _COMMENT_MARKER.finditer(line):
318-
if any(start <= marker.start() < end for start, end in spans):
319-
continue
320-
return line[:marker.start()], marker.group(0), line[marker.end():]
321-
return line, '', ''
322-
323-
324343
def is_credential_finding(rule_id: Any, metadata: Mapping[str, Any] | None = None) -> bool:
325344
"""Report whether a rule's match is itself a credential.
326345

‎tests/test_secret_redaction.py‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -283,6 +283,33 @@ def test_the_comment_marker_survives_so_the_line_still_reads(self):
283283
)
284284

285285

286+
class TestMultilineAndUnterminatedLiterals:
287+
"""Literal spans are a property of the snippet, not of one line.
288+
289+
A snippet is a slice of a file, so a literal can open on one line and close
290+
on another, or never close at all.
291+
"""
292+
293+
TRIPLE_DOUBLE = 'PASSWORD = ' + '"' * 3 + 'hunter2\n# not a comment\n' + '"' * 3
294+
TRIPLE_SINGLE = 'SQL = ' + "'" * 3 + '\nSELECT hunter2 -- inline\n' + "'" * 3
295+
BACKTICK = 'password = `hunter2\n// js template\n`'
296+
297+
def test_a_marker_on_a_later_line_of_a_literal_is_not_a_comment(self):
298+
for snippet in (self.TRIPLE_DOUBLE, self.TRIPLE_SINGLE, self.BACKTICK):
299+
assert "hunter2" not in redact_literals(snippet), snippet
300+
301+
def test_an_unterminated_literal_is_masked_rather_than_deferred(self):
302+
# A snippet cut mid-string has an opening quote and no closing one, so
303+
# the literal pass never matches it. Deferring would leave it untouched.
304+
for snippet in ('password = "hunter2\n# broken', "password = 'hunter2\n-- sql"):
305+
assert "hunter2" not in redact_literals(snippet), snippet
306+
307+
def test_a_literal_spanning_lines_keeps_the_line_structure(self):
308+
redacted = redact_literals(self.TRIPLE_DOUBLE)
309+
assert redacted.count("\n") == 2
310+
assert "hunter2" not in redacted
311+
312+
286313
class TestCredentialRuleSelection:
287314
@pytest.mark.parametrize(
288315
"rule_id",

0 commit comments

Comments
 (0)