Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 44 additions & 0 deletions .github/codeql/suppressions.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
{
"note": [
"Reviewed CodeQL findings that are false positives and are re-raised every time",
"the surrounding file is edited, because a shifted line number gives the alert a",
"new fingerprint and GitHub does not carry the dismissal across.",
"",
"Rust has no inline `// codeql[rule]` suppression: the AlertSuppression.ql query",
"that implements it exists for C/C++, C#, Go, Java, JS, Python, Ruby and Swift,",
"but not Rust. codeql-config.yml cannot help either \u2014 `paths-ignore` would drop",
"every rule on the file, and `query-filters` would drop the rule on every file.",
"So the dismissal is replayed by .github/workflows/codeql-suppress.yml instead.",
"",
"An entry is anchored to the SINK TEXT, never to a line number: `sink` must match",
"the source line the alert points at. A rule firing on a different expression in",
"the same file stays open and needs its own review. Keep `reason` under 280 chars",
"\u2014 that is the GitHub API's hard cap on a dismissal comment."
],
"suppressions": [
{
"rule": "rust/path-injection",
"path": "nodedb-wal/src/segment/atomic_io.rs",
"sink": "^\\s*(let mut f = fs::File::create\\(&tmp\\)|fs::rename\\(&(tmp, &dst|live, &backup|staged, &live)\\))",
"reason": "checked_name() runs is_plain_path_component on every name at the top of the function, before any path is built: rejects / \\ : NUL . .. leading/trailing dot-space and control chars, returning InvalidInput before any filesystem call. Both paths join the same caller dir."
},
{
"rule": "rust/path-injection",
"path": "nodedb/src/control/server/shared/ddl/neutral/timeseries/rewrite.rs",
"sink": "^\\s*let _ = std::fs::remove_dir_all\\(ts_base\\.join\\(&backup_name\\)\\);",
"reason": "dir_name is validated with is_plain_path_component immediately above this join (rejects / \\ : NUL . .. leading/trailing dot-space and control chars); a failing name is skipped with a warning before any path is built. backup_name only appends .old to that validated component."
},
{
"rule": "rust/path-injection",
"path": "nodedb-wal/src/segment/atomic_io.rs",
"sink": "^\\s*let dir_file = fs::File::open\\(dir\\)",
"reason": "fsync_directory opens the caller's own directory read-only and calls sync_all on the fd. It appends nothing to the path, so it reaches no resource the caller did not already hold. Every name joined onto that directory is validated by checked_name at its own entry point."
},
{
"rule": "rust/path-injection",
"path": "nodedb/src/control/server/shared/ddl/neutral/timeseries/rewrite.rs",
"sink": "^\\s*let partition_dir = ts_base\\.join\\(dir_name\\);",
"reason": "dir_name is validated with is_plain_path_component immediately above this join (rejects / \\ : NUL . .. leading/trailing dot-space and control chars); a failing name is skipped with a warning before any path is built, so the join stays one level under ts_base."
}
]
}
134 changes: 134 additions & 0 deletions .github/scripts/codeql_suppress.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,134 @@
#!/usr/bin/env python3
"""Replay reviewed CodeQL dismissals that GitHub drops when line numbers shift.

An alert is dismissed only when its rule id, file path, and the *text* of the
line it points at all match an entry in the suppression list. Anchoring on the
sink text rather than the line number is the point: a shifted line is what
re-raises the alert, and a genuinely new sink in the same file will not match
an existing entry and stays open.

Exit code is 0 when every open alert is either dismissed or reported; the
report goes to the workflow step summary so unreviewed alerts stay visible.
"""

import json
import os
import re
import sys
import urllib.error
import urllib.request

API = "https://api.github.com"
REASON_LIMIT = 280 # GitHub's cap on dismissed_comment.


def call(method, path, token, body=None):
data = json.dumps(body).encode() if body is not None else None
req = urllib.request.Request(f"{API}{path}", data=data, method=method)
req.add_header("Authorization", f"Bearer {token}")
req.add_header("Accept", "application/vnd.github+json")
req.add_header("X-GitHub-Api-Version", "2022-11-28")
if data is not None:
req.add_header("Content-Type", "application/json")
with urllib.request.urlopen(req) as resp:
return json.loads(resp.read() or b"null")


def open_alerts(repo, token, ref):
out, page = [], 1
while True:
batch = call(
"GET",
f"/repos/{repo}/code-scanning/alerts"
f"?state=open&ref={ref}&per_page=100&page={page}",
token,
)
if not batch:
return out
out.extend(batch)
page += 1


def source_line(path, line_no):
try:
with open(path, encoding="utf-8", errors="replace") as f:
for i, line in enumerate(f, 1):
if i == line_no:
return line.rstrip("\n")
except OSError:
return None
return None


def match(alert, rules):
loc = alert["most_recent_instance"]["location"]
path, line_no = loc["path"], loc["start_line"]
text = source_line(path, line_no)
if text is None:
return None
for rule in rules:
if rule["rule"] != alert["rule"]["id"] or rule["path"] != path:
continue
if re.search(rule["sink"], text):
return rule
return None


