security(discovery): enforce Zone isolation and retire unsafe legacy writers
Bind Agent observations, review, enrichment and telemetry to current Zone
and inventory authority. Preserve immutable MAC ownership and historical
snapshots with stale-result rejection and transactional security audits.
Retire unbound direct/range/scheduled discovery and one/bulk legacy summary
writes in both modes. Require reconciled exact inventory identity or
explicit Zone-scoped IP selection for CSV metadata imports.
Independent Blocker #1 application-security re-audit: PASS on tree
cf863aadc9. Evidence: 141 focused PostgreSQL/HTTP
tests passed; Chromium and PowerShell checks passed. Two documented baseline
source assertions remain unchanged. Production deployment is not approved.
This commit is contained in:
lussierj committed
1 parent
e898f283ab
commit
8b0d32cd6b
26 files changed
+3798
-4092
No files matched your search
@@ -0,0 +1,219 @@
|
||||
Blocker #1 discovery isolation: candidate review and compatibility contract
|
||||
=======================================================================
|
||||
Review candidate only. Blocker #1 is NOT declared PASS. No production cutover is
|
||||
authorized. Base: e898f283ab0fc21bac77b63db1c4a9ad35e21562 (Stage A / draft PR #6).
|
||||
No schema migration, new constraints, backfill, or runtime DDL is introduced.
|
||||
|
||||
Authority and observations
|
||||
-------------------------
|
||||
All machine discovery/telemetry routes retain per-Agent HTTP authentication.
|
||||
Authority comes from the persisted Agent -> Site/Zone/network assignment, never
|
||||
client Site, room, Zone, target-Agent names, or a body-supplied Agent identity.
|
||||
Conflicting body Agent IDs are rejected. Disabled/unassigned/revoked routing fails
|
||||
closed. Check-in cannot establish or change an Agent's initial assignment: use
|
||||
existing authenticated enrollment or the separately reviewed legacy transition.
|
||||
Unassigned inventory is not implicitly claimed by check-in or snapshots.
|
||||
|
||||
Discovery reports are bounded (128 items, 1 MiB body; 1,024 observations/minute per
|
||||
Agent). IP literals must be in the current Zone networks. MACs must be canonical
|
||||
ASCII unicast addresses. Explicit IDs, IP aliases, MAC aliases and supplied serial
|
||||
must agree with current inventory and immutable historical MAC ownership. Global
|
||||
historical MAC/serial collision checks reveal no foreign inventory IDs. Duplicate
|
||||
current IPs within a Zone are conflicts; distinct Zones may use the same IP.
|
||||
|
||||
Each observation has a stable server ID and version. Raw reports and original
|
||||
provenance are retained, with an atomic discovery.observed audit recording the
|
||||
current authority, raw hash, inventory fingerprint and observation version. Exact
|
||||
retries deduplicate; discovery clients must send discovered_at for a fresh scan
|
||||
(the supplied Windows scanner does). Approval requires a current, proven,
|
||||
non-superseded observation less than 15 minutes old. Legacy/null/mislinked rows
|
||||
without this evidence remain visible but cannot authorize approval or enrichment.
|
||||
There is no automatic evidence backfill, deletion, identity merge or ownership
|
||||
transfer. An authorized operator must investigate conflicts and obtain a fresh
|
||||
observation. Discovery does not implement an ownership-transfer workflow.
|
||||
|
||||
Human policy remains global administration: agents.view for discovery display;
|
||||
agents.manage for stable-ID review/enrichment; miners.control for scan/poll
|
||||
requests. The UI displays authoritative Agent/Zone and escapes all untrusted text.
|
||||
IP-only approval is unsupported. Submit discovery_id, version and one of
|
||||
add_to_inventory / ignore / non_miner. All three revalidate current authority and
|
||||
identity under locks; only that observation changes. Adding new inventory also
|
||||
requires a valid MAC. Repeat the same accepted decision/version to receive its
|
||||
recorded response without another audit or insertion. Stale/different decisions
|
||||
conflict; refresh before retrying. Audit actor is the authenticated human.
|
||||
|
||||
Read commands and enrichment
|
||||
----------------------------
|
||||
Enrichment is selected by discovery_id/version, routed to its actual observing
|
||||
Agent, and bound to the current authority and IP. No arbitrary-IP or hardcoded
|
||||
Plexus fallback exists. Scan/poll requests require an explicit registered Agent ID;
|
||||
optional UUID request_id provides explicit retry idempotency. New requests without
|
||||
request_id receive distinct IDs. Queue claim rejects legacy/unscoped/stale reads
|
||||
with atomic cancellation/audit and preserves unknown delivery outcome for any
|
||||
previous claim evidence. Existing Stage A physical rejection remains in force.
|
||||
|
||||
Before enrichment miner I/O, the upgraded Windows child submits command_id,
|
||||
discovery_id and version to authenticated POST /api/agent/discovery-target. Only
|
||||
a current claimed enrichment command returns a canonical IP. Enrichment submission
|
||||
revalidates the same binding and appends a NEW child observation for human review;
|
||||
it never resets the parent's decision or reparents history. V1/V2/V3 share the
|
||||
same boundary. Replaying old V2 identity merge inputs cannot merge inventory.
|
||||
|
||||
Known-miners returns a short-lived signed miner_version bound to Agent authority
|
||||
and the current miner ID/IP/MAC/updated_at fingerprint. Every miner/pool/API3
|
||||
snapshot or known-miner enrichment must carry this token and matching identifiers.
|
||||
All Windows polling clients propagate it and batch at 128. A concurrent update,
|
||||
revocation or reassignment may invalidate in-flight telemetry: discard the stale
|
||||
response, refetch known-miners and poll again. Do not fall back to IP matching.
|
||||
Descriptive enrichment does not change MAC/IP/serial/ownership. Raw diagnostics
|
||||
are retained in append-only audits, not empty status rows that hide measurements.
|
||||
API3 retains raw snapshots, safe descriptive metadata, actual summary/pool/board
|
||||
measurements and compatibility counters. Failed/metadata-only API3 responses do
|
||||
not replace the most recent measurements. Nested identity contradictions fail.
|
||||
|
||||
Transactions and database roles
|
||||
------------------------------
|
||||
All discovery writers/read-command claims take bounded locks in a shared order:
|
||||
advisory transaction lock 743190031, SHARE on Agent/Zone/network tables, SHARE ROW
|
||||
EXCLUSIVE on miners, then row locks. These table locks prevent phantom inserts
|
||||
from legacy writers which do not use the advisory lock. Agent routing/check-in
|
||||
uses guarded updates. No miner network I/O occurs while these transactions run.
|
||||
Audit insertion, inventory mutation and the observation decision commit together;
|
||||
any error rolls back all changes. Audit-denial and real UPDATE-trigger failure,
|
||||
concurrent approvals, cross-Zone MAC races, Zone disable and claim-vs-review are
|
||||
covered with real PostgreSQL connections and registered HTTP authorization routes.
|
||||
Lock timeout is 5 seconds; statement timeout 15 seconds. Controlled contention
|
||||
returns conflict/retry. Security favors correctness over fleet throughput here.
|
||||
|
||||
Required release review: validate privileges/lock behavior for the actual deployed
|
||||
DB role in a disposable replica and measure expected fleet load. Tests use an
|
||||
isolated database owner plus a limited role with audit INSERT denied. They do not
|
||||
certify the production role. No new uniqueness constraint is proposed, so do not
|
||||
remove the transaction/table locks without a separately reviewed replacement.
|
||||
|
||||
Reproduction
|
||||
------------
|
||||
Use a disposable checkout of this candidate with an already available backend
|
||||
image containing backend/requirements.txt. Never mount production credentials or
|
||||
.env files. From the candidate root:
|
||||
|
||||
MINEROPS_REVIEW_BACKEND_IMAGE=<existing-image> scripts/test-discovery-security.sh
|
||||
|
||||
The runner creates a unique internal Docker network, tmpfs PostgreSQL 16, no
|
||||
published ports, applies the unchanged existing Alembic chain and runs the two
|
||||
PostgreSQL discovery suites. A trap destroys only its own resources. Tests use
|
||||
simulated Agents and FastAPI's registered routes, without app startup/schedulers.
|
||||
|
||||
PowerShell (isolated PowerShell 7 host/container with simulated/loopback I/O):
|
||||
pwsh -NoProfile -File agents/windows/tests/Test-MinerOpsDiscovery.ps1
|
||||
pwsh -NoProfile -File agents/windows/tests/Test-MinerOpsControl.ps1
|
||||
pwsh -NoProfile -File agents/windows/tests/Test-MinerOpsDiagnosticTransport.ps1
|
||||
|
||||
Browser (Python 3, Node, Playwright and its Chromium/runtime installed externally):
|
||||
python3 backend/tests/browser/export_discovery_page.py . /tmp/discovery-page.html
|
||||
node backend/tests/browser/discovery-security.cjs /tmp/discovery-page.html
|
||||
|
||||
The browser test intercepts all requests, supplies harmless event-marker input,
|
||||
and verifies inert rendering and exact same-IP discovery_id/version selection.
|
||||
Its trailing baseline argument demonstrates the baseline stored-markup defect.
|
||||
|
||||
Other regression suites (explicit disposable DB guards apply):
|
||||
tests.test_stage_a_containment_postgres
|
||||
tests.test_stage_a_delivery_evidence_postgres
|
||||
tests.test_stage_a_schedule_cancellation_postgres
|
||||
tests.test_agent_zones_postgres
|
||||
tests.test_agent_credential_transition_postgres
|
||||
tests.test_agent_enrollment_postgres
|
||||
|
||||
Baseline source-test exceptions retained unchanged
|
||||
-------------------------------------------------
|
||||
backend/tests/test_agent_commands.py:
|
||||
AgentEnrichmentIsolationSourceTests.test_legacy_config_remains_explicitly_legacy_only
|
||||
requires the old minerops_require_agent_secret source string / fixed Plexus config.
|
||||
The authenticated config route already changed before this candidate. Replace the
|
||||
obsolete source assertion with real legacy/authenticated route tests in separate
|
||||
work; do not weaken credential authentication to satisfy the string assertion.
|
||||
|
||||
AgentSourceContractTests.test_deployment_preserves_local_state_and_registers_established_tasks
|
||||
blanket-bans the quoted agent-config.json filename in installer source, although
|
||||
the existing credential-recovery installer legitimately preserves/recovers local
|
||||
configuration. Follow up with behavioral installation/state-preservation tests on
|
||||
Windows. Do not remove the recovery path to satisfy the blanket source assertion.
|
||||
|
||||
Both failures reproduce on unchanged 2d89e80 and unchanged e898f28; both method
|
||||
bodies remain identical. They are not silent skips or weakened assertions here.
|
||||
The external-fixture legacy-retirement PostgreSQL test class also requires its
|
||||
own migration-case setup and is not certified by the discovery test runner.
|
||||
|
||||
Outstanding gates
|
||||
-----------------
|
||||
Independent security review of this exact isolated candidate is required before
|
||||
any Blocker #1 PASS or commit approval. Native Windows PowerShell 5.1/SYSTEM and
|
||||
fleet-scale/production-role validation remain outstanding. Unsupported older
|
||||
Agent payloads fail closed and require coordinated upgrade; this is an intentional
|
||||
protocol compatibility change, not transparent mixed-version support.
|
||||
|
||||
Stage A application containment remains a separate audited boundary. Physical
|
||||
execution is disabled; device identity binding and Blocker #3 Stages B-E remain
|
||||
outstanding. Preserve STAGE_A_OLD_AGENT_CUTOVER.txt: verified network fencing must
|
||||
precede stopping/draining/upgrading old Agents. Cached commands cannot be revoked
|
||||
by server cancellation alone. Offline/unreachable Agents stay blocked; restart,
|
||||
reconnection and rollback must not restore an unsafe executor. This candidate
|
||||
neither performs nor authorizes that cutover or any production deployment.
|
||||
|
||||
Independent-audit remediation addendum (October 16, 2026)
|
||||
-------------------------------------------------------
|
||||
The original candidate (99f515924dfc85fe1e1e98361688f1060329c090) failed independent
|
||||
review for F1-F3 in retained legacy callers. This corrected candidate requires a
|
||||
new independent audit. It does not claim Blocker #1 PASS.
|
||||
|
||||
Retired in Hub AND non-Hub modes, with HTTP 410 after existing authentication and
|
||||
permission checks:
|
||||
POST /api/miners/discover-one
|
||||
POST /api/scan-ranges/{scan_range_id}/discover-now
|
||||
POST /api/scanner/run-known-now
|
||||
POST /api/tools/legacy-summary-refresh-one
|
||||
POST /api/tools/legacy-summary-refresh-missing
|
||||
All definitions/aliases of their identity matcher, discovery writer, scheduler
|
||||
adapters, summary writer and global-IP helper reject before transport/database
|
||||
access. Startup creates no local scanner thread; status reports retired/disabled.
|
||||
Ordinary miners.update therefore cannot use discovery to replace a control board
|
||||
or clear a MAC. The dedicated board workflow and immutable MAC ownership remain.
|
||||
|
||||
Functional implication: legacy local discovery, automatic local refresh, and
|
||||
legacy one/bulk summary backfill are unavailable even outside Hub mode. Operators
|
||||
must use authenticated Agent observation/review and inventory-bound Agent polling.
|
||||
Legacy summary replies lack trustworthy identity; there is no retained unversioned
|
||||
write or in-flight read to revalidate. No denied request mutates inventory, audit,
|
||||
or telemetry. The new shared discovery architecture is preserved.
|
||||
|
||||
CSV remains an explicit global administrative workflow protected by miners.update.
|
||||
Each row requires Miner ID, current MAC, serial, or IP plus Agent Zone ID. Zone may
|
||||
come from the query/UI input or a row column; conflicting selections are rejected.
|
||||
All supplied ID/MAC/serial/IP/Site/Zone evidence must agree with current inventory.
|
||||
A failed MAC/serial/ID never falls back to global IP. Missing inventory is reported
|
||||
unmatched; contradictory or ambiguous selection returns row-specific 409, malformed
|
||||
or insufficient identification 422. A failure rolls back the entire import.
|
||||
IP-only rows need a Zone even when only one global match currently exists. Exact
|
||||
MAC/ID/serial selection remains usable by global administrators across Sites.
|
||||
CSV updates only model/type/firmware, never ownership, IP, MAC or telemetry.
|
||||
Duplicate columns are rejected rather than silently using the last value.
|
||||
|
||||
CSV acquires the existing discovery lock order, including the miners table lock,
|
||||
before resolving any row. It holds the locks through metadata changes and one
|
||||
atomic audit containing every changed inventory ID/namespace and old/new values.
|
||||
Lock contention is bounded by existing discovery timeouts; large fleet throughput
|
||||
and actual deployed-role privileges still require independent release validation.
|
||||
Preview uses identical identification checks and rolls back. Repeated imports do
|
||||
not create inventory or telemetry; each applied request retains its import audit.
|
||||
No migration or containment/control policy changes are part of this remediation.
|
||||
|
||||
Regression entrypoint: scripts/test-discovery-security.sh now includes
|
||||
tests.test_legacy_inventory_security_postgres
|
||||
with registered HTTP requests, actual PostgreSQL locks/concurrent ownership moves,
|
||||
actual audit INSERT denial rollback, retired helper/alias coverage, both modes,
|
||||
permission boundaries, ambiguous/conflicting CSV selection, legitimate imports,
|
||||
preview/replay, and retained historical snapshots. All devices are simulated or
|
||||
transport-patched; the runner uses an internal network and disposable database.
|
||||
Full execution evidence and integrity manifest are retained outside the candidate
|
||||
in the remediation evidence directory referenced by the final report.
|
||||
Reference in new issue
Block a user