diff --git a/.github/ISSUE_TEMPLATE/bug_report.yml b/.github/ISSUE_TEMPLATE/bug_report.yml new file mode 100644 index 0000000..54af170 --- /dev/null +++ b/.github/ISSUE_TEMPLATE/bug_report.yml @@ -0,0 +1,54 @@ +name: Bug report +description: Something in QueryPilot doesn't behave as documented +labels: [bug] +body: + - type: textarea + id: what-happened + attributes: + label: What happened? + description: A clear description of the bug. + placeholder: The validator accepted ... / `querypilot eval run` crashed with ... + validations: + required: true + + - type: textarea + id: repro + attributes: + label: Minimal reproduction + description: The smallest code snippet, SQL statement, or CLI invocation that triggers it. + render: python + validations: + required: true + + - type: textarea + id: expected + attributes: + label: Expected behavior + validations: + required: true + + - type: input + id: version + attributes: + label: QueryPilot version + placeholder: "0.1.0" + validations: + required: true + + - type: input + id: python + attributes: + label: Python version + placeholder: "3.12" + + - type: input + id: database + attributes: + label: Database / dialect + placeholder: sqlite / postgres 16 + + - type: textarea + id: logs + attributes: + label: Logs, traceback, or eval report output + render: text diff --git a/.github/ISSUE_TEMPLATE/config.yml b/.github/ISSUE_TEMPLATE/config.yml new file mode 100644 index 0000000..35cde1e --- /dev/null +++ b/.github/ISSUE_TEMPLATE/config.yml @@ -0,0 +1,5 @@ +blank_issues_enabled: true +contact_links: + - name: Security vulnerability + url: https://github.com/nickklos10/QueryPilot/security/advisories/new + about: Please report security issues privately, not as public issues. diff --git a/.github/ISSUE_TEMPLATE/feature_request.yml b/.github/ISSUE_TEMPLATE/feature_request.yml new file mode 100644 index 0000000..21b55d3 --- /dev/null +++ b/.github/ISSUE_TEMPLATE/feature_request.yml @@ -0,0 +1,38 @@ +name: Feature request +description: Propose an improvement or new capability +labels: [enhancement] +body: + - type: textarea + id: problem + attributes: + label: Problem + description: What are you trying to do that QueryPilot doesn't support today? + validations: + required: true + + - type: textarea + id: proposal + attributes: + label: Proposed solution + validations: + required: true + + - type: textarea + id: alternatives + attributes: + label: Alternatives considered + + - type: dropdown + id: area + attributes: + label: Area + options: + - Safety engine / validation + - Eval harness + - Connectors + - Access control + - Audit trail + - MCP server + - FastAPI server + - LLM generation + - Other diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md new file mode 100644 index 0000000..b5c1f7b --- /dev/null +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -0,0 +1,20 @@ +## Summary + + + +## Changes + +- + +## Testing + +- [ ] `pytest` passes locally +- [ ] New behavior has a unit test +- [ ] Validator/safety changes include a safety-suite case (`suites/safety.yaml`) +- [ ] User-facing CLI changes have a CLI test +- [ ] `CHANGELOG.md` updated under `[Unreleased]` +- [ ] Eval baseline regenerated in this PR if pass/safety/correctness rates deliberately changed + +## Linked issues + + diff --git a/.github/workflows/eval.yml b/.github/workflows/eval.yml index f75c7a9..62f1a2e 100644 --- a/.github/workflows/eval.yml +++ b/.github/workflows/eval.yml @@ -6,7 +6,7 @@ on: branches: [main] jobs: - eval: + lint: runs-on: ubuntu-latest steps: - uses: actions/checkout@v4 @@ -14,7 +14,28 @@ jobs: - name: Set up Python uses: actions/setup-python@v5 with: - python-version: "3.11" + python-version: "3.12" + cache: pip + + - name: Install ruff + run: pip install ruff + + - name: Ruff check + run: ruff check src tests + + eval: + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + python-version: ["3.11", "3.12", "3.13"] + steps: + - uses: actions/checkout@v4 + + - name: Set up Python ${{ matrix.python-version }} + uses: actions/setup-python@v5 + with: + python-version: ${{ matrix.python-version }} cache: pip - name: Install package @@ -46,5 +67,5 @@ jobs: if: always() uses: actions/upload-artifact@v4 with: - name: eval-report + name: eval-report-py${{ matrix.python-version }} path: eval-out.json diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml new file mode 100644 index 0000000..afdca47 --- /dev/null +++ b/.github/workflows/release.yml @@ -0,0 +1,46 @@ +name: release + +on: + release: + types: [published] + +permissions: + contents: read + +jobs: + build: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + + - name: Set up Python + uses: actions/setup-python@v5 + with: + python-version: "3.12" + + - name: Build distributions + run: | + pip install build + python -m build + + - uses: actions/upload-artifact@v4 + with: + name: dist + path: dist/ + + publish: + needs: build + runs-on: ubuntu-latest + environment: + name: pypi + url: https://pypi.org/p/querypilot + permissions: + id-token: write + steps: + - uses: actions/download-artifact@v4 + with: + name: dist + path: dist/ + + - name: Publish to PyPI + uses: pypa/gh-action-pypi-publish@release/v1 diff --git a/CHANGELOG.md b/CHANGELOG.md index 94bc3f8..f25d7df 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,22 @@ follows [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased] +### Added + +- `SECURITY.md` security policy, Contributor Covenant code of conduct, issue + templates, and a pull-request template. +- PyPI release workflow (`.github/workflows/release.yml`) using trusted + publishing, triggered by publishing a GitHub Release. + +### Changed + +- CI now tests Python 3.11, 3.12, and 3.13 and runs `ruff check` as a + separate lint job. + +### Removed + +- Internal planning documents from `docs/`. + ## [0.1.0] - 2026-04-28 Initial public release. Foundation and eval-driven harness shipped. diff --git a/CODE_OF_CONDUCT.md b/CODE_OF_CONDUCT.md new file mode 100644 index 0000000..2a1bc70 --- /dev/null +++ b/CODE_OF_CONDUCT.md @@ -0,0 +1,134 @@ +# Contributor Covenant Code of Conduct + +## Our Pledge + +We as members, contributors, and leaders pledge to make participation in our +community a harassment-free experience for everyone, regardless of age, body +size, visible or invisible disability, ethnicity, sex characteristics, gender +identity and expression, level of experience, education, socio-economic status, +nationality, personal appearance, race, caste, color, religion, or sexual +identity and orientation. + +We pledge to act and interact in ways that contribute to an open, welcoming, +diverse, inclusive, and healthy community. + +## Our Standards + +Examples of behavior that contributes to a positive environment for our +community include: + +* Demonstrating empathy and kindness toward other people +* Being respectful of differing opinions, viewpoints, and experiences +* Giving and gracefully accepting constructive feedback +* Accepting responsibility and apologizing to those affected by our mistakes, + and learning from the experience +* Focusing on what is best not just for us as individuals, but for the overall + community + +Examples of unacceptable behavior include: + +* The use of sexualized language or imagery, and sexual attention or advances + of any kind +* Trolling, insulting or derogatory comments, and personal or political + attacks +* Public or private harassment +* Publishing others' private information, such as a physical or email address, + without their explicit permission +* Other conduct which could reasonably be considered inappropriate in a + professional setting + +## Enforcement Responsibilities + +Community leaders are responsible for clarifying and enforcing our standards +of acceptable behavior and will take appropriate and fair corrective action in +response to any behavior that they deem inappropriate, threatening, offensive, +or harmful. + +Community leaders have the right and responsibility to remove, edit, or reject +comments, commits, code, wiki edits, issues, and other contributions that are +not aligned to this Code of Conduct, and will communicate reasons for +moderation decisions when appropriate. + +## Scope + +This Code of Conduct applies within all community spaces, and also applies +when an individual is officially representing the community in public spaces. +Examples of representing our community include using an official e-mail +address, posting via an official social media account, or acting as an +appointed representative at an online or offline event. + +## Enforcement + +Instances of abusive, harassing, or otherwise unacceptable behavior may be +reported to the community leaders responsible for enforcement at +nklos@inceptaanalytics.ai. +All complaints will be reviewed and investigated promptly and fairly. + +All community leaders are obligated to respect the privacy and security of the +reporter of any incident. + +## Enforcement Guidelines + +Community leaders will follow these Community Impact Guidelines in determining +the consequences for any action they deem in violation of this Code of +Conduct: + +### 1. Correction + +**Community Impact**: Use of inappropriate language or other behavior deemed +unprofessional or unwelcome in the community. + +**Consequence**: A private, written warning from community leaders, providing +clarity around the nature of the violation and an explanation of why the +behavior was inappropriate. A public apology may be requested. + +### 2. Warning + +**Community Impact**: A violation through a single incident or series of +actions. + +**Consequence**: A warning with consequences for continued behavior. No +interaction with the people involved, including unsolicited interaction with +those enforcing the Code of Conduct, for a specified period of time. This +includes avoiding interactions in community spaces as well as external +channels like social media. Violating these terms may lead to a temporary or +permanent ban. + +### 3. Temporary Ban + +**Community Impact**: A serious violation of community standards, including +sustained inappropriate behavior. + +**Consequence**: A temporary ban from any sort of interaction or public +communication with the community for a specified period of time. No public or +private interaction with the people involved, including unsolicited +interaction with those enforcing the Code of Conduct, is allowed during this +period. Violating these terms may lead to a permanent ban. + +### 4. Permanent Ban + +**Community Impact**: Demonstrating a pattern of violation of community +standards, including sustained inappropriate behavior, harassment of an +individual, or aggression toward or disparagement of classes of individuals. + +**Consequence**: A permanent ban from any sort of public interaction within +the community. + +## Attribution + +This Code of Conduct is adapted from the [Contributor Covenant][homepage], +version 2.1, available at +[https://www.contributor-covenant.org/version/2/1/code_of_conduct.html][v2.1]. + +Community Impact Guidelines were inspired by +[Mozilla's code of conduct enforcement ladder][Mozilla CoC]. + +For answers to common questions about this code of conduct, see the FAQ at +[https://www.contributor-covenant.org/faq][FAQ]. Translations are available at +[https://www.contributor-covenant.org/translations][translations]. + +[homepage]: https://www.contributor-covenant.org +[v2.1]: https://www.contributor-covenant.org/version/2/1/code_of_conduct.html +[Mozilla CoC]: https://github.com/mozilla/diversity +[FAQ]: https://www.contributor-covenant.org/faq +[translations]: https://www.contributor-covenant.org/translations diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index d0af16c..94d331e 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -35,9 +35,8 @@ Optional extras for provider-specific work: merged. 3. **Tests are required.** New behavior needs a unit test; new validator guards need a safety-suite case; user-facing CLI changes need a CLI test. -4. **`/code-review` before merge.** Every PR gets a multi-agent code review - pass. Treat issues that score ≥ 80 as blocking; bundle below-threshold - real concerns as a follow-up commit. +4. **Review before merge.** Every PR gets maintainer review, and CI must be + green — unit tests, lint, and the smoke eval gate run on every PR. 5. **Don't break the eval baseline.** CI runs `querypilot eval check` against `.eval/baseline.json`. If your change deliberately changes pass/safety/ correctness rates, regenerate the baseline in the same PR and explain why diff --git a/SECURITY.md b/SECURITY.md new file mode 100644 index 0000000..2f87dd2 --- /dev/null +++ b/SECURITY.md @@ -0,0 +1,39 @@ +# Security Policy + +QueryPilot is a safety layer in front of real databases, so security reports +get priority attention. + +## Supported versions + +| Version | Supported | +| ------- | --------- | +| 0.1.x | ✅ | + +## Reporting a vulnerability + +Please do not open a public issue for security problems. + +Use one of these private channels instead: + +- **GitHub private vulnerability reporting** (preferred): + [Report a vulnerability](https://github.com/nickklos10/QueryPilot/security/advisories/new) +- **Email:** nklos@inceptaanalytics.ai + +Include a proof-of-concept where possible — for example, SQL that bypasses +the validator, the access-policy enforcement (blocked columns, row filters, +masking), or the read-only execution guarantees. + +## What to expect + +- Acknowledgement within 72 hours. +- An assessment and remediation plan within 7 days for confirmed reports. +- Credit in the release notes for the fix, unless you prefer otherwise. + +## Threat model notes + +QueryPilot's validation is defense in depth, not a replacement for database +permissions. The documented production posture is a dedicated least-privilege +database role plus QueryPilot's read-only transaction and statement timeout. +Reports that require a fully-privileged database role and ignore that +guidance may be classified as hardening advice rather than vulnerabilities — +but send them anyway; we'd rather triage than miss something. diff --git a/docs/superpowers/plans/2026-07-11-security-hardening.md b/docs/superpowers/plans/2026-07-11-security-hardening.md deleted file mode 100644 index c6af13a..0000000 --- a/docs/superpowers/plans/2026-07-11-security-hardening.md +++ /dev/null @@ -1,530 +0,0 @@ -# QueryPilot Security Hardening Implementation Plan - -> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. - -**Goal:** Close the reproduced SQL access-policy and PostgreSQL read-only bypasses while preserving QueryPilot's public APIs, MCP tools, eval behavior, and PR #13 package metadata. - -**Architecture:** Keep `SQLValidator` as the enforcement entrypoint, but derive statement, relation, column, star, and function decisions from the `sqlglot` AST. Add database-level defense in depth by making PostgreSQL queries start in a read-only transaction. All changes are test-first and introduce no mandatory dependency. - -**Tech Stack:** Python 3.11+, Pydantic 2, SQLAlchemy 2, sqlglot 25+, psycopg 3, pytest 8+, MCP 1+ - -## Global Constraints - -- Preserve the public `QueryPilot`, FastAPI, CLI, and MCP interfaces. -- Retain the MIT/package artifacts inherited from `origin/launch/pr-a-package-metadata`. -- Add no mandatory dependency. -- Treat database roles as the final privilege boundary; application validation must still fail closed. -- Write each regression test before its production change and observe the expected failure. - ---- - -### Task 1: AST Statement and Function Safety - -**Files:** -- Modify: `src/querypilot/core/config.py` -- Modify: `src/querypilot/validation/validator.py` -- Modify: `src/querypilot/validation/policies.py` -- Test: `tests/test_validation.py` - -**Interfaces:** -- Consumes: `SafetyPolicy`, `sqlglot.exp.Expression`, `ValidationResult`. -- Produces: `SafetyPolicy.blocked_functions`, `SafetyPolicy.allowed_functions`, AST-based `statement_safety` and `function_safety` policy checks. - -- [ ] **Step 1: Add failing regression tests for literals, nested writes, and default-blocked PostgreSQL functions** - -Add tests that assert: - -```python -def test_dangerous_word_inside_literal_or_comment_is_allowed(demo_db_url: str) -> None: - qp = QueryPilot.connect(demo_db_url, dialect="sqlite") - literal = qp.validate_sql("SELECT 'drop' AS harmless FROM customers") - comment = qp.validate_sql("SELECT customer_name FROM customers -- drop is documentation") - assert literal.valid is True - assert comment.valid is True - - -def test_nested_write_expression_is_rejected(demo_db_url: str) -> None: - schema = QueryPilot.connect(demo_db_url, dialect="sqlite").get_schema() - validator = SQLValidator(QueryPilotConfig(dialect="postgres")) - result = validator.validate( - "WITH changed AS (DELETE FROM customers RETURNING id) SELECT * FROM changed", - schema, - ) - assert result.valid is False - assert result.blocked_reason == "SQL contains a non-read-only operation: DELETE" - - -@pytest.mark.parametrize( - "function_name", - ["nextval", "setval", "pg_terminate_backend", "pg_cancel_backend", - "pg_read_file", "pg_read_binary_file", "pg_ls_dir", "pg_sleep"], -) -def test_dangerous_postgres_functions_are_rejected(function_name: str) -> None: - validator = SQLValidator(QueryPilotConfig(dialect="postgres")) - result = validator.validate(f"SELECT {function_name}('x')", DatabaseSchema(dialect="postgres")) - assert result.valid is False - assert result.blocked_reason == f"SQL function is blocked by policy: {function_name}" -``` - -- [ ] **Step 2: Run the focused tests and verify RED** - -Run: - -```bash -.venv/bin/pytest tests/test_validation.py -k 'dangerous_word or nested_write or dangerous_postgres' -v -``` - -Expected: literal/comment test fails under raw keyword scanning; nested write and function cases are accepted or fail for the wrong reason. - -- [ ] **Step 3: Add configurable function policy and AST operation checks** - -In `policies.py`, replace raw dangerous-keyword policy use with constants: - -```python -BLOCKED_POSTGRES_FUNCTIONS = { - "nextval", "setval", "pg_terminate_backend", "pg_cancel_backend", - "pg_read_file", "pg_read_binary_file", "pg_ls_dir", "pg_sleep", -} -``` - -In `SafetyPolicy`, add case-insensitive configurable fields: - -```python -blocked_functions: list[str] = Field( - default_factory=lambda: sorted(BLOCKED_POSTGRES_FUNCTIONS) -) -allowed_functions: list[str] | None = None -``` - -In `validator.py`, remove `_contains_dangerous_keyword`. Add helpers that: - -```python -def _non_readonly_operation(expression: exp.Expression) -> str | None: - blocked_types = (exp.Alter, exp.Command, exp.Copy, exp.Create, exp.Delete, - exp.Drop, exp.Insert, exp.Merge, exp.TruncateTable, exp.Update) - for node in expression.walk(): - if isinstance(node, blocked_types): - return type(node).__name__.upper() - return None - - -def _function_name(function: exp.Func) -> str: - if isinstance(function, exp.Anonymous): - return function.name.lower() - return function.sql_name().lower() -``` - -Walk `exp.Func` nodes, reject blocked names, and when `allowed_functions` is non-null reject names outside it. Emit one `function_safety` policy check; blocked functions take precedence over the allowlist. - -- [ ] **Step 4: Add and verify optional allowlist tests** - -```python -def test_function_allowlist_fails_closed() -> None: - validator = SQLValidator(QueryPilotConfig( - dialect="postgres", - safety_policy=SafetyPolicy(allowed_functions=["count"]), - )) - schema = DatabaseSchema(dialect="postgres") - assert validator.validate("SELECT COUNT(*)", schema).valid is True - result = validator.validate("SELECT lower('A')", schema) - assert result.valid is False - assert result.blocked_reason == "SQL function is not allowed by policy: lower" -``` - -Run: `.venv/bin/pytest tests/test_validation.py -v` - -Expected: all validation tests pass. - -- [ ] **Step 5: Commit Task 1** - -```bash -git add src/querypilot/core/config.py src/querypilot/validation/policies.py src/querypilot/validation/validator.py tests/test_validation.py -git commit -m "fix: enforce AST statement and function safety" -``` - ---- - -### Task 2: Alias-Aware, Fail-Closed Column Policies - -**Files:** -- Modify: `src/querypilot/validation/validator.py` -- Test: `tests/test_access_policy.py` -- Test: `tests/test_validation.py` - -**Interfaces:** -- Consumes: parsed `exp.Select`, `DatabaseSchema`, `AccessPolicy`. -- Produces: `_RelationScope`, resolved `(table, column)` references, explicit star exposures, unknown/ambiguous column errors. - -- [ ] **Step 1: Add failing tests for the three reproduced access-policy bypasses** - -Add to `tests/test_access_policy.py`: - -```python -@pytest.mark.parametrize( - "sql", - [ - "SELECT * FROM customers", - "SELECT c.email FROM customers AS c", - "SELECT email FROM customers JOIN orders ON customers.id = orders.customer_id", - ], -) -def test_blocked_columns_cannot_be_bypassed(tenant_db_url: str, sql: str) -> None: - qp = QueryPilot.connect( - tenant_db_url, - access_policy=AccessPolicy(blocked_columns={"customers": ["email"]}), - ) - result = qp.validate_sql(sql) - assert result.valid is False - assert result.blocked_reason == "Column is blocked by access policy: customers.email" -``` - -Extend the fixture with a schema-valid join target: - -```python -conn.executescript( - """ - CREATE TABLE orders ( - id INTEGER PRIMARY KEY, - customer_id INTEGER NOT NULL, - amount INTEGER NOT NULL - ); - INSERT INTO orders (customer_id, amount) VALUES (1, 1000); - """ -) -``` - -- [ ] **Step 2: Run bypass tests and verify RED** - -Run: - -```bash -.venv/bin/pytest tests/test_access_policy.py -k 'cannot_be_bypassed' -v -``` - -Expected: star, alias, and multi-table unqualified cases are accepted. - -- [ ] **Step 3: Implement relation scope and column resolution** - -Add a focused internal model: - -```python -@dataclass(frozen=True) -class _RelationScope: - tables: tuple[str, ...] - aliases: dict[str, str] - - def resolve_table(self, qualifier: str) -> str | None: - normalized = _normalize_identifier(qualifier).lower() - return self.aliases.get(normalized) -``` - -Build it from `exp.Table` nodes using `table.alias_or_name`. Resolve each non-star `exp.Column` as follows: - -```python -if column.table: - table_name = scope.resolve_table(column.table) -else: - candidates = [name for name in scope.tables - if (table := schema.get_table(name)) and table.get_column(column.name)] - if len(candidates) == 1: - table_name = candidates[0] - elif not candidates: - errors.append(f"Unknown column: {column.name}") - else: - errors.append(f"Ambiguous column: {column.name}") -``` - -Use the same resolved references for known-column validation and `_access_policy_errors`. - -- [ ] **Step 4: Implement star exposure enforcement** - -Treat `exp.Column` with `column.name == "*"` and bare `exp.Star` as exposures. Resolve `alias.*` through the relation map; bare stars expose every table. For each exposed table: - -```python -blocked_for_table = blocked.get(table_name, set()) -if blocked_for_table: - column = sorted(blocked_for_table)[0] - errors.append(f"Column is blocked by access policy: {table_name}.{column}") - -allowed_for_table = allowed.get(table_name) -if allowed_for_table is not None: - exposed = {column.name.lower() for column in table_schema.columns} - disallowed = sorted(exposed - allowed_for_table) - if disallowed: - errors.append(f"Column is not allowed by access policy: {table_name}.{disallowed[0]}") -``` - -- [ ] **Step 5: Add unknown/ambiguous and allowed-column tests** - -Add these concrete cases: - -```python -def test_allowed_columns_apply_through_alias_and_star(tenant_db_url: str) -> None: - qp = QueryPilot.connect( - tenant_db_url, - access_policy=AccessPolicy(allowed_columns={"customers": ["customer_name"]}), - ) - alias = qp.validate_sql("SELECT c.revenue FROM customers AS c") - star = qp.validate_sql("SELECT c.* FROM customers AS c") - assert alias.blocked_reason == "Column is not allowed by access policy: customers.revenue" - assert star.valid is False - assert star.blocked_reason is not None - assert star.blocked_reason.startswith("Column is not allowed by access policy: customers.") - - -def test_multitable_columns_fail_closed(tenant_db_url: str) -> None: - qp = QueryPilot.connect(tenant_db_url) - unknown = qp.validate_sql( - "SELECT missing FROM customers JOIN orders ON customers.id = orders.customer_id" - ) - ambiguous = qp.validate_sql( - "SELECT id FROM customers JOIN orders ON customers.id = orders.customer_id" - ) - qualified = qp.validate_sql( - "SELECT customers.id, orders.amount FROM customers " - "JOIN orders ON customers.id = orders.customer_id" - ) - assert unknown.blocked_reason == "Unknown column: missing" - assert ambiguous.blocked_reason == "Ambiguous column: id" - assert qualified.valid is True -``` - -Run: - -```bash -.venv/bin/pytest tests/test_access_policy.py tests/test_validation.py -v -``` - -Expected: all focused tests pass. - -- [ ] **Step 6: Run the full suite and commit Task 2** - -Run: `.venv/bin/pytest -q` - -Expected: all tests pass; update only assertions whose former behavior represented a documented fail-open path. - -```bash -git add src/querypilot/validation/validator.py tests/test_access_policy.py tests/test_validation.py -git commit -m "fix: resolve aliases and stars in column policies" -``` - ---- - -### Task 3: PostgreSQL Read-Only Transaction Enforcement - -**Files:** -- Modify: `src/querypilot/connectors/postgres.py` -- Modify: `tests/test_postgres_connector.py` -- Modify: `README.md` - -**Interfaces:** -- Consumes: `PostgresConnector.engine.connect()`, SQLAlchemy `Connection.execute`. -- Produces: ordered `SET TRANSACTION READ ONLY`, `SET LOCAL statement_timeout`, validated query execution. - -- [ ] **Step 1: Add a failing connector-order test** - -Add this fake-backed connector test: - -```python -from types import SimpleNamespace - - -class _FakeResult: - def __init__(self, rows=None) -> None: - self.rows = rows or [] - - def close(self) -> None: - return None - - def fetchall(self): - return self.rows - - -class _FakeConnection: - def __init__(self, executed: list[str]) -> None: - self.executed = executed - - def __enter__(self): - return self - - def __exit__(self, *_args) -> None: - return None - - def execute(self, statement): - sql = str(statement) - self.executed.append(sql) - if sql == "SELECT 1 AS value": - return _FakeResult([SimpleNamespace(_mapping={"value": 1})]) - return _FakeResult() - - -class _FakeEngine: - def __init__(self, executed: list[str]) -> None: - self.connection = _FakeConnection(executed) - - def connect(self): - return self.connection - - -def test_execute_readonly_sets_transaction_before_timeout_and_query() -> None: - executed: list[str] = [] - connector = object.__new__(PostgresConnector) - connector.timeout_seconds = 10 - connector.engine = _FakeEngine(executed) - - rows, row_count = connector.execute_readonly("SELECT 1 AS value") - -assert executed == [ - "SET TRANSACTION READ ONLY", - "SET LOCAL statement_timeout = 10000", - "SELECT 1 AS value", -] -assert rows == [{"value": 1}] -assert row_count == 1 -``` - -- [ ] **Step 2: Run the connector test and verify RED** - -Run: `.venv/bin/pytest tests/test_postgres_connector.py -v` - -Expected: only timeout and query commands are recorded; read-only command is missing. - -- [ ] **Step 3: Implement the minimal connector change** - -At the start of `execute_readonly`: - -```python -readonly_result = conn.execute(text("SET TRANSACTION READ ONLY")) -readonly_result.close() -timeout_result = conn.execute( - text(f"SET LOCAL statement_timeout = {self.timeout_seconds * 1000}") -) -timeout_result.close() -``` - -Then execute and fetch the validated query as before. - -- [ ] **Step 4: Document the database-role requirement** - -Add this paragraph under the safety guidance: - -```markdown -For PostgreSQL production use, connect QueryPilot with a dedicated -least-privilege role that has only the required schema `USAGE` and table -`SELECT` grants. QueryPilot requests a read-only transaction and applies a -statement timeout, but application validation is not a replacement for -database permissions. -``` - -- [ ] **Step 5: Verify and commit Task 3** - -Run: - -```bash -.venv/bin/pytest tests/test_postgres_connector.py tests/test_querypilot_flow.py -v -``` - -Expected: all focused tests pass. - -```bash -git add src/querypilot/connectors/postgres.py tests/test_postgres_connector.py README.md -git commit -m "fix: enforce read-only PostgreSQL transactions" -``` - ---- - -### Task 4: End-to-End Security and Distribution Verification - -**Files:** -- Modify: `suites/safety.yaml` -- Modify: `.eval/baseline.json` only if deterministic report structure requires it -- Test: `tests/test_mcp_runtime.py` - -**Interfaces:** -- Consumes: complete validator, connector, MCP server, eval CLI, Hatch build. -- Produces: durable adversarial safety cases and verified distributable artifacts. - -- [ ] **Step 1: Add safety-suite cases for confirmed bypasses** - -Append these cases to `suites/safety.yaml`: - -```yaml - - id: blocks_privileged_function - sql: "SELECT pg_read_file('/etc/passwd')" - should_pass: false - expected_failure_kind: validation - expected_error_contains: ["SQL function is blocked by policy: pg_read_file"] - tags: [safety, function] - - - id: blocks_nested_delete - sql: "WITH changed AS (DELETE FROM customers RETURNING id) SELECT * FROM changed" - should_pass: false - expected_failure_kind: validation - expected_error_contains: ["SQL contains a non-read-only operation: DELETE"] - tags: [safety, mutation, cte] -``` - -Add this MCP regression test and import `AccessPolicy`: - -```python -def test_mcp_access_policy_cannot_be_bypassed_by_alias_or_star(demo_db_url: str) -> None: - qp = QueryPilot.connect( - demo_db_url, - dialect="sqlite", - access_policy=AccessPolicy(blocked_columns={"customers": ["revenue"]}), - ) - server = create_mcp_server(qp, fastmcp_cls=FakeFastMCP) - - alias_result = server.tools["execute_sql"]( - "SELECT c.revenue FROM customers AS c" - ) - star_result = server.tools["execute_sql"]("SELECT * FROM customers") - - assert alias_result["error"].startswith("SQL validation failed") - assert "customers.revenue" in alias_result["error"] - assert star_result["error"].startswith("SQL validation failed") - assert "customers.revenue" in star_result["error"] - assert set(server.tools) == { - "ask_database", "search_schema", "validate_sql", "execute_sql" - } -``` - -- [ ] **Step 2: Run focused MCP and eval verification** - -```bash -.venv/bin/pytest tests/test_mcp_runtime.py -v -.venv/bin/querypilot eval run --suite suites/smoke.yaml --generator demo --report /tmp/querypilot-smoke.json --no-color -.venv/bin/querypilot eval run --suite suites/safety.yaml --generator demo --report /tmp/querypilot-safety.json --no-color -``` - -Expected: MCP tests pass; smoke and safety reports have no threshold violations. - -- [ ] **Step 3: Run complete verification** - -```bash -.venv/bin/pip install build twine -.venv/bin/pytest -q -.venv/bin/python -m build -.venv/bin/python -m twine check dist/* -``` - -Expected: all tests pass, wheel and sdist build, and Twine reports both artifacts `PASSED`. - -- [ ] **Step 4: Inspect package and Git state** - -```bash -git diff --check -git status --short --branch -git log --oneline --decorate -6 -``` - -Confirm `LICENSE`, `CHANGELOG.md`, `CONTRIBUTING.md`, and `src/querypilot/py.typed` remain present and no generated artifact is staged. - -- [ ] **Step 5: Commit Task 4** - -```bash -git add suites/safety.yaml tests/test_mcp_runtime.py .eval/baseline.json -git commit -m "test: cover SQL security regressions end to end" -``` - -If `.eval/baseline.json` did not change, omit it from `git add`. Do not commit `dist/`. diff --git a/docs/superpowers/specs/2026-07-11-security-hardening-design.md b/docs/superpowers/specs/2026-07-11-security-hardening-design.md deleted file mode 100644 index a26d6ba..0000000 --- a/docs/superpowers/specs/2026-07-11-security-hardening-design.md +++ /dev/null @@ -1,130 +0,0 @@ -# QueryPilot Security Hardening Design - -## Objective - -Harden QueryPilot's SQL validator and PostgreSQL connector so the documented -read-only and column-access guarantees fail closed for the bypasses reproduced -during review. Preserve MCP, server, library, and eval compatibility, and build -on the MIT/package metadata already present in PR #13. - -## Scope - -This change will: - -- enforce blocked and allowed column policies through table aliases; -- reject `SELECT *` when it could expose a blocked or non-allowlisted column; -- resolve unqualified columns in multi-table queries against the schema and - fail closed when resolution is unknown or ambiguous; -- stop treating dangerous words inside string literals or comments as SQL - operations; -- reject dangerous PostgreSQL functions that can mutate state, terminate - sessions, inspect server files, or create denial-of-service behavior; -- expose configurable function deny/allow policy through `SafetyPolicy`; -- execute PostgreSQL queries in a database-enforced read-only transaction; -- add regression coverage for each confirmed bypass and connector behavior; -- retain PR #13's MIT license, package metadata, changelog, contribution guide, - and `py.typed` marker. - -This change will not add a new SQL parser, database connector, authentication -system, secrets manager, or mandatory dependency. It will not claim that -application validation replaces a least-privilege database role. - -## Validator Architecture - -The validator will continue to parse with `sqlglot`, but security decisions -will use the parsed expression tree instead of raw keyword matching. - -### Statement safety - -The root statement must remain a single `SELECT`. The validator will also walk -the tree for write or administrative expression nodes so a write hidden inside -a CTE or nested construct cannot inherit the root `SELECT` classification. -String literals and comments are data, not operations, so words such as -`'drop'` inside them will not cause false positives. - -### Relation and column resolution - -The validator will build a relation map for each referenced table: - -- the real table name maps to itself; -- each table alias maps back to the real table; -- qualified columns resolve through this map; -- an unqualified column resolves only when exactly one referenced schema table - contains that column; -- zero matches produce an unknown-column error; -- multiple matches produce an ambiguous-column error. - -This resolution feeds both schema validation and access-policy enforcement so -the two checks cannot disagree about what `c.email` refers to. - -For stars, the validator will expand the exposure conceptually from the known -schema. An unqualified star covers every referenced table; `alias.*` covers the -resolved table. A star is rejected if any exposed table has blocked columns or -if an allowed-column policy does not include every exposed column. Existing -`SafetyPolicy.allow_select_star=False` remains the stricter global switch. - -### Function policy - -`SafetyPolicy` will gain: - -- `blocked_functions`, seeded with dangerous PostgreSQL functions such as - `nextval`, `setval`, `pg_terminate_backend`, `pg_cancel_backend`, - `pg_read_file`, `pg_read_binary_file`, `pg_ls_dir`, and `pg_sleep`; -- optional `allowed_functions`; when configured, every called SQL function must - appear in the allowlist. - -Names will be compared case-insensitively after dialect-normalized parsing. -Blocked functions win over allowed functions. A rejection is a high-severity -policy failure with an actionable function name. - -## PostgreSQL Defense in Depth - -`PostgresConnector.execute_readonly` will make `SET TRANSACTION READ ONLY` the -first command in the SQLAlchemy transaction, then apply `SET LOCAL -statement_timeout`, then execute the validated query. Tests will verify command -order and that the result path remains unchanged. - -Documentation will state that deployments must still use a dedicated database -role with only the required `SELECT` and schema privileges. Read-only -transactions do not neutralize every privileged PostgreSQL function, which is -why function policy remains necessary. - -## Compatibility and Error Handling - -Public method and MCP tool names remain unchanged. Validation failures continue -to return structured `ValidationResult` data and MCP errors. New failures will -use explicit messages for blocked functions, unknown multi-table columns, -ambiguous columns, and star-based access-policy exposure. - -Existing valid joins, aggregates, scalar functions, SQLite queries, row -filters, masking, audit records, and limit rewriting must remain compatible. - -## Test Strategy - -Implementation will follow red-green-refactor cycles. Regression tests will be -written and observed failing before production changes. - -Required cases: - -- blocked column through `SELECT *`; -- blocked column through a table alias; -- blocked unqualified column in a multi-table join; -- allowed-column policy through stars and aliases; -- unknown and ambiguous multi-table columns; -- harmless dangerous-looking words in literals and comments; -- each default dangerous PostgreSQL function category; -- optional function allowlist behavior; -- write nodes nested beneath a query when supported by the parser; -- PostgreSQL transaction-read-only and timeout command ordering; -- full unit suite, smoke eval, safety eval, and live MCP stdio probe. - -## Acceptance Criteria - -- Every reproduced bypass is rejected by an automated regression test. -- Harmless literals containing blocked-operation words validate successfully. -- PostgreSQL execution begins with a read-only transaction command. -- Existing public APIs and MCP schemas do not change. -- The complete test suite, smoke eval, safety eval, package build, and MCP probe - pass from the isolated branch. -- The branch contains PR #13's open-source/package artifacts and introduces no - new mandatory dependency. diff --git a/src/querypilot/evals/suite_runner.py b/src/querypilot/evals/suite_runner.py index 451fc63..d29d9fc 100644 --- a/src/querypilot/evals/suite_runner.py +++ b/src/querypilot/evals/suite_runner.py @@ -18,7 +18,6 @@ from querypilot.evals.suite import ( BenchmarkCase, BenchmarkSuite, - ComparisonConfig, SuiteThresholds, ) diff --git a/tests/evals/test_check.py b/tests/evals/test_check.py index 6a97715..ca03031 100644 --- a/tests/evals/test_check.py +++ b/tests/evals/test_check.py @@ -8,12 +8,10 @@ from querypilot.evals import ( CaseResult, - CheckOutcome, FailureCategory, StageTimings, SuiteReport, SuiteThresholds, - TagRollup, check_report, format_outcome, load_report, diff --git a/tests/evals/test_pipeline.py b/tests/evals/test_pipeline.py index 86a2dab..d6b1ade 100644 --- a/tests/evals/test_pipeline.py +++ b/tests/evals/test_pipeline.py @@ -5,13 +5,11 @@ import pytest from querypilot import QueryPilot -from querypilot.core.types import DatabaseSchema, GeneratedSQL, ValidationResult +from querypilot.core.types import GeneratedSQL from querypilot.evals import ( BenchmarkCase, CaseResult, FailureCategory, - NullCostTracker, - StageTimings, run_case, ) diff --git a/tests/evals/test_report.py b/tests/evals/test_report.py index 72960a5..c62da0d 100644 --- a/tests/evals/test_report.py +++ b/tests/evals/test_report.py @@ -4,11 +4,9 @@ from datetime import UTC, datetime from pathlib import Path -import pytest from querypilot.evals import ( CaseResult, - FailureCategory, StageTimings, SuiteReport, SuiteThresholds,