def main():
token = os.environ["GITHUB_TOKEN"]
repo = os.environ["GITHUB_REPOSITORY"]
ref = os.environ.get("TARGET_REF", "refs/heads/main")
dry_run = os.environ.get("DRY_RUN") == "true"

with open(".github/codeql/suppressions.json", encoding="utf-8") as f:
rules = json.load(f)["suppressions"]

for rule in rules:
if len(rule["reason"]) > REASON_LIMIT:
sys.exit(
f"suppressions.json: reason for {rule['rule']} on {rule['path']} is "
f"{len(rule['reason'])} chars; the API rejects anything over {REASON_LIMIT}"
)
re.compile(rule["sink"])

dismissed, unmatched = [], []
for alert in open_alerts(repo, token, ref):
loc = alert["most_recent_instance"]["location"]
where = f"{alert['rule']['id']} {loc['path']}:{loc['start_line']}"
rule = match(alert, rules)
if rule is None:
unmatched.append(f"#{alert['number']} {where}")
continue
if not dry_run:
call(
"PATCH",
f"/repos/{repo}/code-scanning/alerts/{alert['number']}",
token,
{
"state": "dismissed",
"dismissed_reason": "false positive",
"dismissed_comment": rule["reason"],
},
)
dismissed.append(f"#{alert['number']} {where}")

report = ["## CodeQL suppression replay", ""]
verb = "Would dismiss" if dry_run else "Dismissed"
report.append(f"{verb} {len(dismissed)} re-raised alert(s):")
report += [f"- `{d}`" for d in dismissed] or ["- none"]
report += ["", f"Open and unreviewed — {len(unmatched)} alert(s):"]
report += [f"- `{u}`" for u in unmatched] or ["- none"]

text = "\n".join(report)
print(text)
summary = os.environ.get("GITHUB_STEP_SUMMARY")
if summary:
with open(summary, "a", encoding="utf-8") as f:
f.write(text + "\n")


if __name__ == "__main__":
try:
main()
except urllib.error.HTTPError as e:
sys.exit(f"GitHub API {e.code}: {e.read().decode(errors='replace')}")
48 changes: 48 additions & 0 deletions .github/workflows/codeql-suppress.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
# Replays reviewed CodeQL dismissals that GitHub drops when line numbers shift.
#
# GitHub fingerprints an alert partly by location, so editing a file re-raises
# every already-dismissed finding in it as a new alert. Rust has no inline
# `// codeql[rule]` suppression to pin them with, so the dismissal is replayed
# here from .github/codeql/suppressions.json, anchored to the sink text.
#
# Runs after CodeQL finishes on main, so the replay lands on the alerts that
# run just created. Alerts that match no entry are left open and listed in the
# step summary.

name: CodeQL suppression replay

on:
workflow_run:
workflows: ["CodeQL"]
types: [completed]
branches: [main]
workflow_dispatch:
inputs:
dry_run:
description: "Report what would be dismissed without dismissing it"
type: boolean
default: true

concurrency:
group: codeql-suppress
cancel-in-progress: false

permissions:
contents: read
security-events: write

jobs:
replay:
name: Replay reviewed dismissals
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
with:
ref: main
- name: Replay
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
GITHUB_REPOSITORY: ${{ github.repository }}
TARGET_REF: refs/heads/main
DRY_RUN: ${{ inputs.dry_run || 'false' }}
run: python3 .github/scripts/codeql_suppress.py
4 changes: 4 additions & 0 deletions nodedb-physical/src/physical_plan/document/op.rs
Original file line number Diff line number Diff line change
Expand Up @@ -337,6 +337,10 @@ pub enum DocumentOp {
source_collection: QualifiedCollection,
source_filters: Vec<u8>,
source_limit: usize,
/// zerompk-encoded `Vec<ComputedColumn>`: one entry per target column,
/// its `alias` the target column name and its `expr` the source-row
/// expression. Empty means copy each source row unchanged.
column_map: Vec<u8>,
},

/// Upsert: insert or merge. When `on_conflict_updates` is non-empty,
Expand Down
9 changes: 9 additions & 0 deletions nodedb-physical/src/physical_plan/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,15 @@ pub enum QueryOp {
/// Row-level-security filters for rows scanned from
/// `right_collection` locally. Same semantics as `left_rls_filters`.
right_rls_filters: Vec<u8>,
/// Predicates from the left side's own `WHERE`, applied to rows
/// scanned from `left_collection` locally. Empty when `left_input` is
/// `Some`, because the child plan then carries its own predicates.
/// Applied per side before the join, for the same reason as
/// `left_rls_filters`.
left_scan_filters: Vec<u8>,
/// Predicates from the right side's own `WHERE`. Same semantics as
/// `left_scan_filters`.
right_scan_filters: Vec<u8>,
},

/// Cross-node shuffle-join CONSUMER (E4b): run the node-local grace-hash
Expand Down
2 changes: 2 additions & 0 deletions nodedb-physical/src/physical_plan/wire.rs
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,8 @@ mod tests {
right_input: None,
left_rls_filters: Vec::new(),
right_rls_filters: Vec::new(),
left_scan_filters: Vec::new(),
right_scan_filters: Vec::new(),
left_bitmap: None,
right_bitmap: None,
});
Expand Down
Loading
Loading