Repository navigation
v0.5.0: repo standard (0 FAIL), uv.lock, live-test runner, list_plans / find_plan_for_path / get_backup_freshness, F-04 + #1182 - #8
Merged
Conversation
- git mv backrest_mcp -> src/backrest_mcp; setuptools finds packages under src/. - Every range bounded. Runtime floors are what the production venv runs (fastmcp 3.4.0, httpx 0.28.1, pydantic 2.13.4, structlog 25.5.0), ceilings the next major (next minor for 0.x). ruff pinned exactly so the format gate does not drift. - uv.lock committed (uv 0.11.18). - ruff select expanded to the mandated families; the fixes follow in their own commits. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
No hand edits. Produced by `ruff format src tests` then `ruff check --fix src tests` with ruff 0.16.10 under the expanded select. Hand fixes for the remaining E501/SIM117 follow separately. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Long SECURITY[deferred] comments wrapped, wording unchanged. - client.post_streaming: one parenthesised async with (SIM117). - from __future__ import annotations in the two empty __init__ modules (Python appendix). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…n, CodeQL, Scorecard, Dependabot (B2 B6 B12 B14 F2-F6 F10 F11) - ci.yml: top-level `permissions: contents: read`; every action SHA-pinned with a version comment; install from uv.lock (`uv sync --frozen`); ruff format --check; a dependency audit that reads the lock (runtime and dev sets separately) after `uv lock --check`; a wheel build imported from outside the tree (catches a broken src/ packages.find). - release.yml: read at the top, `contents: write` only on the release job; checkout pin moved to the same commit as the other workflows. - secret-scan.yml (gitleaks 8.30.1, digest re-verified 2026-10-20), codeql.yml, scorecard.yml, CODEOWNERS and dependabot.yml (uv + github-actions, codeql group on both channels), modelled on loki-mcp. - tests/check_gitleaks_gate.py and tests/check_action_pins.py: two-sided gates copied from loki-mcp; both run red on a planted violation before passing the real tree. - tests/test_workflows.py: SHA pin + version comment and top-level permissions in every workflow, enforced on every PR (repo-conform only runs on forge). - .gitleaksignore: one fingerprint for the synthetic LEAKCANARY hook command in public history at 5628e2f (gitleaks 8.30.1 `curl-auth-header`). No rule- or path-wide allowlist. - .gitignore: *.key/*.pem/*.p12/*.pfx; bare `core.*` replaced by `/core` + `core.[0-9]*`, which no longer matches core.py (#581, backrest half). - README badge row: Claude Code first, License last. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…wlists (F-04) redact.py copied prunePolicy, checkPolicy, forgetPolicy, commandPrefix, schedule and retention whole. Safe against Backrest v1.13.0, but a field a later Backrest adds to one of them would have passed straight through. Each is now projected through its own allowlist (option a), recursing into Schedule/RetentionPolicy/TimeBucketedCounts; a scalar slot only accepts str/int/float/bool. - REPO_MESSAGES / PLAN_MESSAGES name the v1.13.0 JSON fields; a test pins them by hand. - unknown_policy_keys(cfg) reports unlisted key paths (names only) for the live check. - Tests: unknown sub-fields dropped at every level, non-scalar in a scalar slot dropped, and the pin test goes red when an allowlist gains a field (checked by mutation). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…1182)
- Default BACKREST_RESTORE_ALLOWED_PREFIX is ~/backrest-restore/, expanded at load. The old
/tmp/backrest-restore/ did not exist on forge, and realpath() followed whatever symlink
was planted at that name first.
- ecosystem.config.js set the /tmp path explicitly, so the code default would never have
applied in production; it now uses path.join(HOME, "backrest-restore") + "/".
- safety.check_restore_prefix() refuses, with a fixed message and no API call, a prefix that
is missing, a symlink, not a directory, not owned by the effective uid, or group/world
writable. The trailing slash is stripped first because lstat("link/") follows the link.
- One test per refusal plus the allowed case and the default; each check was mutated out
and its test went red.
- Part (b), the window between the check and Backrest creating the target, cannot be closed
from this side. Documented in the docstring; restore_snapshot stays unregistered in
production.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…, via redact.py) All three read GetConfig through redact.project_config and never see the raw config. - list_plans: the projected plans, same shape as get_config's. - find_plan_for_path(path): absolute path only (normalised, length- and NUL-checked); pure string matching against each plan's paths, with the first matching excludes/iexcludes pattern. Exclude matching approximates restic (anchored, unanchored basename, *, ?, [..], **, case-insensitive iexcludes; no negation or exclude files), so the response says "match": "approximate". - get_backup_freshness(max_age_hours=None): per plan, the last backup operation that ended SUCCESS or WARNING among its last 200 operations (GetOperations, planId + repoGuid), age, latest status and a state: ok / stale / unscheduled. Threshold: the argument, else 2x maxFrequencyHours, 2x maxFrequencyDays, or 26 h for cron. Disabled schedules are unscheduled, never stale. No cron parser added. - GetOperations moved into _get_operations_raw, shared with get_operations. - The server instructions point coverage questions at the new tools instead of get_config. - plans.py holds the pure matching/threshold logic, table-tested at the boundaries. - Every new tool passes the LEAKCANARY fixture with quoted-key secret checks; every read tool returns the generic error on a backend 500; read-only registration is now an exact set. index_snapshots is not added: do_repo_task(task="index") already sends TASK_INDEX_SNAPSHOTS behind the same BACKREST_READONLY=false gate. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…edentials (#4) - scripts/run_live_conformance.py: reads BACKREST_* in-process from BACKREST_LIVE_ENV_FILE (default ~/.secrets/forge.env, so #1181 can move the repo password without a code change), runs the live file with -p no:cacheprovider --tb=line, imports from this tree's src/, and exits non-zero on any failure (1), an unreadable env file (2), or every test skipped (3). Prints key names only. Unit-tested without a subprocess. - Live tests for list_plans, find_plan_for_path("/home/ted") and get_backup_freshness: shape plus secret absence (repo password, quoted "password"/"env" keys), bool-then-assert with fixed messages. A live check warns, by key name only, when Backrest's nested policy messages carry a field redact.py does not list (F-04). - Measured on forge 2026-10-20 with the production venv against worktree code: 10 ran, 0 skipped, 0 failed, no unknown policy fields. With wrong credentials: 8 failed, exit 1, and the credential does not appear in the output. - #4: NATS_URL is not set on the running process (checked as a bool), so NATS telemetry is off in production. connect() now takes NATS_CREDS_FILE (user_credentials) or NATS_USER + NATS_PASSWORD; half a pair warns by name and connects without. The SECURITY[deferred] comment for BKRST-1 is removed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…GELOG - fail_under = 96 (measured 96.18%, 242 passed, 2026-10-20) with the dated F8 comment. - README and AGENTS: the three new tools, src/ layout, uv-based dev setup, the live runner, the restore prefix rules. README's PM2 section no longer tells you to pass --env-file, which PM2 6.x does not have; ecosystem.config.js reads forge.env itself. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool. What Enabling Code Scanning Means:
For more information about GitHub Code Scanning, check out the documentation. |
Phase 1.6: watch tests/test_workflows.py turn the test job red on an unpinned action. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This reverts commit 0cc87f0.
Phase 1.6: the tag-pinned action is reverted; this round watches ruff turn the test job red. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This reverts commit 0ce611f.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
- IV-47 (FAIL): find_plan_for_path's exclude matcher was superlinear in a caller-chosen path: the recursive matcher re-matched every ancestor and branched on each **, cubic for the common **/name form (measured 3.4 s at 400 components; the 4096-char cap allowed 2048). It is now an NFA walk over the components, linear in components x pattern length, and normalise_path caps paths at 256 components. A differential test checks the new matcher against the old one on 4000 random cases; a time-bounded test runs four worst-case patterns at the cap. - SC-06: the live runner passes only the four BACKREST_* keys the tests use (URL, USERNAME, PASSWORD, REPO_PASSWORD), not every BACKREST_* in forge.env (which includes the MCP bearer token). Live re-run: 10 ran, 0 skipped, 0 failed. - Comments in tests/check_gitleaks_gate.py named 8.28.0 as this repo's pin; it is 8.30.1. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Owner
Author
Security audit started.
Findings will be posted here when the audit completes. |
Owner
Author
Security audit complete (round 1) — 6 findings.
Categories: Finding detail is in the internal report referenced above. |
…f umask Finding: F-03 from backrest-mcp-hardening-2026-10-p2-conformance-and-tools audit. check_restore_prefix refused only group/world-WRITABLE prefixes, so 0755 and 0750 passed while the docs said 0700. restic restores snapshot modes, so a tree whose privacy came from a 0700 parent would be readable by other local uids under a 0755 prefix. It now refuses any group or other bit (S_IRWXG | S_IRWXO). Tests for 0755/0750/0705/0740; the old check fails 4 of them. Finding: F-02 from the same audit. Five restore tests created the prefix with a plain mkdir(), so under forge's session umask 0002 it was 0775 and refused before the code under test ran. A _private_dir() helper chmods to 0700 explicitly; a test runs a restore under umask 0002. test_safety.py: 29 passed under umask 022, 002 and 077 (old fixtures: 5 failed under 002). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Finding: F-04 from backrest-mcp-hardening-2026-10-p2-conformance-and-tools audit. The live tests compared the plain repo password with the tool's JSON output, so a password containing a quote, backslash, control or non-ASCII character (escaped in JSON) would pass even if it leaked. _contains_secret() checks the raw, ASCII-escaped and UTF-8-escaped forms; used by every live leak check including p1's get_config test. Bool-then-assert kept. tests/test_live_helpers.py covers it in CI (the old substring check misses 'pa"ss'). Live re-run: 10 ran, 0 skipped, 0 failed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Finding: F-05 from backrest-mcp-hardening-2026-10-p2-conformance-and-tools audit. A plan whose schedule is disabled is 'unscheduled' and never stale, so it dropped out of stale_count while producing no backups: a monitor reading stale_count alone would see 'schedule turned off' as healthy. The response now carries unscheduled_count, and the docstring tells consumers to look at both. Semantics of stale are unchanged. Boundary tests assert both counts; the live shape check requires both. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Finding: F-06 from backrest-mcp-hardening-2026-10-p2-conformance-and-tools audit. setup-uv was SHA-pinned but installed the latest uv, so uv lock --check, the pylock export the dependency audit reads, and uv build could change between runs with no workflow edit. All three jobs now pass version: "0.13.4" (the version CI has been running green on). tests/test_workflows.py::test_setup_uv_pins_the_uv_version requires every setup-uv step to carry a version; it finds 0 of 3 on the previous ci.yml. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Finding: F-01 from backrest-mcp-hardening-2026-10-p2-conformance-and-tools audit. README/AGENTS: install and update with UV_PROJECT_ENVIRONMENT=.venv uv sync --frozen --extra dev (dev: the live runner needs pytest), then an import check and pip-audit on the venv. Rehearsed on a scratch venv built from the production venv's pip freeze: 35 vulns / 8 packages before, 'No known vulnerabilities found' after; imports OK; live runner 10 ran, 0 failed from the synced venv. The production venv itself was not touched. CHANGELOG 0.5.0 updated for F-01..F-06. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part 2 of the backrest-mcp hardening programme. Part 1 (v0.4.1, #7) fixed the
get_configsecret leak.What changes
Conformance. With
visibility: publicdeclared,repo-conform.pywent from 10 pass / 12 fail on5628e2fto 25 pass / 0 fail. One SKIP remains: B11 needs aGITHUB_TOKEN. The changes:src/layoutuv.lock; CI installs--frozenand audits the locked runtime and dev sets separatelypermissions: contents: readeverywhereuv+github-actions, with the codeql group on both channels)E F W I UP B SIM RUFplusruff format.gitignoreadds*.key/*.pem, and the barecore.*that matchedcore.pyis gone (#581)Commits are split for review.
3f1647bis the mechanical ruff format/autofix pass only, and2f91593the hand fixes. Read the feature commits on their own.F-04 (p1 audit). The six nested policy messages are no longer copied whole. Each goes through its own allowlist of v1.13.0 field names; that is option (a). A hand-written test pins the names and was seen red under mutation. The live run warns, by key name only, on any field the allowlist lacks.
#1182. The restore prefix default is now
~/backrest-restore/, andecosystem.config.jschanged too, since it hard-coded/tmp.restore_snapshotrefuses a prefix that is missing, a symlink, not a directory, foreign-owned, or group/world-writable. There is one test per refusal, and each check was mutated out to watch its test fail. Part (b), the window after the check, is documented as not closable from this side.New read-only tools:
list_plans,find_plan_for_pathandget_backup_freshness. All go throughredact.project_config, are LEAKCANARY-tested with quoted-key secret checks, and have table-driven boundary tests.find_plan_for_pathapproximates restic's exclude matching and says so in its response ("match": "approximate"). Read-only registration is now asserted as an exact set.Live tests.
scripts/run_live_conformance.pyreadsBACKREST_*in-process fromBACKREST_LIVE_ENV_FILE. Exit codes:On forge (production venv, this tree's
src/): 10 ran, 0 skipped, 0 failed. With wrong credentials it exits 1, and the credential never appears in the output.#4. NATS is off in production (
NATS_URLunset, checked as a bool).connect()now takesNATS_CREDS_FILEorNATS_USER+NATS_PASSWORD, with tests.Decisions to argue
index_snapshotswas not added. The plan put it behind the write gate.do_repo_task(task="index")already sendsTASK_INDEX_SNAPSHOTS(enum 1 inservice.protov1.13.0) behind the sameBACKREST_READONLY=falsegate. A second tool would add surface without adding capability.actionCommandmatches gitleaks 8.30.1'scurl-auth-headerrule, and it sits in public history at5628e2f..gitleaksignoreholds one fingerprint for it: no rule-wide or path-wide allowlist.get_configin favour oflist_plans. That is a/etc/forge/manifestschange.Deploy note
The
src/move breakspython -m backrest_mcp.serverfrom the repo root until the editable install is redone. The deploy is one sysadmin task, in this order:git pull.venv/bin/pip install -e .pm2 restartmkdir -m 0700 ~/backrest-restore🤖 Generated with Claude Code