Skip to content

feat: add ruff and pip-audit CI gates, fix style violations, and add …#70

Merged
wpak-ai merged 3 commits into
masterfrom
feat/ci-lint-security-policy
Jun 9, 2026
Merged

feat: add ruff and pip-audit CI gates, fix style violations, and add …#70
wpak-ai merged 3 commits into
masterfrom
feat/ci-lint-security-policy

Conversation

@clean6378-max-it

@clean6378-max-it clean6378-max-it commented Jun 9, 2026

Copy link
Copy Markdown
Collaborator

Closes #67

Summary

  • Add ruff check and pip-audit CI job on ubuntu-latest and windows-latest
  • Configure ruff (E, F, W, I) in pyproject.toml; add ruff and pip-audit to requirements-dev.txt
  • Fix existing ruff violations across api/, utils/, scripts/, and tests/ (no blanket # noqa)
  • Add SECURITY.md with GHSA reporting path, response timeline, and attack-surface scope
  • Link SECURITY.md from README.md; update CI section in README

Test plan

  • ruff check . passes locally
  • pip-audit -r requirements.txt passes locally
  • pytest -q, mypy, and npm test pass locally
  • CI matrix green on ubuntu-latest and windows-latest
  • Enable private vulnerability reporting on GitHub (Settings → Security) or note admin blocker in PR

Notes

  • pip-audit scans production requirements.txt only (Flask + transitive deps)
  • Pre-release policy: security fixes on latest master only (0.1.0.dev0)

Summary by CodeRabbit

  • New Features

    • Added a repository security policy with reporting guidance and response timelines
  • Chores

    • CI expanded to run linting and dependency security audits on Ubuntu and Windows
    • Added development tooling/configuration for linting and auditing
  • Documentation

    • README and CONTRIBUTING updated to document dual-platform CI, lint/audit gates, and link to the new security policy
  • Tests

    • Added/adjusted tests including an exclusion-rule session-stats check ensuring excluded sessions return 404

@coderabbitai

coderabbitai Bot commented Jun 9, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: ea0e33ce-7b2e-4e56-b230-8e1a4759c8a1

📥 Commits

Reviewing files that changed from the base of the PR and between 2eebda6 and a6ef270.

📒 Files selected for processing (5)
  • CONTRIBUTING.md
  • README.md
  • SECURITY.md
  • api/export_api.py
  • tests/test_error_propagation.py
✅ Files skipped from review due to trivial changes (3)
  • SECURITY.md
  • CONTRIBUTING.md
  • api/export_api.py

📝 Walkthrough

Walkthrough

Adds a CI job that runs Ruff and pip-audit on Ubuntu and Windows, configures Ruff in pyproject, adds ruff and pip-audit to dev requirements, publishes SECURITY.md and README link, and applies import/formatting adjustments across code and tests.

Changes

Linting, Auditing & Security Policy

Layer / File(s) Summary
Security policy & README link
SECURITY.md, README.md
Adds SECURITY.md describing reporting, scope, and timelines; README links to security policy and notes CI runs on Ubuntu and Windows.
CI job: lint-and-audit
.github/workflows/ci.yml
Introduces lint-and-audit job that runs on ubuntu/windows matrix, installs dev deps, runs ruff check ., ruff format --check ., and pip-audit -r requirements.txt.
Ruff config & dev deps
pyproject.toml, requirements-dev.txt
Adds [tool.ruff] configuration (target py312, select E/F/W/I, per-file ignores) and adds ruff>=0.9.0 and pip-audit>=2.7.0 to requirements-dev.txt.
Import reordering & unused import removal
api/*.py, app.py, models/export.py, scripts/export.py, utils/*
Removes unused imports (json, NotRequired, json_error), reorders and splits utils.* imports across modules to comply with Ruff import rules; no changes to runtime control flow except the removed duplicate exclusion-return in get_session_stats.
Formatting & test updates
scripts/export.py, tests/*, utils/md_exporter.py, utils/jsonl_parser.py
Reformats CLI help/args, multi-line literals, many tests and fixtures for readability; adds a test asserting excluded session stats return 404 and updates the static leak guard to a regex. Minor behavioral change: utils/md_exporter._render_tool_result uses .get() for optional total_tool_use_count.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested labels

enhancement

Suggested reviewers

  • timon0305
  • wpak-ai
  • jonathanMLDev

Poem

🐰 I hopped through files with lint in my paw,
Cleaned imports and whitespace so CI saw.
Ruff tut-tuts softly, pip-audit keeps watch,
SECURITY.md stands guard, a vigilant notch.
Ubuntu and Windows now run side by side.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 31.22% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title 'feat: add ruff and pip-audit CI gates, fix style violations, and add …' clearly describes the main change: adding linting/audit infrastructure and fixing style violations, which aligns with the primary objectives outlined in issue #67.
Linked Issues check ✅ Passed The PR fully implements objectives from issue #67: ruff and pip-audit CI gates run on ubuntu/windows matrix [A], ruff violations fixed throughout codebase without blanket noqa, SECURITY.md created with reporting path/timeline/scope, and README.md links SECURITY.md [B]. All acceptance criteria are met.
Out of Scope Changes check ✅ Passed All changes directly support the stated objectives: CI workflow additions, ruff configuration, dependency updates, style violation fixes across utils/api/scripts/tests, new SECURITY.md/README updates, and contributing guide updates. No unrelated changes are present.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/ci-lint-security-policy

Comment @coderabbitai help to get the list of available commands and usage tips.

@clean6378-max-it clean6378-max-it self-assigned this Jun 9, 2026
@clean6378-max-it

Copy link
Copy Markdown
Collaborator Author

@CodeRabbit, can you review this PR fully now?

@coderabbitai

coderabbitai Bot commented Jun 9, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
echo "=== .github/workflows/ci.yml ==="
cat .github/workflows/ci.yml

echo ""
echo "=== pyproject.toml ==="
cat pyproject.toml

echo ""
echo "=== SECURITY.md ==="
cat SECURITY.md

echo ""
echo "=== requirements-dev.txt ==="
cat requirements-dev.txt

echo ""
echo "=== requirements.txt ==="
cat requirements.txt

echo ""
echo "=== app.py ==="
cat app.py

echo ""
echo "=== utils/md_exporter.py ==="
cat utils/md_exporter.py

echo ""
echo "=== api/export_api.py ==="
cat api/export_api.py

echo ""
echo "=== api/projects.py ==="
cat api/projects.py

echo ""
echo "=== api/search.py ==="
cat api/search.py

echo ""
echo "=== api/sessions.py ==="
cat api/sessions.py

echo ""
echo "=== scripts/export.py ==="
cat scripts/export.py

echo ""
echo "=== models/export.py ==="
cat models/export.py

echo ""
echo "=== utils/export_day_filter.py ==="
cat utils/export_day_filter.py

echo ""
echo "=== utils/jsonl_parser.py ==="
cat utils/jsonl_parser.py

echo ""
echo "=== tests/test_exclusion_helpers.py ==="
cat tests/test_exclusion_helpers.py

echo ""
echo "=== tests/test_export_state.py ==="
cat tests/test_export_state.py

echo ""
echo "=== tests/test_cli_args.py ==="
cat tests/test_cli_args.py

echo ""
echo "=== README.md (first 100 lines) ==="
head -100 README.md

Length of output: 131170



@clean6378-max-it Sure! Here is the full review of PR #70.


PR Review: feat/ci-lint-security-policymaster

Overall: The PR meets all acceptance criteria from issue #67. CI gates are well-structured, ruff violations are fixed without blanket # noqa, SECURITY.md is comprehensive, and the existing test/type gates are preserved. A few items worth addressing before merge:


🔴 Bug — api/sessions.py: Duplicate exclusion check in get_session_stats()

The is_session_excluded check is performed twice in get_session_stats() — once inside the try block, and again immediately after it. Since the except block always returns, the second check is unreachable when the first check already returned a 404. The second rules assignment is also redundant:

`@sessions_bp.route`("/api/sessions/<path:project_name>/<session_id>/stats")
def get_session_stats(project_name: str, session_id: str) -> FlaskReturn:
    ...
    try:
        session = parse_session(filepath)
        rules = current_app.config.get("EXCLUSION_RULES") or []
        if is_session_excluded(rules, session, project_name):   # ← check `#1`
            return error_response(ErrorCode.SESSION_NOT_FOUND, "Session not found", 404)
    except _PARSE_ERRORS:
        ...
        return error_response(...)

    rules = current_app.config.get("EXCLUSION_RULES") or []    # ← redundant re-fetch
    if is_session_excluded(rules, session, project_name):       # ← unreachable duplicate
        return error_response(ErrorCode.SESSION_NOT_FOUND, "Session not found", 404)

This is a pre-existing issue but the file was touched in this PR. The second block (lines roughly 60–65 in api/sessions.py) should be removed entirely — the exclusion check inside the try block already handles this correctly.


🟡 CI — ruff format --check . not enforced

The lint-and-audit job runs ruff check . (linting) but does not run ruff format --check . (formatting). Code style formatting violations (e.g. extra blank lines, trailing whitespace) would not fail CI. Consider adding:

- name: Ruff format check
  run: ruff format --check .

🟡 utils/jsonl_parser.py — Six separate single-symbol import blocks from the same module

Each helper from utils.jsonl_helpers is imported in its own block:

from utils.jsonl_helpers import (
    entry_message as _entry_message,
)
from utils.jsonl_helpers import (
    extract_images as _extract_images,
)
from utils.jsonl_helpers import (
    extract_text as _extract_text,
)
...  # 3 more blocks

Ruff's isort rule (I) passes this, but a single consolidated import is more idiomatic Python and avoids the question of whether this was an intentional split:

from utils.jsonl_helpers import (
    entry_message as _entry_message,
    extract_images as _extract_images,
    extract_text as _extract_text,
    infer_title as _infer_title,
    normalize_content as _normalize_content,
    strip_system_tags as _strip_system_tags,
)

🟡 utils/md_exporter.py — Lazy import inside _build_header()

def _build_header(session: SessionDict) -> str:
    ...
    if wall is not None:
        from utils.session_stats import _format_duration   # ← inline import

This is a pre-existing pattern and not introduced by this PR, but since the file was reviewed: _format_duration is a private symbol (leading _) being imported from another module — this couples the two modules on an internal API. If session_stats is a required dependency of md_exporter, the import should be at the module level.


🟢 CI Workflow — Looks good

  • Pinned action SHA digests used throughout (security best practice ✓)
  • fail-fast: false on the matrix allows both OS results to surface ✓
  • requirements-dev.txt includes -r requirements.txt, so pip-audit -r requirements.txt running against production only is correct ✓
  • cache-dependency-path keys on both requirements files ✓
  • permissions: contents: read scoped correctly ✓

🟢 pyproject.toml — Ruff config

  • target-version = "py312" matches the runtime ✓
  • select = ["E", "F", "W", "I"] matches issue #67 requirements ✓
  • per-file-ignores for E402 on scripts/export.py and tests/*.py is appropriate given the sys.path.insert bootstrapping pattern ✓
  • line-length = 100 is a deliberate deviation from ruff's default of 88; no objection if intentional

🟢 SECURITY.md

  • Response timeline, scope table, and existing controls reference are all well-structured ✓
  • The note about requiring GitHub private vulnerability reporting to be enabled (or using a private channel) is a good fallback ✓
  • Minor: | latest \master`|is missing a space before the closing|` in the Supported Versions table — cosmetic only

🟢 Style fixes across api/, utils/, scripts/, tests/

  • Removed unused imports (json from export_api.py, NotRequired from models/export.py, os/tempfile/pytest from several test files) ✓
  • Fixed f"" string without format specifiers in _render_system()
  • Import reordering consistent with isort (I) rules ✓
  • No blanket # noqa comments used ✓

Summary: The PR is in good shape. The duplicate exclusion check in get_session_stats() is the only functional concern; the rest are style/robustness suggestions.

@clean6378-max-it

Copy link
Copy Markdown
Collaborator Author

@CodeRabbit. all are addressed now?

@coderabbitai

coderabbitai Bot commented Jun 9, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
echo "=== api/sessions.py ==="
cat api/sessions.py

echo ""
echo "=== .github/workflows/ci.yml (lint-and-audit job) ==="
grep -n "ruff\|format\|audit" .github/workflows/ci.yml

echo ""
echo "=== utils/jsonl_parser.py (imports) ==="
head -60 utils/jsonl_parser.py

echo ""
echo "=== utils/md_exporter.py (_build_header function) ==="
grep -n "_format_duration\|format_duration" utils/md_exporter.py

Length of output: 6141



@clean6378-max-it Yes, everything is addressed! Here's the confirmation for each item:

# Issue Status
🔴 api/sessions.py — duplicate exclusion check in get_session_stats() ✅ Removed — only one check now exists inside the try block
🟡 CI — ruff format --check . not enforced ✅ Added — lines 74–75 in .github/workflows/ci.yml
🟡 utils/jsonl_parser.py — six separate single-symbol import blocks ✅ Consolidated — all six helpers are in one from utils.jsonl_helpers import (...) block
🟡 utils/md_exporter.py — lazy inline import of _format_duration ✅ Fixed — format_duration is now imported at module level (and upgraded from private _format_duration to public format_duration)

The PR looks ready to merge. 🐇

[approve]

Comment thread SECURITY.md
Comment thread .github/workflows/ci.yml
@clean6378-max-it

Copy link
Copy Markdown
Collaborator Author

Timon's concern is addressed? @CodeRabbit.
Also security is satisifieed enough>?

@coderabbitai

coderabbitai Bot commented Jun 9, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Fetch all PR comments and reviews to identify Timon's concern
gh pr view 70 --repo cppalliance/claude-code-chat-browser --json reviews,comments

echo ""
echo "=== PR Review comments ==="
gh api repos/cppalliance/claude-code-chat-browser/pulls/70/reviews

echo ""
echo "=== PR Comments ==="
gh api repos/cppalliance/claude-code-chat-browser/issues/70/comments

Length of output: 159026



@clean6378-max-it Here's the answer to both questions:

Timon's concern

Timon (timon0305) left no written concerns — their review (submitted at 13:22 today) has an empty body and a state of Approved. There are no inline comments from them either. In other words, Timon reviewed the PR and approved it without raising any issues, so there is nothing to address on their end. ✅

Is security satisfied?

Yes. The security posture for this PR is solid:

Item Status
SECURITY.md with private reporting path (GHSA) ✅ Present
Response timeline (acknowledgment + fix target) ✅ Documented
Supported versions (pre-release: latest master only) ✅ Clear
In-scope / out-of-scope attack surface ✅ Enumerated
Existing controls with file references ✅ Listed
Linked from README.md ✅ Done
pip-audit -r requirements.txt CI gate ✅ Running on both ubuntu/windows
Pinned GitHub Action SHA digests ✅ Security best practice applied

The only outstanding pre-merge check warning is Docstring Coverage (31% vs 80% threshold), which is unrelated to the security requirements in issue #67. The PR is ready to merge. 🐇

@clean6378-max-it clean6378-max-it requested a review from wpak-ai June 9, 2026 14:31
@wpak-ai wpak-ai merged commit 32e60f4 into master Jun 9, 2026
11 checks passed
@wpak-ai wpak-ai deleted the feat/ci-lint-security-policy branch June 9, 2026 15:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

claude-code-chat-browser: Add ruff + pip-audit CI gates and SECURITY.md reporting policy

3 participants