refactor!: security hardening, standards alignment, and unified API (v0.9.0)#32
Merged
Merged
Conversation
…aths Close path traversal across every user-supplied filesystem path in both the modular server and the legacy monolith: - train_anomaly_model built its pickle output path from an unvalidated model_name (arbitrary-file-write reachable by any MCP client); the model-load sites and read_report_metadata read path were likewise unvalidated. - load_signal_data (the shared read sink behind every analysis tool) allowed arbitrary file-content reads via a traversal signal_file. Introduce a canonical path_safety module (safe_resolve via Path.is_relative_to, sanitize_filename, validate_name_component, resolve_model_paths -> ModelPaths), re-exported from mcp_tools._utils, and route every write/read site through it. Invalid names are rejected before any I/O; the diagnosis pipeline degrades to None rather than aborting. Add a 52-case security regression suite covering the sibling-directory bypass (regression of 61627b0 -> d689886). Bump to 0.8.1 (security-only; features stay under [Unreleased]). Realign CITATION.cff from stale 0.7.1. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Document the v0.8.1 fix as a searchable docs/solutions entry: the str.startswith sibling-directory bypass, the single-source-of-truth path_safety choke point, validate-don't-rewrite for security names, and the cross-platform backslash test gotcha. Surface docs/solutions/ in CLAUDE.md so future agents discover it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…st citation One threshold table (ISO 10816-3:2009 values, 4-zone scheme) lives in diagnostics/iso10816.py; alerts.py's drifted duplicate is gone and every path now classifies identically. Consumers switch from the invented machine_class I-IV mapping to native machine_group/support_type. The engine refuses assessment when Nyquist < 1 kHz or declared power < 15 kW, reports the real integration band, and stamps provenance (20816-3:2022 merges zones A/B) on every output.
…move fictitious references Keep only source-traceable entries: 6205 and 6203 from CWRU Bearing Data Center (with published frequency factors cross-checked in tests) and UER204 from the XJTU-SY dataset. Delete 19 entries with fabricated internal geometry misattributed to ISO 15:2017 — the old 6205 pitch (34.55 mm vs real 39.04) was contaminated from the XJTU bearing. document_reader's in-memory fallback now reads the same JSON instead of duplicating values; kinematic formulas cite Randall & Antoni 2011 instead of ISO 15243; the hardcoded 81.13 Hz 'example @ 1500 RPM' block no longer pollutes envelope output. New geometry-validation suite (36 tests) guards every entry: bore<pitch<OD, ball fit, BPFO/fr+BPFI/fr≈Z, non-empty source.
…nvented confidence estimate_rul now takes a timestamped measurement series (explicit values or multiple stored signals) and refuses single-recording extrapolation, pointing to the within-recording trend screening instead. Weibull estimator removed. Kalman delta-method variance gains the missing covariance cross-term; its precision field is explicitly heuristic. Trend direction is gated on the computed p-value, not R-squared>0.3, and onset detection can no longer fire inside its own baseline window. Confidence fields renamed to honest ones (fit_r_squared, evidence_strength derived from corroborating evidence rather than severity) and no tool accepts confidence as input anymore.
…uessing ISO severity verdicts are only produced when the signal unit is declared — via load_signal(signal_unit=...) or the companion _metadata.json — never guessed from amplitude. The RMS>0.5→'g' heuristic, the HYPOTHESIS/PROCEEDING flow and the PLEASE-CONFIRM ctx.info wall are gone. diagnose_vibration degrades instead of failing: the iso_severity block becomes a schema-level ISOSeverityRefusal (status/reason/remedy) when the unit is undeclared or Nyquist is below the ISO band, while spectral, bearing and anomaly blocks still run. sampling_rate follows the same discipline: explicit > metadata > structured error, no silent 1 or 10 kHz defaults anywhere.
…rror contract All 54 endpoints (46 tools, 4 resources, 4 prompts) are now module-level functions importable and testable without FastMCP; register() is a plain list of registrations. Protocol identity verified against a frozen inventory snapshot (byte-identical descriptions and prompt renders). Error contract unified on two rails: misuse/failures raise ValueError with problem-plus-remedy messages (FastMCP maps them to isError), and legitimate negative outcomes are typed results (BearingCatalogMiss). Error-shaped dicts returned as success and f-string JSON in resources are gone, guarded by AST source checks. New resolve_signal helper replaces the repeated repository-lookup idiom at eight sites; report_tools no longer imports from the monolith.
…r server only Delete machinery_diagnostics_server.py (5,289 lines) and the five root import shims; move the real signal_loader/signal_repository implementations to signal_acquisition/ (loaders.py, repository.py) so every import path is canonical. Rename diagnostics/iso10816.py to iso20816.py now that its last legacy importers are gone, and make the unreachable unknown-unit fallback raise. report_generator raises natively instead of returning error dicts. Migrate/delete the six monolith-bound test files and add a CI guard (test_monolith_removed.py) that fails on any reappearing monolith reference or bare ISO 10816 citation. Dockerfile health check and build check, .vscode/mcp.json, validate_server.py and operational docs repointed to the modular server. Removal ahead of the v1.0.0 promise (commit 114bed3) is deliberate: the unpatched second copy was a standing security liability. Full rationale lands in the v0.9.0 CHANGELOG migration notes. BREAKING CHANGE: machinery_diagnostics_server and root-level shim modules no longer exist; import from the package's canonical module paths.
…ository correctness Every analysis, diagnostics, prognostics and report tool now takes signal_id resolved through the shared resolve_signal helper; filename parameters and their silent 10 kHz sampling-rate defaults are gone (clean cut, no aliases). load_signal accepts str | list[str] with fail-fast atomic batch semantics: every path, derived id and collision is validated up front and nothing loads on the first problem. Repository fixes: default ids derive from the relative path (real_train/baseline_1 and real_test/baseline_1 no longer silently collide), reloading an existing id errors unless overwrite=True, absolute paths outside DATA_DIR load that exact file instead of a same-named file inside DATA_DIR, arrays are returned as read-only views over repository-owned memory, and the not-found message lists loaded ids and explains cache eviction. Analysis segments are deterministic by default with random sampling behind an explicit random_seed parameter. BREAKING CHANGE: filename/signal_file parameters removed from all analysis-era tools; use load_signal then signal_id.
… tools Four major merges with golden characterization armor (old-tool outputs snapshotted on fixed fixtures before removal, asserted equivalent through the new tools): - assess_severity absorbs evaluate_iso_20816, assess_vibration_severity, check_vibration_alert and check_custom_vibration_alert (signal_id XOR rms_velocity_mm_s, native machine_group/support_type, optional custom thresholds, machine_power_kw scope refusal) - analyze_envelope absorbs compute_envelope_spectrum_tool (500-5000 Hz default band, invalid band raises instead of clamping, detrend + Hann window before the envelope FFT kills the DC skirt on the FTF zone) - check_bearing_faults absorbs check_bearing_fault_peak_tool, check_bearing_faults_direct and lookup_bearing_and_compute_tool (bearing_id XOR frequencies XOR explicit geometry; canonical fault vocabulary and catalog source echoed) - analyze_signal_trend absorbs detect_signal_degradation_onset (onset fields folded into within-recording screening output) BREAKING CHANGE: the nine absorbed tool names no longer exist.
Minor merges: list_signals(scope), clear_signals(signal_id=None), list_html_reports absorbs get_report_info (read path stays on safe_resolve), plot_spectrum/plot_envelope/plot_iso_20816_chart fold into their generate_* reports. The four duplicate resources and the ASCII-art ISO prompt are dropped; every piece of information they exposed is reachable through tools. Loop closures: generate_test_signal writes companion metadata, auto-registers and returns StoredSignalInfo; predict_anomalies returns bounded summaries instead of per-segment arrays and its not-found error lists disk models; report filenames are timestamped; fault_types is a canonical Literal list that raises on unknown values; get_signal_info exposes complete source_metadata. Naming sweep: diagnose_vibration drops the _tool suffix, one name per concept (rpm, file_name, bearing_id, signal_id). New surface-parity test maps all 54 v0.8.x endpoints to their destinations and pins the inventory at exactly 33/0/3; W1-W8 workflow walkability tests prove the golden paths end-to-end. BREAKING CHANGE: resources removed, plot_* and get_report_info merged into report tools, several parameters renamed to the canonical vocabulary.
… drift guards Test-first: test_documented_calls.py parses every backtick tool call in plugin/**/*.md and in the rendered MCP prompt templates and validates name + kwargs against the introspected inventory; test_version_alignment pins version strings across pyproject/__init__/server.json/CITATION.cff and endpoint counts in both READMEs to the introspected surface. The guards were born red on the old docs (dead tool names, filename-era params), then all 8 skills, 2 agents, 3 commands, plugin README (which finally documents the prognostics skill) and the 3 MCP prompts were regenerated against the real 33-tool schema until green. Golden-path workflows follow load_signal(signal_unit=...) -> analyze -> assess_severity/diagnose -> report, and every regenerated document keeps the augments-expert-judgment framing. Manifests untouched beyond content (marketplace schema strictness). README endpoint counts corrected.
CHANGELOG 0.9.0 with full migration table (54 -> 36 endpoints, parameter renames, honest-field renames) and the explicit rationale for removing the monolith ahead of the v1.0.0 promise: after 0.8.1 the unmaintained twin copy was a standing security and drift liability. Version bumped to 0.9.0 across pyproject, __init__, server.json, CITATION.cff and the README BibTeX block (the last one caught by the new version-alignment guard); plugin bumped to 1.1.0 in plugin.json and marketplace.json. Untracked README drafts and stale rag_result.txt removed; .claude/worktrees/ gitignored. Pre-tag checks: mcp-name string present in README, server.json description 91 chars (limit 100), suite 861 passed / coverage 92.23%, server import smoke test green.
…ensions) Adversarial multi-agent review of the U2-U11 diff surfaced 28 findings; all are addressed. Full suite 884 passed / 17 skipped / coverage 92.17%. BLOCKING: - Envelope default band is now fs-aware: compute_envelope_spectrum takes a None sentinel and resolves the default upper edge to min(5000, Nyquist-1) so a bearing check on any fs<=10kHz signal analyzes 500-Nyquist instead of hard-raising; an explicitly over-Nyquist band still raises. fs=10000 output is byte-identical (golden tests green). - The diagnose_bearing prompt no longer fabricates signal_unit='g' on the ISO-refusal path — it collects the unit up front and instructs confirming the real unit with the user before re-loading, restoring the U2 discipline. SHOULD-FIX: - resolve_signal and the load path reject non-positive sampling rates (was ZeroDivisionError downstream); StoredSignalInfo gains gt=0. - predict_anomalies and generate_pca_visualization_report guard signals shorter than one segment with a clear message instead of an opaque sklearn error. - get_signal_info/list_signals deep-copy the cached info so callers can't mutate source_metadata/shape in the cache. - CITATION.cff preferred-citation bumped to 0.9.0; the version-alignment guard now checks both version occurrences. - Gear crest-factor screening gate reconciled to >5 across prompt and skill. - evidence_strength moderate/strong branches now have tests. NICE-TO-HAVE: - Domain-math edge cases: ISO band-coverage guard refuses fs<2106Hz (was a truncated-band verdict); Kalman no longer double-uses the first two samples; constant-baseline onset requires an epsilon margin. - Robustness: batch load_signals is atomic under one lock; get_metadata_path routes through safe_resolve; read-only-view docstring softened to match. - De-duplication: RUL/pipeline docstrings and severity labels reference the single-source ISO table; document_reader demo sources 6205 from the catalog; fault-vocabulary sync guard added. - Dead code removed (SignalInfo model, unused imports); signal-management skill gains the augments-expert-judgment framing. Deliberately not changed: the absolute-path signal read (pre-existing accepted local-first design) and the report-backend coverage omit (plan-sanctioned deferral).
… honest outputs) Knowledge-track learning distilling the five reusable patterns from the v0.9.0 refactor: single-source-of-truth + CI drift guards for the repo's recurring value-drift bug class; refuse-don't-guess for physical units and sampling rate; golden characterization for behavior-preserving merges; security rationale for removing a duplicated live copy ahead of a deprecation promise; and default-vs-explicit resolution so tightened validation doesn't reject legitimate data. Each is anchored to a concrete near-miss the adversarial review caught.
The list_html_reports traversal regression asserted 'Invalid report
filename' for a backslash payload ('..\secret.txt'), but a backslash is
only a path separator on Windows — on POSIX it is a literal filename
character, so safe_resolve correctly treats it as an in-bounds
(nonexistent) name and the read falls through to 'Report not found'. The
containment itself is correct on every platform (the forward-slash and
absolute-path payloads prove it); only the backslash case is now gated to
Windows. Fixes the Linux/macOS CI failure.
LGDiMaggio
changed the title
Reword the credibility-refactor learnings to describe failure modes and the patterns that prevent them without characterizing earlier work negatively — 'input-aligned outputs' rather than 'honest outputs', 'inferred' rather than 'guessed', 'confidently incorrect' as a failure mode to engineer against. Technical substance unchanged.
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
1 participant
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.
Summary
This release advances the project along its industrial-grade roadmap: it extends the path-safety hardening to every published code path, tightens the alignment between outputs and their declared inputs and cited standards, and consolidates the tool surface into a single, coherent golden path. It implements Phases 0 and 1 of the internal roadmap.
Final surface: 33 tools + 3 prompts + 0 resources (consolidated from 54 endpoints). Test suite: 884 passed, 17 skipped, coverage 92.17% (gate 85%). CI green across the full matrix (Python 3.11/3.12 × Windows/Linux/macOS, mypy, flake8, black).
What changed, by unit
train_anomaly_modelwrite side, the model/report read sides, the signal loader) through a singlepath_safetymodule (safe_resolve/resolve_model_paths). Adds a 43-case regression suite including the sibling-directory bypass class.iso20816.py) with a precise citation (ISO 10816-3:2009 values, 4-zone scheme, noting that ISO 20816-3:2022 merges zones A/B). The engine returns an explicit refusal when an assessment is not technically valid (Nyquist below the evaluation band, machine power out of scope).sourcefield (CWRU 6205/6203, XJTU UER204), with a geometry-validation test guarding every entry. Reference values are single-sourced from the catalog JSON.estimate_rulrequires a multi-measurement series (single-recording extrapolation is refused with a pointer to within-recording screening); output fields are named for what they measure (fit_r_squared,evidence_strengthderived from corroborating evidence); the Kalman delta-method variance gains its covariance cross-term.diagnose_vibrationdegrades gracefully — the ISO block returns a structuredrefusedresult (reason + remedy) while the spectral/bearing/anomaly blocks still run. Sampling rate follows the same discipline: explicit > companion metadata > structured error.machinery_diagnostics_server.py) and the root shims are removed so the package ships one modular server; a CI guard prevents their reintroduction.signal_idis the single analysis/report/prognostics handle; the repository gains relative-path ids, explicit collision handling, atomic batch loads, read-only array views, and a fix for absolute-path loading.generate_test_signalauto-registers,predict_anomaliesreturns bounded summaries, report filenames are timestamped).test_documented_calls.py,test_version_alignment.py).Review and follow-up fixes
After the units landed, an adversarial multi-agent review across 10 dimensions (correctness, security, cross-module consistency, domain math, test quality, dead code…) produced 28 verified findings, all addressed in
4a2e8be. Highlights:diagnose_bearingprompt no longer defaults the signal unit on the ISO-refusal path — it collects the real unit first, keeping the U5 discipline intact.Tests & verification
pytest: 884 passed, 17 skipped, coverage 92.17%; green on all six platform/version combinations.validate_server.pygreen (33 tools / 3 prompts / 0 resources).Release notes
v*tag triggers the automated, non-reversible publish pipeline (PyPI → MCP Registry); tag onmainafter merge.report_generator.py/html_templates.pyremain excluded from the coverage figure (a planned follow-up adds their backend tests), so the headline number excludes those two report backends.🤖 Generated with Claude Code