Commit Graph

2 Commits

Author SHA1 Message Date
cproudlock
5fa5160420 Apply skill-driven review fixes: security, hook isolation, tests, docs
Addresses findings from a 6-lens review against the project skills
(defining-asset-contract, enforcing-plugin-contract, hardening-flask-config,
integrating-plugin-hooks, pinning-flask-behavior, simplifying-python).

Security (hardening-flask-config):
- Load per-plugin COLLECTOR_API_KEY_<PLUGIN> from env in create_app. from_object
  only copies class attributes, so per-plugin keys (ADR-006) were dead in real
  deploys and silently fell back to the shared key.
- EMPLOYEE_DB_USER/PASSWORD no longer default to root/rootpassword (no safe
  default for a secret; unset fails loud). Documented in .env.example + DEPLOY.md.
- COLLECTOR_API_KEY + per-plugin + EMPLOYEE_DB_* added to .env.example/DEPLOY.md.

Hook isolation (integrating-plugin-hooks):
- collector _collector_plugins and dashboard get_navigation now re-raise in
  dev/test and log+isolate in prod, instead of silently swallowing a broken
  plugin hook.

Plugin loader (enforcing-plugin-contract):
- enable_plugin/install_plugin read dependencies+version from the manifest
  instead of instantiating the plugin class.
- _register_plugin_components rejects a second plugin claiming an already-used
  api_prefix (reset per app in init_app).

Tests (pinning-flask-behavior):
- test_identifiers.py: gauge/maintenance round-trip on computer/printer/network
  create+update; per-type seed yields the 12 identifier keys.
- contract tests for apply_collector_payload presence + schema-declarers-implement.
- security tests for per-plugin key env loading + no employee-db password default.

Docs/contract sync (defining-asset-contract):
- PLUGIN-HOOKS.md documents apply_collector_payload; stale 0.2.0 -> 0.3.0.
- ADR-006 documents apply_collector_payload + single-dispatch rationale.
- ADR-001 enumerates the expanded shopdb.api import surface.

Simplify (simplifying-python):
- De-duplicate the 21-entry settings defaults: shared build_default_settings()
  used by both the /settings/seed route and the CLI (were drifting copies).
- Remove dead AssetStatus import + redundant AssetType local import in computers
  plugin; comment the statusid=1 collector default.

153 tests pass (was 145), naming/style green.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-26 19:25:52 -04:00
cproudlock
c70bc37642 Phase 1: pytest baseline, production hardening, pinned requirements
Establishes the safety net required before any structural refactor.

Tests (tests/):
- conftest.py rewritten for Flask-SQLAlchemy 3.x (drop-recreate per
  test, StaticPool-shared in-memory SQLite, admin_user + auth_headers
  fixtures). Removes deprecated db.create_scoped_session pattern.
- test_smoke.py: 8 baseline tests (app boot, JWT login valid+invalid,
  protected routes, paginated response shape, plugin auto-discovery).
- test_security_config.py: 7 tests pinning ProductionConfig.validate
  failure modes (missing/dev SECRET_KEY, missing JWT_SECRET_KEY,
  missing DATABASE_URL, wildcard CORS, empty CORS) and one happy-path.

Production hardening (shopdb/config.py, shopdb/__init__.py):
- ProductionConfig.validate() raises ConfigError on missing or
  insecure SECRET_KEY, JWT_SECRET_KEY, DATABASE_URL, CORS_ORIGINS.
  No silent fallback to dev defaults in production.
- create_app invokes validate() when config_name == 'production'.
- CORS_ORIGINS default no longer wildcard; defaults to localhost
  Vite dev origin.
- Drop os.path.exists probe in serve_frontend (path-traversal risk
  surface). send_from_directory handles safe-join + 404 itself.
- Replace User.query.get with db.session.get (SQLAlchemy 2.0 API).

TestingConfig (shopdb/config.py):
- Add StaticPool + check_same_thread connect_args so SQLite in-memory
  is shared across the test session.

Index dedup (plugins/printers/models/printer_extension.py):
- Rename idx_printer_windowsname -> idx_printerdata_windowsname.
  Two model classes (Printer, PrinterData) declared the same index
  name; SQLite enforces global index uniqueness even across tables.
  Per CONTRIBUTING.md naming convention, indexes follow
  idx_<table>_<column>.

Dependency pinning (requirements.in, requirements.txt):
- requirements.in holds the loose source pins (the human-edited file).
- requirements.txt is now a uv-compiled lockfile (every transitive
  dep pinned to an exact version). Reproducible builds. Run
  `uv pip compile requirements.in -o requirements.txt` to refresh.

Test count: 0 -> 15 passing. All naming/style checks still green.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-08 14:48:19 -04:00