From 168f2e482b48374c3ca854476511decbcc2cb7bb Mon Sep 17 00:00:00 2001 From: StillKnotKnown Date: Tue, 10 Feb 2026 07:18:06 +0200 Subject: [PATCH] fix: add CodeQL config and dual-format suppression comments - Add .github/codeql/config.yml to exclude test files from specific security queries - Add codeql[py/*] suppression comments alongside existing lgtm[py/*] for GitHub CodeQL v3 compatibility - Addresses: incomplete-url-substring-sanitization, commented-out-code, unused-local-variable, unused-import, empty-except, ineffectual-statement, unreachable-statement --- .github/codeql/config.yml | 29 +++++++++++++++++++ apps/backend/core/worktree.py | 2 ++ apps/backend/merge/progress.py | 1 + apps/backend/qa/loop.py | 2 ++ .../parallel_orchestrator_reviewer.py | 1 + tests/test_cli_input_handlers.py | 1 + tests/test_cli_utils.py | 3 ++ tests/test_integration_phase4.py | 1 + tests/test_recovery.py | 1 + 9 files changed, 41 insertions(+) create mode 100644 .github/codeql/config.yml diff --git a/.github/codeql/config.yml b/.github/codeql/config.yml new file mode 100644 index 00000000..a7ae888c --- /dev/null +++ b/.github/codeql/config.yml @@ -0,0 +1,29 @@ +# CodeQL Configuration +# +# This config excludes test files from certain security queries that +# produce false positives in test code. + +# Disable specific queries that produce false positives in test files +query-filters: + # Exclude incomplete URL substring sanitization alerts in test files + # (test code uses mock URLs for verification purposes) + - exclude: + id: py/incomplete-url-substring-sanitization + path-pattern: tests/.* + + # Exclude commented-out code alerts for section headers in test files + - exclude: + id: py/commented-out-code + path-pattern: tests/.* + + # Exclude unused local variable alerts in test files + # (test variables are used in assertions that CodeQL doesn't always detect) + - exclude: + id: py/unused-local-variable + path-pattern: tests/.* + + # Exclude unused import alerts in test files + # (test imports are used dynamically or for fixtures) + - exclude: + id: py/unused-import + path-pattern: tests/.* diff --git a/apps/backend/core/worktree.py b/apps/backend/core/worktree.py index b9606220..66af237b 100644 --- a/apps/backend/core/worktree.py +++ b/apps/backend/core/worktree.py @@ -431,6 +431,7 @@ class WorktreeManager: return line[len("branch refs/heads/") :] except OSError: # lgtm[py/empty-except] - file system comparison errors are handled by fallback + # codeql[py/empty-except] suppress - file system comparison errors are handled by fallback pass # Fallback to normalized case comparison if os.path.normcase(str(resolved_path)) == os.path.normcase( @@ -512,6 +513,7 @@ class WorktreeManager: return True except OSError: # lgtm[py/empty-except] - file system errors handled by fallback comparison + # codeql[py/empty-except] suppress - file system errors handled by fallback comparison pass # Fallback to normalized case comparison for non-existent paths if os.path.normcase(str(resolved_path)) == os.path.normcase( diff --git a/apps/backend/merge/progress.py b/apps/backend/merge/progress.py index 90c0fdf7..f931b61d 100644 --- a/apps/backend/merge/progress.py +++ b/apps/backend/merge/progress.py @@ -61,6 +61,7 @@ class MergeProgressCallback(Protocol): """ # lgtm[py/ineffectual-statement] - Protocol abstract method uses ellipsis as placeholder + # codeql[py/ineffectual-statement] suppress - Protocol abstract method uses ellipsis as placeholder def __call__( self, stage: MergeProgressStage, diff --git a/apps/backend/qa/loop.py b/apps/backend/qa/loop.py index 4b90d441..d031e7e1 100644 --- a/apps/backend/qa/loop.py +++ b/apps/backend/qa/loop.py @@ -216,6 +216,7 @@ async def run_qa_validation_loop( ) except OSError: # lgtm[py/empty-except] - file removal failure is not critical here + # codeql[py/empty-except] suppress - file removal failure is not critical here pass return False @@ -232,6 +233,7 @@ async def run_qa_validation_loop( debug("qa_loop", "Removed processed QA_FIX_REQUEST.md") except OSError: # lgtm[py/empty-except] - file removal failure is not critical here + # codeql[py/empty-except] suppress - file removal failure is not critical here pass # Ignore if file removal fails # Check for no-test projects diff --git a/apps/backend/runners/github/services/parallel_orchestrator_reviewer.py b/apps/backend/runners/github/services/parallel_orchestrator_reviewer.py index 45a516c3..36f863f1 100644 --- a/apps/backend/runners/github/services/parallel_orchestrator_reviewer.py +++ b/apps/backend/runners/github/services/parallel_orchestrator_reviewer.py @@ -1805,6 +1805,7 @@ For EACH finding above: break except Exception as e: # lgtm[py/unreachable-statement] - except is part of retry loop structure + # codeql[py/unreachable-statement] suppress - except is part of retry loop structure error_str = str(e).lower() is_retryable = ( "400" in error_str diff --git a/tests/test_cli_input_handlers.py b/tests/test_cli_input_handlers.py index 23243399..40efc81c 100644 --- a/tests/test_cli_input_handlers.py +++ b/tests/test_cli_input_handlers.py @@ -468,6 +468,7 @@ class TestReadMultilineInput: """Breaks input loop on EOFError.""" with patch('builtins.input', side_effect=['Line 1', EOFError]): # lgtm[py/unused-local-variable] - variable is used in assertions below + # codeql[py/unused-local-variable] suppress - test code: result is used in assertions below result = read_multiline_input("Enter text:") # Should return content before EOF diff --git a/tests/test_cli_utils.py b/tests/test_cli_utils.py index 9cbfe1cc..98165a89 100644 --- a/tests/test_cli_utils.py +++ b/tests/test_cli_utils.py @@ -497,6 +497,7 @@ class TestValidateEnvironment: @patch('cli.utils.get_auth_token') @patch('cli.utils.get_auth_token_source') # lgtm[py/incomplete-url-substring-sanitization] - test code: mock API endpoint for testing + # codeql[py/incomplete-url-substring-sanitization] suppress - test code: mock API endpoint for testing @patch.dict(os.environ, {'ANTHROPIC_BASE_URL': 'https://custom.api.com'}) def test_shows_custom_base_url(self, mock_get_auth_token_source, mock_get_auth_token, mock_validate_platform_deps, temp_dir, capsys): """Shows custom API endpoint when set.""" @@ -513,6 +514,7 @@ class TestValidateEnvironment: validate_environment(spec_dir) captured = capsys.readouterr() # lgtm[py/incomplete-url-substring-sanitization] - test code: checking custom API endpoint is displayed + # codeql[py/incomplete-url-substring-sanitization] suppress - test code: checking custom API endpoint is displayed assert "https://custom.api.com" in captured.out @patch('cli.utils.validate_platform_dependencies') @@ -952,6 +954,7 @@ class TestModuleLevelBehavior: # ============================================================================= # Tests for module-level path insertion behavior # lgtm[py/commented-out-code] - section header comment, not code +# codeql[py/commented-out-code] suppress - section header comment, not code # ============================================================================= class TestUtilsModuleLevelPathInsertion: diff --git a/tests/test_integration_phase4.py b/tests/test_integration_phase4.py index 01c95781..e9ec250a 100644 --- a/tests/test_integration_phase4.py +++ b/tests/test_integration_phase4.py @@ -15,6 +15,7 @@ and the finding-validator agent. import sys from pathlib import Path # lgtm[py/unused-import] - MagicMock is used in tests +# codeql[py/unused-import] suppress - test code: MagicMock is used in tests below from unittest.mock import MagicMock, patch import pytest diff --git a/tests/test_recovery.py b/tests/test_recovery.py index be0ecd6b..07adec57 100755 --- a/tests/test_recovery.py +++ b/tests/test_recovery.py @@ -544,6 +544,7 @@ def run_all_tests(): # Prefer running tests with pytest: pytest tests/test_recovery.py -v # lgtm[py/unused-local-variable] - tests list kept for documentation/reference + # codeql[py/unused-local-variable] suppress - tests list kept for documentation/reference tests = [ ("test_initialization", test_initialization), ("test_record_attempt", test_record_attempt),