Skip to content

fix(data): harden fetch boundary + resolve segmented wall + honest missing signals#61

Closed
jungdaesuh wants to merge 1 commit into
developfrom
bugfix/data-fetch-hardening
Closed

fix(data): harden fetch boundary + resolve segmented wall + honest missing signals#61
jungdaesuh wants to merge 1 commit into
developfrom
bugfix/data-fetch-hardening

Conversation

@jungdaesuh

Copy link
Copy Markdown
Collaborator

Summary

Hardens the data-fetch boundary and fixes two silent data defects surfaced by an
adversarial (Crucible) review. Covered by regression tests.

Related issue

Follow-up to the shot-segmented geometry migration (the wall schema change).

Type of change

  • Analysis core (src/magnetics/core)
  • Data source (src/magnetics/data)
  • Service / API (src/magnetics/service)
  • Web GUI (gui/web)
  • Docs
  • Other

What changed

  • _slcontour/omfit_compat.load_wall: read the migrated segmented wall schema via
    devices.feature_at (was reading the old flat r/z keys → (None, None) → the
    vessel outline was silently dropped from the live sensor_map_rz node);
    service/nodes threads the shot through. (Verified live: load_wall('DIII-D', shot)
    now returns the 257-point outline.)
  • data/fetch/remote.py: validate remote_dir at the run_remote consumption
    boundary — it was interpolated raw into cluster shell commands (mkdir -p, cd),
    allowing shell-metacharacter injection and, for --leading values (-tmp, --help),
    option/argument injection. Rejected loudly (external-input security boundary).
  • data/fetch/toksearch.py: record every requested EFIT-tree signal as missing
    when MdsSignal is unavailable — they were silently dropped (in neither the fetched
    nor the missing report), unlike the mdsthin path.
  • service/mock.py: seed the mock RNG from zlib.crc32 (stable across restarts)
    instead of the per-process-salted builtin hash().

How I tested it

uv sync --extra service; uv run ruff format --check . / ruff check . /
ty check src/magnetics → clean; uv run --with pytest pytest -q → green
(tests/test_data_fetch_hardening.py: load_wall segmented-schema resolution +
remote_dir accept/reject cases including --leading option injection).

Checklist

  • CI is green (analysis: ruff + pytest; web: lint + build) — validated locally
  • No secrets/credentials (tokens, SSH keys, cluster logins) committed
  • No large files or tokamak data committed
  • Physics kept in core (device-agnostic); none added to service routes
  • Docs updated if behavior/usage changed (n/a)

🤖 Generated with Claude Code

…ssing signals

- _slcontour/omfit_compat.load_wall: read the migrated segmented wall schema via
  devices.feature_at (was reading old flat keys → (None,None) → vessel outline
  dropped from the live sensor_map_rz node); service/nodes passes the shot through.
- data/fetch/remote.py: validate remote_dir at the run_remote consumption boundary
  — it was interpolated raw into cluster shell commands (mkdir/cd), allowing shell
  metacharacter injection and, for "-tmp"/"--help", option/argument injection.
  Reject loudly (external-input security boundary).
- data/fetch/toksearch.py: record every requested EFIT-tree signal as missing when
  MdsSignal is unavailable — they were silently dropped (in neither got nor missing).
- service/mock.py: seed the mock RNG from zlib.crc32 (stable across restarts) instead
  of the per-process-salted builtin hash().

Adds tests/test_data_fetch_hardening.py (load_wall segmented schema + remote_dir
accept/reject cases). ruff + ty(src) clean; full suite green.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
@matt-pharr

Copy link
Copy Markdown
Collaborator

Superseded by #68, which is a rebased superset of this PR on current develop (contains all fixes here: segmented-wall loading via omfit_compat, remote_dir validation, missing-signal preservation, and the crc32 mock RNG seed). This PR was also CONFLICTING against develop; #68 is CLEAN. Closing as a duplicate — review/merge #68 instead.

@matt-pharr matt-pharr closed this Jul 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants