From c92269d6d889fd92270f615eb70e1f01b2dd60fd Mon Sep 17 00:00:00 2001 From: Simon van Lierde Date: Wed, 9 Sep 2026 15:58:20 +0000 Subject: [PATCH] fix(ci): use the suppression comment form CodeQL honors The in-code suppressions have never dismissed an alert: every dismissal in the repo was done by hand. CodeQL's shared AlertSuppression library matches `codeql[query-id]` only when the comment starts its own line, and then covers the line below it, so all nine trailing `# codeql[...]` comments were inert. - Rewrite the nine suppressions as trailing `# lgtm[query-id]`, the form that covers its own line, so the flagged expression is inside the annotated range. - Split the two multi-line logger calls in the email service: a trailing annotation on the line that opens a call covers no argument. - Say in the CodeQL config and the CI comment which form works and why. --- .github/codeql-config.yml | 8 ++++++-- .github/workflows/ci.yml | 2 +- .../app/api/auth/services/email/service.py | 20 +++++++++++++------ backend/app/api/auth/services/token_store.py | 2 +- backend/app/api/common/rate_limiting.py | 2 +- backend/app/core/secrets.py | 2 +- scripts/env_policy.py | 4 ++-- 7 files changed, 26 insertions(+), 14 deletions(-) diff --git a/.github/codeql-config.yml b/.github/codeql-config.yml index 01c35b99a..a4bb9619f 100644 --- a/.github/codeql-config.yml +++ b/.github/codeql-config.yml @@ -3,8 +3,12 @@ name: "Relab CodeQL Config" queries: - uses: security-and-quality -# Honor in-code `# codeql[query-id]` / `lgtm[query-id]` suppression comments for -# verified false positives (advanced setup does not include these by default). +# Honor in-code `# lgtm[query-id]` suppression comments for verified false positives +# (advanced setup does not include these queries by default). Write one as a trailing +# comment on the line holding the flagged expression, which for a multi-line call is the +# argument line, not the line that opens the call. The `# codeql[query-id]` spelling is +# matched only when the comment starts its own line, and then covers the line below it. +# See the shared AlertSuppression.qll in github/codeql for both rules. packs: python: - codeql/python-queries:AlertSuppression.ql diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a2a7b76bb..f7e2ffeb8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -560,7 +560,7 @@ jobs: # Code scanning stores but does not act on the suppression data that the # AlertSuppression queries (see codeql-config.yml) emit for in-code - # `# codeql[query-id]` comments; this official action closes the loop by + # `# lgtm[query-id]` comments; this official action closes the loop by # dismissing the matching alerts. Suppressions stay reviewable in diffs. # Default branch only: dismissal is a global alert property, and the action reads # default-branch alerts only. diff --git a/backend/app/api/auth/services/email/service.py b/backend/app/api/auth/services/email/service.py index d9ec4ecf5..7f1a7b4ab 100644 --- a/backend/app/api/auth/services/email/service.py +++ b/backend/app/api/auth/services/email/service.py @@ -108,17 +108,25 @@ async def _send_and_log(provider: EmailProvider, message: EmailMessage, log_labe await provider.send(message) except Exception: if attempt == _SEND_ATTEMPTS: - logger.exception( # codeql[py/clear-text-logging-sensitive-data] -- template filename, not a credential - "%s failed for %s after %d attempts", log_label, recipient, attempt + logger.exception( + "%s failed for %s after %d attempts", + log_label, # lgtm[py/clear-text-logging-sensitive-data] + recipient, # lgtm[py/clear-text-logging-sensitive-data] + attempt, ) return delay = _SEND_BACKOFF_SECONDS * 2 ** (attempt - 1) - logger.warning( # codeql[py/clear-text-logging-sensitive-data] -- template filename, not a credential - "%s attempt %d failed for %s, retrying in %.1fs", log_label, attempt, recipient, delay, exc_info=True + logger.warning( + "%s attempt %d failed for %s, retrying in %.1fs", + log_label, # lgtm[py/clear-text-logging-sensitive-data] + attempt, + recipient, # lgtm[py/clear-text-logging-sensitive-data] + delay, + exc_info=True, ) await anyio.sleep(delay) else: - logger.info("%s sent to %s", log_label, recipient) # codeql[py/clear-text-logging-sensitive-data] + logger.info("%s sent to %s", log_label, recipient) # lgtm[py/clear-text-logging-sensitive-data] return @@ -139,7 +147,7 @@ async def _dispatch( recipient = email_log_token(to_email) if background_tasks: background_tasks.add_task(_send_and_log, provider, message, log_label, recipient) - logger.info("%s queued for %s", log_label, recipient) # codeql[py/clear-text-logging-sensitive-data] + logger.info("%s queued for %s", log_label, recipient) # lgtm[py/clear-text-logging-sensitive-data] else: await _send_and_log(provider, message, log_label, recipient) diff --git a/backend/app/api/auth/services/token_store.py b/backend/app/api/auth/services/token_store.py index 5c549e3bc..5c926e368 100644 --- a/backend/app/api/auth/services/token_store.py +++ b/backend/app/api/auth/services/token_store.py @@ -15,7 +15,7 @@ def token_fingerprint(token: str) -> str: """Return a stable non-secret fingerprint for auth-token storage keys.""" # Fingerprints high-entropy random bearer tokens (secrets.token_urlsafe), not # passwords; a slow KDF adds nothing here. - return hashlib.sha256(token.encode("utf-8")).hexdigest() # codeql[py/weak-sensitive-data-hashing] + return hashlib.sha256(token.encode("utf-8")).hexdigest() # lgtm[py/weak-sensitive-data-hashing] def token_key(key_prefix: str, token: str) -> str: diff --git a/backend/app/api/common/rate_limiting.py b/backend/app/api/common/rate_limiting.py index b26d813d3..22e4bef7d 100644 --- a/backend/app/api/common/rate_limiting.py +++ b/backend/app/api/common/rate_limiting.py @@ -90,7 +90,7 @@ def hit_key(self, rate_string: str, key: str) -> None: if not allowed: # Safe to log: sensitive dimensions arrive as `prefix:`, never raw. - logger.info("Rate limit exceeded for bucket %s", key) # codeql[py/clear-text-logging-sensitive-data] + logger.info("Rate limit exceeded for bucket %s", key) # lgtm[py/clear-text-logging-sensitive-data] raise RateLimitExceededError def hit_request(self, rate_string: str, request: Request) -> None: diff --git a/backend/app/core/secrets.py b/backend/app/core/secrets.py index 01f6e0089..31dfd0882 100644 --- a/backend/app/core/secrets.py +++ b/backend/app/core/secrets.py @@ -59,7 +59,7 @@ def warn_on_placeholder_secrets(logger: logging.Logger, *settings_objects: BaseM "%s", rule, len(offenders), - "\n - ".join(offenders), # codeql[py/clear-text-logging-sensitive-data] -- field NAMES only, never values + "\n - ".join(offenders), # lgtm[py/clear-text-logging-sensitive-data] -- field NAMES only, never values rule, ) return offenders diff --git a/scripts/env_policy.py b/scripts/env_policy.py index eb21a4045..7847d906a 100755 --- a/scripts/env_policy.py +++ b/scripts/env_policy.py @@ -621,12 +621,12 @@ def main(argv: list[str] | None = None) -> int: sys.stdout.write("✅ Environment variable policy checks passed\n") elif args.command == "inventory": # Both suppressed writes below emit secret file NAMES from committed config, never values. - sys.stdout.write(format_inventory(load_secret_inventory())) # codeql[py/clear-text-logging-sensitive-data] + sys.stdout.write(format_inventory(load_secret_inventory())) # lgtm[py/clear-text-logging-sensitive-data] elif args.command == "validation-env": write_validation_env_file(args.path) elif args.command == "secrets-list": for name in compose_secret_names(load_json(args.config)): - sys.stdout.write(f"{name}\n") # codeql[py/clear-text-logging-sensitive-data] + sys.stdout.write(f"{name}\n") # lgtm[py/clear-text-logging-sensitive-data] elif args.command == "secrets-check": run_secrets_check(args.configs) elif args.command == "secrets-placeholder-check":