build(security): pre-commit guard for malformed bearer:disable directives + cleanup (#4821)
* build(security): add pre-commit guard for malformed bearer:disable directives
Bearer silently ignores a bearer:disable directive unless it is the bare rule
id on its own line directly above the statement. Two formats suppress nothing
yet look like protection: same-line directives, and trailing prose after the
rule id. This whole class of bug (PRs #4788/#4792/#4799 chased instances of it)
fails silently, so add a guard that makes it impossible to reintroduce.
New hook .pre-commit-hooks/check-bearer-disable.py (registered for src .py/.js):
- Python: uses tokenize, so docstrings/strings that merely mention a directive
are not flagged (only real # comments are checked).
- JavaScript: scans // line comments; block/JSDoc comments are out of scope.
- Flags same-line directives and trailing prose after the rule id; ignores
well-formed bare directives and prose mentions.
Unit tests in tests/security/test_bearer_disable_guard.py (12 tests).
Also cleans up the 16 pre-existing malformed directives the guard surfaced
(14 trailing-prose + 2 same-line in alembic_runner.py and xss-protection.js):
rationale moved to a preceding comment line, directive left as the bare rule
id. None was an open Bearer finding (all latent or globally-skipped rules);
changes are comment-only and behaviour-neutral. eslint clean; Bearer reports 0
enforced findings on the changed files.
* fix(security): harden bearer:disable guard after adversarial review
Adversarial review (3 agents, with Bearer ground-truth) found false positives
that would block valid commits and false negatives that let malformed
directives through. Hardened the guard:
False positives fixed:
- Python: a code-line comment that merely MENTIONS bearer:disable in prose
(e.g. 'execute(q) # no bearer:disable needed') was flagged as same-line.
Now the same-line branch requires an actual embedded '# bearer:disable'.
- JS: the line-based scanner flagged a directive inside a string literal
(help text/UI copy) and was confused by '//' inside strings. Replaced with
a char scanner that tracks strings/templates, so only real comments count.
- UTF-8 BOM made a valid own-line directive look same-line; check_file now
reads with utf-8-sig.
False negatives fixed:
- Lowercase trailing prose ('# bearer:disable rule because reasons') passed
because the rule-id pattern accepted bare words. Rule ids must now contain
an underscore (all real Bearer ids are namespaced) and multi-rule lists must
be comma-separated (Bearer's documented syntax).
- JS same-line directive on a line whose string held a '//' (a URL) slipped
past — fixed by the char scanner.
- Block-comment / JSDoc directives (which Bearer also ignores) were out of
scope; now detected. Cleaned up the one live instance (the module-level
JSDoc directive in xss-protection.js, which was redundant — the file's inline
// directives already suppress it; Bearer still reports 0 for the file).
Tests grown to 22 cases covering every false-positive and false-negative class
above. Guard reports 0 violations across all src; eslint clean; Bearer 0
enforced findings on the changed file.
* refactor(security): address AI review on bearer:disable guard
- Drop the dead `//` alternative from _DIRECTIVE_START: it is only used on
Python tokenize COMMENT tokens (always `#`), so the `//` branch was
unreachable. Make it `#`-only for clarity (JS uses _JS_LINE_DIRECTIVE).
- Document the template `${...}` interpolation limitation alongside the
existing regex-literal caveat. Properly parsing ${...} (with nested
templates/strings) adds real complexity and bug surface for an
extremely-rare case (no real directive is written inside ${...}), so it is
documented as accepted rather than handled.
No behavior change (the `//` branch never matched a `#` comment). 22 guard
tests pass; guard reports 0 violations across src.
This commit is contained in:
@@ -229,6 +229,14 @@ repos:
|
||||
files: \.js$
|
||||
exclude: (tests?/|spec\.|test\.|\.test\.|\.spec\.)
|
||||
description: "Ensure JS files use SafeLogger instead of raw console.log/error/warn"
|
||||
- id: check-bearer-disable
|
||||
name: Check bearer:disable directives are well-formed
|
||||
entry: .pre-commit-hooks/check-bearer-disable.py
|
||||
language: script
|
||||
files: ^src/local_deep_research/.*\.(py|js)$
|
||||
description: "Bearer ignores a bearer:disable directive unless it is the bare
|
||||
rule id on its own line above the statement; reject same-line or
|
||||
trailing-prose directives that would silently suppress nothing"
|
||||
- id: check-loguru-formatting
|
||||
name: Check loguru formatting style
|
||||
entry: .pre-commit-hooks/check-loguru-formatting.py
|
||||
|
||||
Executable
+234
@@ -0,0 +1,234 @@
|
||||
#!/usr/bin/env python3
|
||||
"""Pre-commit guard: Bearer ``bearer:disable`` directives must be well-formed.
|
||||
|
||||
Bearer SILENTLY ignores a suppression directive (the finding stays open) unless
|
||||
it is written exactly as:
|
||||
|
||||
* a line comment (``#`` in Python, ``//`` in JavaScript) on its OWN line,
|
||||
directly above the statement — a *same-line* trailing directive
|
||||
(``code() # bearer:disable rule``) is ignored; and
|
||||
* the bare rule id(s) only, with NO trailing prose — ``# bearer:disable rule
|
||||
-- why`` is ignored. Put the rationale on a separate comment line above the
|
||||
bare directive; and
|
||||
* NOT inside a block comment — Bearer ignores ``/* bearer:disable rule */``
|
||||
and JSDoc ``* bearer:disable rule`` too.
|
||||
|
||||
Each failure mode is silent, so a malformed directive looks like protection
|
||||
while suppressing nothing. This hook fails the commit when it finds one.
|
||||
|
||||
Implementation notes:
|
||||
* Python uses ``tokenize`` — directives mentioned inside docstrings or string
|
||||
literals are not flagged, only real ``#`` comments.
|
||||
* JavaScript uses a small char scanner that tracks strings/templates and
|
||||
block comments, so a ``//`` inside a string (e.g. a URL) is not mistaken
|
||||
for a comment. Not special-cased (rare edges, accepted): regex literals,
|
||||
and code inside template ``${...}`` interpolations (a directive written
|
||||
inside ``${...}`` is treated as template text and would be missed) — no
|
||||
real directive is written in either place.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import io
|
||||
import re
|
||||
import sys
|
||||
import tokenize
|
||||
|
||||
# A Bearer rule id is always namespaced with underscores, e.g.
|
||||
# python_lang_sql_injection / javascript_lang_dangerous_insert_html — never a
|
||||
# bare English word. Requiring an underscore stops lowercase prose
|
||||
# ("because reasons") from masquerading as a rule id.
|
||||
_RULE = r"[a-z][a-z0-9]*(?:_[a-z0-9]+)+"
|
||||
# After `bearer:disable`: one or more rule ids, comma-separated only (Bearer's
|
||||
# documented multi-rule syntax), and nothing else.
|
||||
_VALID_AFTER = re.compile(rf"^[ \t]+{_RULE}(?:[ \t]*,[ \t]*{_RULE})*[ \t]*$")
|
||||
# A Python `#` comment whose content begins with the directive.
|
||||
_DIRECTIVE_START = re.compile(r"^#[ \t]*bearer:disable\b(.*)$")
|
||||
# An embedded `# bearer:disable` (a directive trailing another comment).
|
||||
_EMBEDDED = re.compile(r"#[ \t]*bearer:disable\b")
|
||||
# A `// bearer:disable ...` line comment (used to read the bit after the rule).
|
||||
_JS_LINE_DIRECTIVE = re.compile(r"^//[ \t]*bearer:disable\b(.*)$")
|
||||
# A block-comment line that STARTS with the directive (after `/*` / JSDoc `*`),
|
||||
# vs. one that merely mentions it in prose.
|
||||
_BLOCK_DIRECTIVE_LINE = re.compile(
|
||||
r"^[ \t]*(?:/\*+|\*+)?[ \t]*bearer:disable\b"
|
||||
)
|
||||
|
||||
_SAME_LINE_MSG = (
|
||||
"same-line `bearer:disable` is silently ignored by Bearer — put the bare "
|
||||
"directive on its own line directly above the statement"
|
||||
)
|
||||
_TRAILING_MSG = (
|
||||
"trailing text after the rule id is silently ignored by Bearer — keep the "
|
||||
"directive line as the bare rule id and move the rationale to a separate "
|
||||
"comment line above it"
|
||||
)
|
||||
_BLOCK_MSG = (
|
||||
"`bearer:disable` in a block comment is silently ignored by Bearer — use a "
|
||||
"line comment (// or #) on its own line directly above the statement"
|
||||
)
|
||||
_MISSING_MSG = "`bearer:disable` is missing a rule id"
|
||||
|
||||
|
||||
def _after_violation(after: str) -> str | None:
|
||||
"""Validate the text that follows ``bearer:disable``."""
|
||||
if not after.strip():
|
||||
return _MISSING_MSG
|
||||
if not _VALID_AFTER.match(after):
|
||||
return _TRAILING_MSG
|
||||
return None
|
||||
|
||||
|
||||
def _check_python(content: str) -> list[tuple[int, str]]:
|
||||
errors: list[tuple[int, str]] = []
|
||||
lines = content.splitlines()
|
||||
try:
|
||||
tokens = list(tokenize.generate_tokens(io.StringIO(content).readline))
|
||||
except (tokenize.TokenError, IndentationError, SyntaxError):
|
||||
return errors # malformed files are caught by other tools
|
||||
for tok in tokens:
|
||||
if tok.type != tokenize.COMMENT or "bearer:disable" not in tok.string:
|
||||
continue
|
||||
row, col = tok.start
|
||||
code_before = (
|
||||
lines[row - 1][:col].strip() if row - 1 < len(lines) else ""
|
||||
)
|
||||
m = _DIRECTIVE_START.match(tok.string)
|
||||
if m:
|
||||
if code_before:
|
||||
errors.append((row, _SAME_LINE_MSG))
|
||||
else:
|
||||
msg = _after_violation(m.group(1))
|
||||
if msg:
|
||||
errors.append((row, msg))
|
||||
elif code_before and _EMBEDDED.search(tok.string):
|
||||
# A real directive trailing another comment on a code line, e.g.
|
||||
# `run(q) # noqa: S608 # bearer:disable rule`.
|
||||
errors.append((row, _SAME_LINE_MSG))
|
||||
# else: a comment that only mentions the text in prose — not a directive.
|
||||
return errors
|
||||
|
||||
|
||||
def _classify_js_line_comment(
|
||||
lineno: int, code_before: str, comment: str, errors: list[tuple[int, str]]
|
||||
) -> None:
|
||||
m = _JS_LINE_DIRECTIVE.match(comment)
|
||||
if not m:
|
||||
return # `// other prose ... bearer:disable ...` — not a directive
|
||||
if code_before.strip():
|
||||
errors.append((lineno, _SAME_LINE_MSG))
|
||||
else:
|
||||
msg = _after_violation(m.group(1))
|
||||
if msg:
|
||||
errors.append((lineno, msg))
|
||||
|
||||
|
||||
def _check_js(content: str) -> list[tuple[int, str]]:
|
||||
"""Char scanner: only `bearer:disable` reached as a real comment counts."""
|
||||
errors: list[tuple[int, str]] = []
|
||||
n = len(content)
|
||||
i = 0
|
||||
line = 1
|
||||
line_start = 0
|
||||
state = "code" # code | block | sq | dq | tmpl
|
||||
block_start_i = 0
|
||||
block_start_line = 0
|
||||
|
||||
def flag_block(text: str, start_line: int) -> None:
|
||||
# Flag only a block line that STARTS with the directive (a real
|
||||
# block-comment suppression), not prose that mentions it.
|
||||
for off, ln in enumerate(text.splitlines()):
|
||||
if _BLOCK_DIRECTIVE_LINE.match(ln):
|
||||
errors.append((start_line + off, _BLOCK_MSG))
|
||||
return
|
||||
|
||||
while i < n:
|
||||
ch = content[i]
|
||||
nxt = content[i + 1] if i + 1 < n else ""
|
||||
if ch == "\n":
|
||||
line += 1
|
||||
line_start = i + 1
|
||||
i += 1
|
||||
continue
|
||||
if state == "code":
|
||||
if ch == "/" and nxt == "/":
|
||||
eol = content.find("\n", i)
|
||||
if eol == -1:
|
||||
eol = n
|
||||
_classify_js_line_comment(
|
||||
line, content[line_start:i], content[i:eol], errors
|
||||
)
|
||||
i = eol
|
||||
elif ch == "/" and nxt == "*":
|
||||
state, block_start_i, block_start_line = "block", i, line
|
||||
i += 2
|
||||
elif ch == '"':
|
||||
state = "dq"
|
||||
i += 1
|
||||
elif ch == "'":
|
||||
state = "sq"
|
||||
i += 1
|
||||
elif ch == "`":
|
||||
state = "tmpl"
|
||||
i += 1
|
||||
else:
|
||||
i += 1
|
||||
elif state == "block":
|
||||
if ch == "*" and nxt == "/":
|
||||
flag_block(content[block_start_i : i + 2], block_start_line)
|
||||
state = "code"
|
||||
i += 2
|
||||
else:
|
||||
i += 1
|
||||
else: # sq | dq | tmpl
|
||||
quote = {"sq": "'", "dq": '"', "tmpl": "`"}[state]
|
||||
if ch == "\\":
|
||||
i += 2
|
||||
elif ch == quote:
|
||||
state = "code"
|
||||
i += 1
|
||||
else:
|
||||
i += 1
|
||||
if state == "block": # unterminated block comment
|
||||
flag_block(content[block_start_i:n], block_start_line)
|
||||
return errors
|
||||
|
||||
|
||||
def check_file(filename: str) -> list[tuple[int, str]]:
|
||||
try:
|
||||
# utf-8-sig strips a leading BOM so it is not mistaken for code.
|
||||
with open(filename, "r", encoding="utf-8-sig") as fh:
|
||||
content = fh.read()
|
||||
except (UnicodeDecodeError, OSError):
|
||||
return []
|
||||
if filename.endswith(".py"):
|
||||
return _check_python(content)
|
||||
if filename.endswith(".js"):
|
||||
return _check_js(content)
|
||||
return []
|
||||
|
||||
|
||||
def main(argv: list[str]) -> int:
|
||||
failed = False
|
||||
for filename in argv:
|
||||
errors = check_file(filename)
|
||||
if errors:
|
||||
failed = True
|
||||
print(f"\n{filename}:")
|
||||
for line_num, msg in sorted(errors):
|
||||
print(f" Line {line_num}: {msg}")
|
||||
if failed:
|
||||
print(
|
||||
"\n❌ Malformed `bearer:disable` directive(s). Bearer only honors a "
|
||||
"directive that is\n the bare rule id on its own line directly "
|
||||
"above the statement:\n"
|
||||
" # bearer:disable python_lang_sql_injection\n"
|
||||
" <statement>\n"
|
||||
" Put any rationale on separate comment line(s) above it."
|
||||
)
|
||||
return 1
|
||||
return 0
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
sys.exit(main(sys.argv[1:]))
|
||||
@@ -260,7 +260,8 @@ def _drop_orphan_alembic_temp_tables(conn: Connection) -> None:
|
||||
# parent table name from ``inspector.get_table_names()``; both
|
||||
# come from the database's own catalog and cannot contain
|
||||
# injection vectors.
|
||||
conn.exec_driver_sql(f'DROP TABLE IF EXISTS "{name}"') # noqa: S608 # bearer:disable python_lang_sql_injection
|
||||
# bearer:disable python_lang_sql_injection
|
||||
conn.exec_driver_sql(f'DROP TABLE IF EXISTS "{name}"') # noqa: S608
|
||||
|
||||
|
||||
def _disable_fk_for_migration(conn: Connection) -> None:
|
||||
|
||||
@@ -103,7 +103,8 @@ async function handleCreateCollection(e) {
|
||||
// Redirect to the new collection after a short delay
|
||||
setTimeout(() => {
|
||||
if (data.collection && data.collection.id) {
|
||||
// bearer:disable javascript_lang_open_redirect — server-generated ID in hardcoded /library/collections/ path
|
||||
// server-generated ID in hardcoded /library/collections/ path
|
||||
// bearer:disable javascript_lang_open_redirect
|
||||
window.location.href = `/library/collections/${data.collection.id}`;
|
||||
} else {
|
||||
window.location.href = '/library/collections';
|
||||
|
||||
@@ -1330,7 +1330,8 @@
|
||||
const viewResultsBtn = document.getElementById('view-results-btn');
|
||||
if (viewResultsBtn) {
|
||||
viewResultsBtn.addEventListener('click', () => {
|
||||
// bearer:disable javascript_lang_open_redirect — URLBuilder produces /results/{id}
|
||||
// URLBuilder produces /results/{id}
|
||||
// bearer:disable javascript_lang_open_redirect
|
||||
window.location.href = URLBuilder.resultsPage(researchId);
|
||||
});
|
||||
}
|
||||
@@ -1340,7 +1341,8 @@
|
||||
const viewJournalsBtn = document.getElementById('view-journals-btn');
|
||||
if (viewJournalsBtn) {
|
||||
viewJournalsBtn.addEventListener('click', () => {
|
||||
// bearer:disable javascript_lang_open_redirect — URLBuilder produces a same-origin path
|
||||
// URLBuilder produces a same-origin path
|
||||
// bearer:disable javascript_lang_open_redirect
|
||||
window.location.href = URLBuilder.journalQualityPage(researchId);
|
||||
});
|
||||
}
|
||||
|
||||
@@ -356,7 +356,8 @@
|
||||
}
|
||||
|
||||
// Pre-load logs if hash includes #logs
|
||||
// bearer:disable javascript_lang_observable_timing — timing comparison on URL hash, not secrets
|
||||
// timing comparison on URL hash, not secrets
|
||||
// bearer:disable javascript_lang_observable_timing
|
||||
if (window.location.hash === '#logs' && researchId) {
|
||||
SafeLogger.log('Auto-loading logs due to #logs in URL');
|
||||
setTimeout(() => {
|
||||
|
||||
@@ -2655,7 +2655,8 @@
|
||||
// Settings are saved to database via the API, not localStorage
|
||||
|
||||
// Redirect to the progress page
|
||||
// bearer:disable javascript_lang_open_redirect — URLBuilder produces /progress/{uuid}
|
||||
// URLBuilder produces /progress/{uuid}
|
||||
// bearer:disable javascript_lang_open_redirect
|
||||
window.location.href = URLBuilder.progressPage(data.research_id);
|
||||
} else {
|
||||
// Show error message
|
||||
|
||||
@@ -102,7 +102,8 @@
|
||||
loadingDiv.style.display = 'none';
|
||||
|
||||
if (!collections || collections.length === 0) {
|
||||
// bearer:disable javascript_lang_dangerous_insert_html — static HTML, no user data
|
||||
// static HTML, no user data
|
||||
// bearer:disable javascript_lang_dangerous_insert_html
|
||||
itemsDiv.innerHTML = `
|
||||
<div class="text-center text-muted py-3">
|
||||
<i class="fas fa-folder-open"></i>
|
||||
|
||||
@@ -326,7 +326,8 @@ class FollowUpResearch {
|
||||
|
||||
// Redirect to progress page to show the research is running.
|
||||
// Single navigation site; no concurrent writers to window.location.
|
||||
// bearer:disable javascript_lang_open_redirect — server-generated UUID in hardcoded /progress/ path
|
||||
// server-generated UUID in hardcoded /progress/ path
|
||||
// bearer:disable javascript_lang_open_redirect
|
||||
// eslint-disable-next-line require-atomic-updates
|
||||
window.location.href = `/progress/${data.research_id}`;
|
||||
} else {
|
||||
|
||||
@@ -383,7 +383,8 @@ function setupEventListeners() {
|
||||
} else {
|
||||
// URLValidator not available — fall back to safe internal path only
|
||||
SafeLogger.error('URLValidator not available — blocking external redirect');
|
||||
// bearer:disable javascript_lang_open_redirect — server-generated ID in hardcoded /results/ path
|
||||
// server-generated ID in hardcoded /results/ path
|
||||
// bearer:disable javascript_lang_open_redirect
|
||||
window.location.href = `/results/${item.research_id}`;
|
||||
}
|
||||
}
|
||||
@@ -500,7 +501,8 @@ async function performAdvancedNewsSearch(query, strategy = 'source-based', model
|
||||
showAlert('Authentication required. Please log in to perform research.', 'error');
|
||||
// Redirect to login after a short delay
|
||||
setTimeout(() => {
|
||||
// bearer:disable javascript_lang_open_redirect — hardcoded /auth/login target, next param is current page URL
|
||||
// hardcoded /auth/login target, next param is current page URL
|
||||
// bearer:disable javascript_lang_open_redirect
|
||||
window.location.href = '/auth/login?next=' + encodeURIComponent(window.location.href);
|
||||
}, 2000);
|
||||
return;
|
||||
@@ -600,7 +602,8 @@ function createSubscriptionFromItem(newsId) {
|
||||
research_id: item.research_id
|
||||
});
|
||||
|
||||
// bearer:disable javascript_lang_open_redirect — hardcoded /news path, only query params are dynamic
|
||||
// hardcoded /news path, only query params are dynamic
|
||||
// bearer:disable javascript_lang_open_redirect
|
||||
window.location.href = `/news/subscriptions/new?${params.toString()}`;
|
||||
}
|
||||
|
||||
|
||||
@@ -5,9 +5,10 @@
|
||||
* cross-site scripting (XSS) attacks when rendering dynamic content.
|
||||
* Uses DOMPurify for proven, security-reviewed HTML sanitization.
|
||||
*
|
||||
* bearer:disable javascript_lang_dangerous_insert_html - This module
|
||||
* intentionally provides HTML sanitization utilities. All innerHTML
|
||||
* operations use DOMPurify sanitization or escapeHtml encoding.
|
||||
* This module intentionally provides HTML sanitization utilities. All innerHTML
|
||||
* operations use DOMPurify sanitization or escapeHtml encoding, and each is
|
||||
* suppressed by an inline directive at its own call site (a module-level
|
||||
* suppression in this block comment would be ignored by the scanner anyway).
|
||||
*
|
||||
* ARCHITECTURE NOTE: Inline Fallback Pattern
|
||||
* ------------------------------------------
|
||||
@@ -108,7 +109,8 @@
|
||||
* @param {string} text - The text to escape for attribute context
|
||||
* @returns {string} - The escaped text safe for HTML attributes
|
||||
*/
|
||||
// bearer:disable javascript_lang_manual_html_sanitization - This IS the sanitization function
|
||||
// This IS the sanitization function
|
||||
// bearer:disable javascript_lang_manual_html_sanitization
|
||||
function escapeHtmlAttribute(text) {
|
||||
if (typeof text !== 'string') {
|
||||
text = String(text);
|
||||
@@ -125,7 +127,8 @@ function escapeHtmlAttribute(text) {
|
||||
* @param {string} content - The content to set (will be sanitized)
|
||||
* @param {boolean} allowHtmlTags - If true, allows basic HTML tags, otherwise escapes everything
|
||||
*/
|
||||
// bearer:disable javascript_lang_dangerous_insert_html - Content is sanitized by DOMPurify before insertion
|
||||
// Content is sanitized by DOMPurify before insertion
|
||||
// bearer:disable javascript_lang_dangerous_insert_html
|
||||
function safeSetInnerHTML(element, content, allowHtmlTags = false) {
|
||||
if (!element) {
|
||||
return;
|
||||
@@ -143,7 +146,7 @@ function safeSetInnerHTML(element, content, allowHtmlTags = false) {
|
||||
const sanitized = DOMPurify.sanitize(contentString, SANITIZE_CONFIG);
|
||||
// bearer:disable javascript_lang_dangerous_insert_html
|
||||
// eslint-disable-next-line no-unsanitized/property -- audited 2026-03-28: content already sanitized by DOMPurify.sanitize() above
|
||||
element.innerHTML = sanitized; // bearer:disable javascript_lang_dangerous_insert_html - Already sanitized by DOMPurify
|
||||
element.innerHTML = sanitized;
|
||||
} else if (allowHtmlTags) {
|
||||
// DOMPurify not available but HTML requested - escape all HTML for safety
|
||||
SafeLogger.warn('DOMPurify not available, escaping HTML instead of sanitizing');
|
||||
|
||||
@@ -162,7 +162,8 @@
|
||||
if (viewBtn && viewBtn.style.display !== 'none') {
|
||||
// Validate same-origin before navigating (URLValidator may not be loaded yet)
|
||||
if (viewBtn.href && viewBtn.href.startsWith(window.location.origin + '/')) {
|
||||
// bearer:disable javascript_lang_open_redirect — same-origin validated on preceding line
|
||||
// same-origin validated on preceding line
|
||||
// bearer:disable javascript_lang_open_redirect
|
||||
window.location.href = viewBtn.href;
|
||||
} else {
|
||||
SafeLogger.error('Blocked non-same-origin redirect in keyboard shortcut');
|
||||
|
||||
@@ -345,7 +345,8 @@
|
||||
const storedTheme = getCurrentTheme();
|
||||
|
||||
// Validate stored theme (in case localStorage was corrupted)
|
||||
// bearer:disable javascript_lang_observable_timing — timing comparison on public theme names, not secrets
|
||||
// timing comparison on public theme names, not secrets
|
||||
// bearer:disable javascript_lang_observable_timing
|
||||
const validatedTheme = VALID_THEMES.includes(storedTheme) ? storedTheme : 'hashed';
|
||||
if (validatedTheme !== storedTheme) {
|
||||
SafeLogger.warn('Invalid stored theme, resetting to hashed');
|
||||
|
||||
@@ -0,0 +1,191 @@
|
||||
"""Tests for the check-bearer-disable pre-commit guard.
|
||||
|
||||
The guard rejects `bearer:disable` directives that Bearer silently ignores:
|
||||
same-line directives and directives with trailing prose after the rule id.
|
||||
It must NOT flag well-formed directives or mere prose mentions.
|
||||
"""
|
||||
|
||||
# allow: no-sut-import — the SUT is a pre-commit hook script under
|
||||
# .pre-commit-hooks/, not a local_deep_research module; it is loaded via
|
||||
# importlib below.
|
||||
|
||||
import importlib.util
|
||||
from pathlib import Path
|
||||
|
||||
_HOOK = (
|
||||
Path(__file__).resolve().parents[2]
|
||||
/ ".pre-commit-hooks"
|
||||
/ "check-bearer-disable.py"
|
||||
)
|
||||
|
||||
|
||||
def _load():
|
||||
spec = importlib.util.spec_from_file_location("check_bearer_disable", _HOOK)
|
||||
mod = importlib.util.module_from_spec(spec)
|
||||
spec.loader.exec_module(mod)
|
||||
return mod
|
||||
|
||||
|
||||
guard = _load()
|
||||
|
||||
|
||||
# --- Python (#) -----------------------------------------------------------
|
||||
|
||||
|
||||
def test_py_valid_bare_directive_passes():
|
||||
src = "# bearer:disable python_lang_sql_injection\nx = 1\n"
|
||||
assert guard._check_python(src) == []
|
||||
|
||||
|
||||
def test_py_same_line_flagged():
|
||||
src = 'run(f"...{t}") # noqa: S608 # bearer:disable python_lang_sql_injection\n'
|
||||
errors = guard._check_python(src)
|
||||
assert len(errors) == 1
|
||||
assert "same-line" in errors[0][1]
|
||||
|
||||
|
||||
def test_py_trailing_prose_flagged():
|
||||
src = (
|
||||
"# bearer:disable python_lang_sql_injection -- because reasons\nx = 1\n"
|
||||
)
|
||||
errors = guard._check_python(src)
|
||||
assert len(errors) == 1
|
||||
assert "trailing text" in errors[0][1]
|
||||
|
||||
|
||||
def test_py_emdash_trailing_prose_flagged():
|
||||
src = (
|
||||
"# bearer:disable python_lang_sql_injection — because reasons\nx = 1\n"
|
||||
)
|
||||
assert len(guard._check_python(src)) == 1
|
||||
|
||||
|
||||
def test_py_docstring_mention_not_flagged():
|
||||
# A directive quoted inside a docstring must not be treated as real.
|
||||
src = (
|
||||
"def f():\n"
|
||||
' """Carries a ``# bearer:disable python_lang_sql_injection`` note."""\n'
|
||||
" return 1\n"
|
||||
)
|
||||
assert guard._check_python(src) == []
|
||||
|
||||
|
||||
def test_py_comment_prose_mention_not_flagged():
|
||||
# An own-line comment that mentions the directive in backticked prose.
|
||||
src = "# suppressed with ``# bearer:disable python_lang_sql_injection`` above\nx = 1\n"
|
||||
assert guard._check_python(src) == []
|
||||
|
||||
|
||||
def test_py_missing_rule_id_flagged():
|
||||
src = "# bearer:disable\nx = 1\n"
|
||||
errors = guard._check_python(src)
|
||||
assert len(errors) == 1
|
||||
assert "missing a rule id" in errors[0][1]
|
||||
|
||||
|
||||
# --- JavaScript (//) ------------------------------------------------------
|
||||
|
||||
|
||||
def test_js_valid_bare_directive_passes():
|
||||
src = "// bearer:disable javascript_lang_dangerous_insert_html\nel.innerHTML = x;\n"
|
||||
assert guard._check_js(src) == []
|
||||
|
||||
|
||||
def test_js_same_line_flagged():
|
||||
src = "el.innerHTML = x; // bearer:disable javascript_lang_dangerous_insert_html\n"
|
||||
errors = guard._check_js(src)
|
||||
assert len(errors) == 1
|
||||
assert "same-line" in errors[0][1]
|
||||
|
||||
|
||||
def test_js_trailing_prose_flagged():
|
||||
src = " // bearer:disable javascript_lang_open_redirect -- hardcoded path\nfoo();\n"
|
||||
errors = guard._check_js(src)
|
||||
assert len(errors) == 1
|
||||
assert "trailing text" in errors[0][1]
|
||||
|
||||
|
||||
def test_js_nested_prose_mention_not_flagged():
|
||||
src = " // see the // bearer:disable rule note above\nfoo();\n"
|
||||
assert guard._check_js(src) == []
|
||||
|
||||
|
||||
def test_js_url_with_double_slash_not_misflagged():
|
||||
src = ' const u = "https://example.com/x";\n'
|
||||
assert guard._check_js(src) == []
|
||||
|
||||
|
||||
# --- Hardening cases (from adversarial review) ----------------------------
|
||||
|
||||
|
||||
def test_py_prose_mention_on_code_line_not_flagged():
|
||||
# A code line whose comment merely mentions the text in prose (no embedded
|
||||
# `# bearer:disable`) must not be treated as a same-line directive.
|
||||
src = "cursor.execute(safe_q) # parametrized; no bearer:disable needed\n"
|
||||
assert guard._check_python(src) == []
|
||||
|
||||
|
||||
def test_py_bom_bare_directive_not_flagged(tmp_path):
|
||||
# A UTF-8 BOM must not make a valid own-line directive look same-line.
|
||||
# Exercises the real check_file() path (utf-8-sig read strips the BOM).
|
||||
p = tmp_path / "bom.py"
|
||||
p.write_bytes(
|
||||
b"\xef\xbb\xbf# bearer:disable python_lang_sql_injection\nx = 1\n"
|
||||
)
|
||||
assert guard.check_file(str(p)) == []
|
||||
|
||||
|
||||
def test_py_lowercase_trailing_prose_flagged():
|
||||
src = "# bearer:disable python_lang_sql_injection because reasons\nx = 1\n"
|
||||
assert len(guard._check_python(src)) == 1
|
||||
|
||||
|
||||
def test_py_comma_separated_rule_ids_pass():
|
||||
src = (
|
||||
"# bearer:disable python_lang_one_two, python_lang_three_four\nx = 1\n"
|
||||
)
|
||||
assert guard._check_python(src) == []
|
||||
|
||||
|
||||
def test_js_directive_in_string_literal_not_flagged():
|
||||
# A `// bearer:disable ...` inside a string is not a comment.
|
||||
src = 'const H = "use // bearer:disable rule_x_y to suppress";\n'
|
||||
assert guard._check_js(src) == []
|
||||
|
||||
|
||||
def test_js_same_line_with_url_in_string_flagged():
|
||||
# The `//` inside the URL string must not hide the genuine same-line dir.
|
||||
src = (
|
||||
'el.innerHTML = `<a href="https://x">${d}</a>`;'
|
||||
" // bearer:disable javascript_lang_dangerous_insert_html\n"
|
||||
)
|
||||
errors = guard._check_js(src)
|
||||
assert len(errors) == 1
|
||||
assert "same-line" in errors[0][1]
|
||||
|
||||
|
||||
def test_js_lowercase_trailing_prose_flagged():
|
||||
src = "// bearer:disable javascript_lang_dangerous_insert_html trusted input\nf();\n"
|
||||
assert len(guard._check_js(src)) == 1
|
||||
|
||||
|
||||
def test_js_block_comment_directive_flagged():
|
||||
src = "/* bearer:disable javascript_lang_dangerous_insert_html */\nf();\n"
|
||||
errors = guard._check_js(src)
|
||||
assert len(errors) == 1
|
||||
assert "block comment" in errors[0][1]
|
||||
|
||||
|
||||
def test_js_jsdoc_block_directive_flagged():
|
||||
src = "/**\n * bearer:disable javascript_lang_dangerous_insert_html\n */\nf();\n"
|
||||
errors = guard._check_js(src)
|
||||
assert len(errors) == 1
|
||||
assert "block comment" in errors[0][1]
|
||||
|
||||
|
||||
def test_js_block_comment_prose_mention_not_flagged():
|
||||
src = (
|
||||
"/**\n * suppressed by inline directives at call sites; module-level\n"
|
||||
" * would be ignored anyway.\n */\nf();\n"
|
||||
)
|
||||
assert guard._check_js(src) == []
|
||||
Reference in New Issue
Block a user