diff --git a/packages/cli/src/repowise/cli/commands/security_cmd.py b/packages/cli/src/repowise/cli/commands/security_cmd.py new file mode 100644 index 000000000..e857bb576 --- /dev/null +++ b/packages/cli/src/repowise/cli/commands/security_cmd.py @@ -0,0 +1,176 @@ +"""``repowise security scan`` — security signal scanning. + +Subcommands: + scan --history [--since ] [--to ] [--output json] + Walk the entire git history of the repo (not just the working tree) with + the same pattern registry the indexer uses, and persist any secrets or + risky patterns into the shared ``security_findings`` table — tagged with + the commit that introduced them. Re-runs are idempotent. +""" + +from __future__ import annotations + +import json + +import click + +from repowise.cli.helpers import ( + console, + ensure_repowise_dir, + get_db_url_for_repo, + resolve_command_target, + run_async, + silence_logs_for_machine_output, +) + + +@click.group("security") +def security_command() -> None: + """Security signal scanning (working tree + full git history).""" + + +@security_command.command("scan") +@click.option( + "--history", + is_flag=True, + default=False, + help="Scan the full git history, not just the current working tree.", +) +@click.option( + "--since", + default=None, + help="Lower git revision bound (exclusive). Defaults to all history.", +) +@click.option( + "--to", + default=None, + help="Upper git revision bound (inclusive). Defaults to all history/HEAD.", +) +@click.option( + "--path", + "repo", + default=None, + help="Repo path (defaults to cwd / workspace primary).", +) +@click.option( + "--all-patterns", + is_flag=True, + default=False, + help="History mode: also report code-smell patterns (eval, os.system, " + "weak hashes, ...). By default history mode reports only leaked-secret " + "patterns (hardcoded_password / hardcoded_secret) to avoid noise.", +) +@click.option( + "--output", + "output_format", + default="table", + type=click.Choice(["table", "json"]), + help="Output format. ``json`` is the machine-readable summary.", +) +def security_scan( + history: bool, + since: str | None, + to: str | None, + all_patterns: bool, + repo: str | None, + output_format: str, +) -> None: + """Scan for security signals and persist findings to the local store. + + Without ``--history`` this is a no-op stub (working-tree scanning already + happens during ``repowise init`` / ``repowise update``). With ``--history`` + it walks every tracked revision and surfaces leaked secrets / risky + patterns that were later removed — something the working-tree scan cannot + see. + """ + if not history: + console.print( + "[yellow]Working-tree scanning runs automatically during " + "`repowise init`/`repowise update`.[/yellow]\n" + "Pass [cyan]--history[/cyan] to scan the full git history for secrets " + "and risky patterns (including ones deleted in later commits)." + ) + return + + if output_format == "json": + silence_logs_for_machine_output() + + from pathlib import Path + + from repowise.core.analysis.history_scan import HistorySecurityScanner + from repowise.core.persistence import ( + create_engine, + create_session_factory, + get_session, + ) + from repowise.core.persistence.crud import ( + get_repository_by_path, + upsert_repository, + ) + + target = resolve_command_target(path=repo) + target.notice(console, command="security scan --history") + + if target.is_workspace: + primary = target.primary_path() + if primary is None: + raise click.ClickException("Workspace has no primary repo configured.") + repo_path = primary + else: + assert target.repo_path is not None + repo_path = target.repo_path + + ensure_repowise_dir(repo_path) + + async def _do() -> dict: + engine = create_engine(get_db_url_for_repo(repo_path)) + sf = create_session_factory(engine) + async with get_session(sf) as session: + row = await get_repository_by_path(session, str(repo_path)) + if row is None: + row = await upsert_repository( + session, + name=repo_path.name, + local_path=str(repo_path), + ) + scanner = HistorySecurityScanner(session, row.id) + summary = await scanner.scan_history( + Path(repo_path), + since=since, + to=to, + secrets_only=not all_patterns, + progress=lambda msg: ( + console.print(f"[dim]{msg}[/dim]") if output_format != "json" else None + ), + ) + await session.commit() + return { + "commits_scanned": summary.commits_scanned, + "blobs_scanned": summary.blobs_scanned, + "files_scanned": summary.files_scanned, + "findings_inserted": summary.findings_inserted, + "by_severity": summary.by_severity, + "by_kind": summary.by_kind, + } + + result = run_async(_do()) + + if output_format == "json": + click.echo(json.dumps(result, indent=2)) + return + + console.print(f"[bold]repowise security scan --history[/bold] — {repo_path}") + console.print(f" Commits scanned: {result['commits_scanned']}") + console.print(f" Blobs scanned: {result['blobs_scanned']}") + console.print(f" Files scanned: {result['files_scanned']}") + console.print(f" Findings stored: {result['findings_inserted']}") + if result["by_severity"]: + sev = ", ".join(f"{k}={v}" for k, v in sorted(result["by_severity"].items())) + console.print(f" By severity: {sev}") + if result["by_kind"]: + kinds = ", ".join(f"{k}={v}" for k, v in sorted(result["by_kind"].items())) + console.print(f" By kind: {kinds}") + console.print( + "\nFindings are written to the security_findings table and show up in " + "`repowise server`'s security API and UI. Re-running is idempotent." + ) diff --git a/packages/cli/src/repowise/cli/main.py b/packages/cli/src/repowise/cli/main.py index 32eaedc90..fc60ae35c 100644 --- a/packages/cli/src/repowise/cli/main.py +++ b/packages/cli/src/repowise/cli/main.py @@ -29,6 +29,7 @@ from repowise.cli.commands.risk_cmd import risk_command from repowise.cli.commands.saved_cmd import saved_command from repowise.cli.commands.search_cmd import search_command +from repowise.cli.commands.security_cmd import security_command from repowise.cli.commands.serve_cmd import serve_command from repowise.cli.commands.status_cmd import status_command from repowise.cli.commands.telemetry_cmd import telemetry_command @@ -77,6 +78,7 @@ def cli(ctx: click.Context) -> None: register_command(distill_command) register_command(expand_command) register_command(saved_command) +register_command(security_command) register_command(corrections_command) register_command(export_command) register_command(hook_group) diff --git a/packages/core/alembic/versions/0037_security_finding_history.py b/packages/core/alembic/versions/0037_security_finding_history.py new file mode 100644 index 000000000..a4537c1e0 --- /dev/null +++ b/packages/core/alembic/versions/0037_security_finding_history.py @@ -0,0 +1,50 @@ +"""security_findings: full-history provenance + idempotent dedup. + +Adds ``commit_sha`` / ``commit_at`` columns so a finding can be tied to the +commit that introduced it (full-history scans via ``repowise security scan +--history``), and a unique constraint over +``(repository_id, file_path, kind, line_number, commit_sha)`` so re-runs never +double-insert the same signal within the same commit. Working-tree findings +leave both columns NULL (the constraint keys on the empty-string default). + +Revision ID: 0037 +Revises: 0036 +Create Date: 2026-07-13 +""" + +from __future__ import annotations + +from collections.abc import Sequence + +import sqlalchemy as sa +from alembic import op + +# revision identifiers +revision: str = "0037" +down_revision: str | None = "0036" +branch_labels: str | Sequence[str] | None = None +depends_on: str | Sequence[str] | None = None + + +def upgrade() -> None: + op.add_column( + "security_findings", + sa.Column("commit_sha", sa.String(40), nullable=True, server_default=""), + ) + op.add_column( + "security_findings", + sa.Column("commit_at", sa.DateTime(timezone=True), nullable=True), + ) + op.create_unique_constraint( + "uq_security_finding_provenance", + "security_findings", + ["repository_id", "file_path", "kind", "line_number", "commit_sha"], + ) + + +def downgrade() -> None: + op.drop_constraint( + "uq_security_finding_provenance", "security_findings", type_="unique" + ) + op.drop_column("security_findings", "commit_at") + op.drop_column("security_findings", "commit_sha") diff --git a/packages/core/src/repowise/core/analysis/history_scan.py b/packages/core/src/repowise/core/analysis/history_scan.py new file mode 100644 index 000000000..e7bdbf4c2 --- /dev/null +++ b/packages/core/src/repowise/core/analysis/history_scan.py @@ -0,0 +1,294 @@ +"""Full git-history secret/risk scanning. + +Complements :mod:`~repowise.core.analysis.security_scan` (which only looks at +the working tree during indexing). ``HistorySecurityScanner.scan_history`` walks +the full git history of a repo and reuses the exact same pattern registry, so a +leaked credential that was "deleted" in a later commit still surfaces — tagged +with the commit that introduced it. + +Everything lands in the shared ``security_findings`` table. The +``(repository_id, file_path, kind, line_number, commit_sha)`` unique constraint +(migration 0037) makes re-runs idempotent. + +Design notes (in response to review) +------------------------------------ +* **Scan unique blobs, not commits x files.** ``git rev-list --objects --all`` + enumerates every object once, deduped by blob SHA, so each distinct blob's + content is scanned a single time. Hits are then attributed to the + first-introducing commit for provenance. Git already dedups content by blob, + so we get natural dedup + a big speedup for free (vs. ``git ls-tree`` per + commit, which re-reads identical content thousands of times). + +* **History mode defaults to the secret-oriented subset.** Most of the 11 + patterns are code smells (``eval``/``os.system``/``weak_hash``) rather than + leaked credentials; running those across all of history produces mostly noise + ("os.system in a two-year-old commit") with little to act on. The + history-relevant subset is ``hardcoded_password`` / ``hardcoded_secret``. This + positions history scanning as complementary to a real secret scanner + (gitleaks / trufflehog) rather than a noisy replacement. ``--all-patterns`` + opts back into the full registry when desired. + +The git layer is isolated so the iteration logic can be exercised in unit tests +without a real repository. +""" + +from __future__ import annotations + +import subprocess +from dataclasses import dataclass, field +from datetime import UTC, datetime +from pathlib import Path +from typing import Any + +from repowise.core.analysis.security_scan import ( + SECRET_KINDS, + SecurityScanner, +) +from repowise.core.ingestion.models import EXTENSION_TO_LANGUAGE + + +def _run_git(repo_path: Path, args: list[str], *, timeout: float = 30.0) -> str: + """Run a ``git`` command in *repo_path* and return stdout (best-effort). + + Returns ``""`` on any failure so callers degrade gracefully (a repo with no + git history, a missing binary, or an unexpected ref all yield an empty + scan rather than a crash). + """ + try: + result = subprocess.run( + ["git", *args], + cwd=str(repo_path), + capture_output=True, + text=True, + timeout=timeout, + ) + except (OSError, subprocess.SubprocessError): + return "" + if result.returncode != 0: + return "" + return result.stdout + + +def _parse_author_date(iso: str) -> datetime | None: + """Parse a git ``%aI`` timestamp into a timezone-aware datetime (or None).""" + iso = iso.strip() + if not iso: + return None + try: + # git's %aI is strict ISO-8601; normalise a trailing Z for older parsers. + return datetime.fromisoformat(iso.replace("Z", "+00:00")).astimezone(UTC) + except ValueError: + return None + + +@dataclass +class HistoryScanSummary: + """Aggregate result of a full-history scan, for CLI/JSON output.""" + + commits_scanned: int = 0 + blobs_scanned: int = 0 + files_scanned: int = 0 + findings_inserted: int = 0 + by_severity: dict[str, int] = field(default_factory=dict) + by_kind: dict[str, int] = field(default_factory=dict) + + +class HistorySecurityScanner: + """Scan the full git history of a repository for security signals.""" + + def __init__(self, session: Any, repo_id: str) -> None: + self._session = session + self._repo_id = repo_id + self._scanner = SecurityScanner(session, repo_id) + + # ------------------------------------------------------------------ + # Git layer (thin wrappers around _run_git; overridable for tests) + # ------------------------------------------------------------------ + + def _list_commits(self, repo_path: Path, since: str | None, to: str | None) -> list[tuple[str, str]]: + """Return ``[(sha, author_iso), ...]`` oldest→newest for the range. + + *since* / *to* mirror ``git rev-list`` range syntax: ``since..to``. + When both are None, the whole reachable history is scanned (``--all``). + """ + if since and to: + rev_range = f"{since}..{to}" + elif to: + rev_range = to + elif since: + rev_range = f"{since}..HEAD" + else: + rev_range = "--all" + + # %x1f is a unit separator; %H is the full SHA, %aI the author date. + raw = _run_git(repo_path, ["log", "--reverse", "--format=%H%x1f%aI", rev_range]) + commits: list[tuple[str, str]] = [] + for line in raw.splitlines(): + line = line.strip() + if not line or "\x1f" not in line: + continue + sha, _, iso = line.partition("\x1f") + if sha: + commits.append((sha.strip(), iso)) + return commits + + def _unique_blobs(self, repo_path: Path, since: str | None, to: str | None) -> dict[str, str]: + """Return ``{blob_sha: first_seen_path}`` across the requested range. + + Uses ``git rev-list --objects`` over the range so each distinct blob is + enumerated once and deduped by content hash. The first path a blob is + seen at is kept for attribution/provenance; the scan only runs once per + blob regardless of how many commits reference it. + """ + if since and to: + rev_range = f"{since}..{to}" + elif to: + rev_range = to + elif since: + rev_range = f"{since}..HEAD" + else: + rev_range = "--all" + + raw = _run_git( + repo_path, + ["rev-list", "--objects", rev_range], + timeout=60.0, + ) + blobs: dict[str, str] = {} + for line in raw.splitlines(): + line = line.strip() + if not line: + continue + parts = line.split() + if len(parts) < 2: + continue + obj_sha, obj_type = parts[0], parts[1] + if obj_type != "blob": + continue + path = parts[2] if len(parts) > 2 else "" + # Only the first reference to a blob is kept; later duplicates are + # ignored — that is the dedup the maintainer asked for. + blobs.setdefault(obj_sha, path) + return blobs + + def _read_blob(self, repo_path: Path, blob_sha: str) -> str: + """Return the textual content of *blob_sha* (empty on failure).""" + return _run_git(repo_path, ["cat-file", "-p", blob_sha], timeout=60.0) + + @staticmethod + def _is_source(path: str) -> bool: + """True when *path* has a language we scan (mirrors the indexer).""" + suffix = Path(path).suffix.lower().lstrip(".") + return suffix in EXTENSION_TO_LANGUAGE + + @staticmethod + def _passes_gate(kind: str, *, secrets_only: bool) -> bool: + """Filter a finding kind against the history scan gate. + + When *secrets_only* is True (the default for history mode), only the + secret-oriented patterns survive — the rest are code smells that are + mostly noise across history. + """ + if secrets_only: + return kind in SECRET_KINDS + return True + + def _blobs_in_commit(self, repo_path: Path, commit: str) -> list[str]: + """Return the blob SHAs tracked by *commit* (git ls-tree -r).""" + raw = _run_git(repo_path, ["ls-tree", "-r", commit]) + out: list[str] = [] + for line in raw.splitlines(): + parts = line.split() + if len(parts) < 4 or parts[1] != "blob": + continue + out.append(parts[2]) + return out + + # ------------------------------------------------------------------ + # Scan driver + # ------------------------------------------------------------------ + + async def scan_history( + self, + repo_path: Path, + *, + since: str | None = None, + to: str | None = None, + secrets_only: bool = True, + progress: Any = None, + ) -> HistoryScanSummary: + """Scan the full git history and persist findings with commit provenance. + + Parameters + ---------- + repo_path: + Repository root. + since / to: + Optional git rev-range bounds (``since..to``). ``None`` scans all + reachable history. + secrets_only: + When True (default), only the secret-oriented patterns + (hardcoded_password / hardcoded_secret) are reported, to avoid the + code-smell noise of scanning all of history. Pass False to scan the + full pattern registry. + progress: + Optional callable ``progress(message)`` for CLI feedback. + """ + summary = HistoryScanSummary() + + commits = self._list_commits(repo_path, since, to) + summary.commits_scanned = len(commits) + if not commits: + return summary + + # Distinct blobs, deduped by content hash. Each blob is scanned once. + blobs = self._unique_blobs(repo_path, since, to) + summary.blobs_scanned = len(blobs) + + # Attribute each blob to the oldest commit (by walk order) that + # contains it, so a hit is reported against its first introduction. + blob_introduced_at: dict[str, str] = {} + for commit, _iso in commits: + for blob_sha in self._blobs_in_commit(repo_path, commit): + blob_introduced_at.setdefault(blob_sha, commit) + + scanned = 0 + for blob_sha, path in blobs.items(): + scanned += 1 + if path and not self._is_source(path): + continue + content = self._read_blob(repo_path, blob_sha) + findings = await self._scanner.scan_file(path, content, []) + if not findings: + continue + # Gate to the secret-oriented subset by default. + kept = [ + f for f in findings + if self._passes_gate(f["kind"], secrets_only=secrets_only) + ] + if not kept: + continue + commit_sha = blob_introduced_at.get(blob_sha) + commit_at: datetime | None = None + for c, iso in commits: + if c == commit_sha: + commit_at = _parse_author_date(iso) + break + inserted = await self._scanner.persist( + path or "", + kept, + commit_sha=commit_sha, + commit_at=commit_at, + ) + summary.findings_inserted += inserted + for f in kept: + sev = f.get("severity", "unknown") + kind = f.get("kind", "unknown") + summary.by_severity[sev] = summary.by_severity.get(sev, 0) + 1 + summary.by_kind[kind] = summary.by_kind.get(kind, 0) + 1 + + if progress is not None: + progress(f"scanned blob {scanned}/{summary.blobs_scanned}") + + summary.files_scanned = scanned + return summary diff --git a/packages/core/src/repowise/core/analysis/security_scan.py b/packages/core/src/repowise/core/analysis/security_scan.py index d9abc6659..c0e20b100 100644 --- a/packages/core/src/repowise/core/analysis/security_scan.py +++ b/packages/core/src/repowise/core/analysis/security_scan.py @@ -3,7 +3,17 @@ Scans indexed symbols and source for keyword/regex patterns that indicate authentication, secret handling, raw SQL, dangerous deserialization, etc. -Stores findings in the security_findings table (see migration 0011). +Two scan surfaces share the same pattern registry and persistence layer: + +* working-tree scans (during indexing) — ``SecurityScanner.scan_file`` + + ``persist`` with no commit provenance; +* full-history scans (``repowise security scan --history``) — iterate every + tracked revision of every source file and persist hits tagged with the + introducing commit's SHA + author date. + +Both paths land in the ``security_findings`` table. The +``(repository_id, file_path, kind, line_number, commit_sha)`` unique +constraint (migration 0037) makes re-runs idempotent. """ from __future__ import annotations @@ -12,6 +22,7 @@ from datetime import UTC, datetime from typing import Any +from sqlalchemy import text from sqlalchemy.ext.asyncio import AsyncSession # --------------------------------------------------------------------------- @@ -36,6 +47,14 @@ r"\b(auth|token|password|jwt|session|crypto)\b", re.IGNORECASE ) +# Patterns whose matches are genuine leaked credentials (as opposed to the +# broader "code smell" patterns like os.system/eval). Full-history scans +# default to this subset: a historical commit that *once* called eval() is +# mostly noise, whereas a committed secret is actionable and persists in +# history. This positions history mode as complementary to gitleaks / +# trufflehog rather than a noisy replacement. +SECRET_KINDS: frozenset[str] = frozenset({"hardcoded_password", "hardcoded_secret"}) + class SecurityScanner: """Scan a single file for security signals and persist to the database.""" @@ -94,25 +113,68 @@ async def scan_file( return findings - async def persist(self, file_path: str, findings: list[dict]) -> None: + def _uses_sqlite(self) -> bool: + """True when the bound session talks to SQLite (local/dev backend).""" + try: + name = self._session.bind.dialect.name # type: ignore[attr-defined] + except AttributeError: + name = "" + return name == "sqlite" + + async def persist( + self, + file_path: str, + findings: list[dict], + *, + commit_sha: str | None = None, + commit_at: datetime | None = None, + ) -> int: """Insert security findings into the security_findings table. - Uses raw INSERT to stay independent of any ORM session state. - Silently skips if the table doesn't exist yet (pre-migration). + Re-runs never duplicate rows: the unique provenance constraint + (``uq_security_finding_provenance``) makes a conflicting INSERT a no-op. + We pick the conflict clause per dialect — Postgres supports + ``ON CONFLICT ON CONSTRAINT ... DO NOTHING``; SQLite uses + ``INSERT OR IGNORE`` (``ON CONFLICT ON CONSTRAINT`` is unsupported). + + ``commit_sha`` / ``commit_at`` carry the git-history provenance; omit + them (working-tree scans) to leave the columns NULL/empty. The dedup key + uses ``""`` (not NULL) for working-tree findings so the constraint keys + identically across runs. + + A per-row failure is skipped (``continue``) rather than aborting the + whole batch, so one malformed finding cannot silently drop the rest. + Returns the number of rows actually inserted, taken from the statement's + ``rowcount`` (the constraint makes duplicate inserts report 0 affected + rows on Postgres; SQLite reports the inserted count via ``rowcount`` too). """ - from sqlalchemy import text - if not findings: - return + return 0 now = datetime.now(UTC) + # The dedup key uses "" (not NULL) for working-tree findings so the + # unique constraint keys identically across runs. + sha_key = commit_sha or "" + uses_sqlite = self._uses_sqlite() + if uses_sqlite: + conflict_clause = "INSERT OR IGNORE INTO security_findings " + else: + conflict_clause = ( + "INSERT INTO security_findings " + "ON CONFLICT ON CONSTRAINT uq_security_finding_provenance " + "DO NOTHING " + ) + + inserted = 0 for finding in findings: try: - await self._session.execute( + result = await self._session.execute( text( - "INSERT INTO security_findings " - "(repository_id, file_path, kind, severity, snippet, line_number, detected_at) " - "VALUES (:repo_id, :file_path, :kind, :severity, :snippet, :line, :detected_at)" + conflict_clause + + "(repository_id, file_path, kind, severity, snippet, line_number, " + "commit_sha, commit_at, detected_at) " + "VALUES (:repo_id, :file_path, :kind, :severity, :snippet, :line, " + ":commit_sha, :commit_at, :detected_at)" ), { "repo_id": self._repo_id, @@ -121,8 +183,12 @@ async def persist(self, file_path: str, findings: list[dict]) -> None: "severity": finding["severity"], "snippet": finding.get("snippet", ""), "line": finding.get("line", 0), + "commit_sha": sha_key, + "commit_at": commit_at, "detected_at": now, }, ) - except Exception: # noqa: BLE001 — table may not exist pre-migration - break + inserted += max(result.rowcount or 0, 0) + except Exception: + continue + return inserted diff --git a/packages/core/src/repowise/core/persistence/models.py b/packages/core/src/repowise/core/persistence/models.py index 05d550cc1..5e270bb72 100644 --- a/packages/core/src/repowise/core/persistence/models.py +++ b/packages/core/src/repowise/core/persistence/models.py @@ -848,9 +848,28 @@ class LlmCost(Base): class SecurityFinding(Base): - """A security signal detected during file ingestion.""" + """A security signal detected during file ingestion or full-history scan. + + Working-tree findings (from indexing) leave ``commit_sha`` / ``commit_at`` + NULL. Full-history scans (``repowise security scan --history``) populate + them so a finding can be tied to the commit that introduced it. The + ``(repository_id, file_path, kind, line_number, commit_sha)`` constraint + makes re-runs idempotent: the same signal in the same commit is never + double-inserted, while a signal that recurs across distinct commits stays + a separate row (its provenance differs). + """ __tablename__ = "security_findings" + __table_args__ = ( + UniqueConstraint( + "repository_id", + "file_path", + "kind", + "line_number", + "commit_sha", + name="uq_security_finding_provenance", + ), + ) id: Mapped[int] = mapped_column(Integer, primary_key=True, autoincrement=True) repository_id: Mapped[str] = mapped_column( @@ -861,10 +880,20 @@ class SecurityFinding(Base): severity: Mapped[str] = mapped_column(String(20), nullable=False) snippet: Mapped[str | None] = mapped_column(Text, nullable=True) line_number: Mapped[int | None] = mapped_column(Integer, nullable=True) + # Full-history provenance. NULL for working-tree findings. The constraint + # above uses these for dedup; for working-tree rows they default to the + # empty string so the unique key stays stable. + commit_sha: Mapped[str | None] = mapped_column(String(40), nullable=True, default="") + commit_at: Mapped[datetime | None] = mapped_column(DateTime(timezone=True), nullable=True) detected_at: Mapped[datetime] = mapped_column( DateTime(timezone=True), nullable=False, default=_now_utc ) + @property + def found_in_history(self) -> bool: + """True when this finding was sourced from git history (has a commit).""" + return bool(self.commit_sha) + class DeadCodeFinding(Base): """Dead code finding: unreachable files, unused exports, zombie packages.""" diff --git a/packages/server/src/repowise/server/routers/security.py b/packages/server/src/repowise/server/routers/security.py index cdd76a942..9a4aea41e 100644 --- a/packages/server/src/repowise/server/routers/security.py +++ b/packages/server/src/repowise/server/routers/security.py @@ -2,10 +2,10 @@ from __future__ import annotations +from fastapi import APIRouter, Depends, Query from sqlalchemy import select from sqlalchemy.ext.asyncio import AsyncSession -from fastapi import APIRouter, Depends, Query from repowise.core.persistence.models import SecurityFinding from repowise.server.deps import get_db_session, verify_api_key from repowise.server.schemas import SecurityFindingResponse @@ -22,6 +22,10 @@ async def list_security_findings( repo_id: str, file_path: str | None = Query(None, description="Filter by relative file path"), severity: str | None = Query(None, description="Filter by severity: high, med, or low"), + history: bool | None = Query( + None, + description="If true, only full-history findings; if false, only working-tree findings; omit for both.", + ), limit: int = Query(100, ge=1, le=500), session: AsyncSession = Depends(get_db_session), # noqa: B008 ) -> list[SecurityFindingResponse]: @@ -34,6 +38,13 @@ async def list_security_findings( if severity is not None: stmt = stmt.where(SecurityFinding.severity == severity) + if history is not None: + # Working-tree rows store "" for commit_sha; history rows store a SHA. + if history: + stmt = stmt.where(SecurityFinding.commit_sha != "") + else: + stmt = stmt.where(SecurityFinding.commit_sha == "") + stmt = stmt.order_by(SecurityFinding.detected_at.desc()).limit(limit) result = await session.execute(stmt) @@ -47,6 +58,8 @@ async def list_security_findings( severity=row.severity, snippet=row.snippet, detected_at=row.detected_at, + commit_sha=row.commit_sha or None, + found_in_history=bool(row.commit_sha), ) for row in rows ] diff --git a/packages/server/src/repowise/server/schemas/code_quality.py b/packages/server/src/repowise/server/schemas/code_quality.py index 2f17a2850..9bb78c429 100644 --- a/packages/server/src/repowise/server/schemas/code_quality.py +++ b/packages/server/src/repowise/server/schemas/code_quality.py @@ -88,6 +88,10 @@ class SecurityFindingResponse(BaseModel): severity: str snippet: str | None detected_at: datetime + # Present when the finding was sourced from git history (full-history + # scan). ``None`` for working-tree findings produced during indexing. + commit_sha: str | None + found_in_history: bool class RepoStatsResponse(BaseModel): diff --git a/tests/unit/analysis/test_security_scan_history.py b/tests/unit/analysis/test_security_scan_history.py new file mode 100644 index 000000000..bec6bbde3 --- /dev/null +++ b/tests/unit/analysis/test_security_scan_history.py @@ -0,0 +1,156 @@ +"""Unit tests for security scanning: working-tree persist + full-history scan. + +Covers the maintainer's review points for issue #818: +* idempotent re-runs via the unique provenance constraint (no duplicate rows); +* secret-oriented gating for history mode (code smells excluded by default); +* per-row failure isolation (``continue`` rather than aborting the batch); +* unique-blob dedup so identical content is scanned once. +""" + +from __future__ import annotations + +from pathlib import Path + +import pytest +from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker, create_async_engine + +from repowise.core.analysis.history_scan import HistorySecurityScanner +from repowise.core.analysis.security_scan import ( + SECRET_KINDS, + SecurityScanner, +) +from repowise.core.persistence.models import Base + + +@pytest.fixture +async def session() -> AsyncSession: + """In-memory SQLite session with the full schema (incl. unique constraint).""" + engine = create_async_engine( + "sqlite+aiosqlite:///:memory:", + connect_args={"check_same_thread": False}, + ) + async with engine.begin() as conn: + await conn.run_sync(Base.metadata.create_all) + factory: async_sessionmaker[AsyncSession] = async_sessionmaker( + engine, expire_on_commit=False, class_=AsyncSession + ) + async with factory() as s: + yield s + await engine.dispose() + + +def _findings() -> list[dict]: + return [ + {"kind": "hardcoded_password", "severity": "high", "snippet": "password='x'", "line": 1}, + {"kind": "eval_call", "severity": "high", "snippet": "eval(x)", "line": 2}, + ] + + +async def test_persist_uses_insert_ignore_and_counts_inserted(session: AsyncSession) -> None: + """Non-duplicate rows are inserted; insert count reflects actual writes.""" + scanner = SecurityScanner(session, "repo-1") + inserted = await scanner.persist("a.py", _findings()) + assert inserted == 2 + + +async def test_persist_is_idempotent_on_rerun(session: AsyncSession) -> None: + """Re-running the same findings does not create duplicate rows.""" + scanner = SecurityScanner(session, "repo-1") + first = await scanner.persist("a.py", _findings()) + second = await scanner.persist("a.py", _findings()) + assert first == 2 + # Same provenance (no commit_sha -> "" key) -> nothing new inserted. + assert second == 0 + rows = (await session.execute(Base.metadata.tables["security_findings"].select())).all() + assert len(rows) == 2 + + +async def test_persist_continues_past_row_failure(session: AsyncSession) -> None: + """A bad row is skipped (continue) and the rest still insert. + + We force a failure by inserting a row whose ``severity`` is too long for the + String(20) column on SQLite (which is lenient) — instead we exercise the + exception path by monkeypatching execute to raise once. This proves the loop + isolates failures rather than aborting the batch. + """ + scanner = SecurityScanner(session, "repo-1") + real_execute = session.execute + + calls = {"n": 0} + + async def _boom(*args, **kwargs): + calls["n"] += 1 + if calls["n"] == 1: + raise RuntimeError("simulated DB error on first row") + return await real_execute(*args, **kwargs) + + session.execute = _boom # type: ignore[assignment] + inserted = await scanner.persist("a.py", _findings()) + # One row failed, one succeeded -> at least 1 inserted, no crash. + assert inserted >= 1 + + +async def test_history_gate_excludes_code_smells_by_default(session: AsyncSession) -> None: + """History mode keeps only secret kinds when secrets_only (default) is set.""" + assert HistorySecurityScanner._passes_gate("hardcoded_password", secrets_only=True) + assert HistorySecurityScanner._passes_gate("hardcoded_secret", secrets_only=True) + assert not HistorySecurityScanner._passes_gate("eval_call", secrets_only=True) + assert not HistorySecurityScanner._passes_gate("os_system", secrets_only=True) + # With secrets_only=False the full registry is reported. + assert HistorySecurityScanner._passes_gate("eval_call", secrets_only=False) + + +async def test_history_secrets_only_scan_skips_non_secret_patterns(session: AsyncSession) -> None: + """scan_history with defaults only persists secret findings.""" + content = "password = 'hunter2'\neval(open('x'))\nos.system('ls')\n" + scanner = HistorySecurityScanner(session, "repo-1") + # Stub the git layer: a single blob introduced at a commit. + scanner._list_commits = lambda *a, **k: [("abc123", "2026-01-01T00:00:00+00:00")] # type: ignore[assignment] + scanner._unique_blobs = lambda *a, **k: {"blob1": "src/secret.py"} # type: ignore[assignment] + scanner._read_blob = lambda *a, **k: content # type: ignore[assignment] + scanner._blobs_in_commit = lambda *a, **k: ["blob1"] # type: ignore[assignment] + scanner._is_source = staticmethod(lambda p: True) # type: ignore[assignment] + + summary = await scanner.scan_history(Path("/tmp/repo"), secrets_only=True) + # Only hardcoded_password survives the gate. + assert summary.findings_inserted == 1 + assert summary.by_kind == {"hardcoded_password": 1} + assert summary.by_severity == {"high": 1} + + row = (await session.execute(Base.metadata.tables["security_findings"].select())).first() + assert row is not None + assert row._mapping["commit_sha"] == "abc123" + + +async def test_history_unique_blob_scanned_once(session: AsyncSession) -> None: + """Identical content across two commits is scanned once, attributed to first.""" + content = "api_key = 'LEAKED'\n" + scanner = HistorySecurityScanner(session, "repo-1") + reads = {"n": 0} + + scanner._list_commits = lambda *a, **k: [ # type: ignore[assignment] + ("c1", "2026-01-01T00:00:00+00:00"), + ("c2", "2026-02-01T00:00:00+00:00"), + ] + scanner._unique_blobs = lambda *a, **k: {"blob1": "src/key.py"} # type: ignore[assignment] + + def _read(*a, **k): + reads["n"] += 1 + return content + + scanner._read_blob = _read # type: ignore[assignment] + scanner._blobs_in_commit = lambda *a, **k: ["blob1"] # type: ignore[assignment] + scanner._is_source = staticmethod(lambda p: True) # type: ignore[assignment] + + summary = await scanner.scan_history(Path("/tmp/repo"), secrets_only=True) + # Blob read exactly once despite being in two commits. + assert reads["n"] == 1 + # Attributed to the first-introducing commit. + assert summary.by_kind == {"hardcoded_secret": 1} + row = (await session.execute(Base.metadata.tables["security_findings"].select())).first() + assert row._mapping["commit_sha"] == "c1" + + +def test_secret_kinds_are_the_two_secret_patterns() -> None: + """Guard against the registry drifting away from the history gate.""" + assert {"hardcoded_password", "hardcoded_secret"} == SECRET_KINDS