fix(import,export): scope-aware writes, refuse dangerous categories by default#17
Merged
Merged
Conversation
…y default
mythy import used to write every key the YAML mentioned as long as
the catalog didn't mark it READONLY="YES". That meant a hand-edited
YAML carrying e.g. CodeNum or EnableBoard_* could put a fielded
protection relay into a FATALE state requiring SET_DB_DEFAULT and a
physical power-cycle. Catalog markers exist to flag these cases but
mythy was ignoring them outside of READONLY.
Add two layers of scope filtering on writes, both gated on
locale-independent catalog attributes:
- SKIP="YES" — identification / IP / comm config. Writing any of
these over the live Modbus connection typically drops the
connection mid-transaction. Excluded by default; --include-skip
plus --yes-i-understand to opt in.
- VISIBILITY="3" — Administrator / Menu Riservato subtree, plus
DATA-level overrides like CodeNum. Cascades from the containing
group AND honored on the DATA element itself. Excluded by default;
--include-hidden plus --yes-i-understand.
- --all is shorthand for both opt-ins; READONLY="YES" stays a hard
mythy-side refuse (the device empirically does not enforce
read-only on the wire, so this defensive guard remains load-
bearing).
Path-based per-category opt-ins (--include-ntp, etc.) from the
original spec are dropped: top-level menu names are locale-dependent
(`Network Services` in us vs `Comandi` in it), and the catalog
attributes give a strictly better classification anyway. See the
design note pinned to the issue for the full reasoning.
Banner + abort: when an --include-* flag is set without
--yes-i-understand, the command prints the affected keys grouped by
reason and refuses to proceed. With --yes-i-understand the same
grouped banner prints before the writes hit the device, so even in
the CI case the operator has accountability output.
Export gains the symmetric flag set: --include-skip is new; --all is
new; --include-readonly/--include-hidden already existed. No
confirmation gate on export — it's read-only.
Closes #8
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
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.
Closes #8. The full design rationale lives in issue #8's pinned design comment; this PR description focuses on what landed.
Default behaviour change
mythy importnow refuses to write keys flagged by either of two locale-independent catalog markers:SKIP="YES"— declared on identification / IP / comm config (in NV10P-EB0-u:SerialNumber,ethernet_hw_address,IP_address,IP_netMsk,IP_gateway,MB_address,MB_485_baudrate,MB_232_baudrate,SelProto,NTPSynch). Writing these over the live Modbus connection drops the connection mid-transaction.VISIBILITY="3"— the Administrator / Menu Riservato subtree. Cascade from the containing<GROUP>is honoured, plus DATA-level overrides likeCodeNumthat sit in an otherwise-visible group.READONLY="YES"is still a hard mythy-side refuse (the device empirically accepts writes to fields the catalog labels read-only, so this defensive guard remains load-bearing).Path-based per-category opt-ins (
--include-ntp,--include-oscillography, etc.) from the original issue spec are dropped — top-level menu names are locale-dependent and the catalog markers give a strictly better classification.Opt-in flags (both
importandexport)--include-skipSKIP="YES"--include-hiddenVISIBILITY="3"cascade and DATA-level--include-readonly--all--include-skip+--include-hidden; export = those two plus--include-readonlyImport-only:
--yes-i-understandis required when any--include-*is set. Without it, the command prints the banner of keys that would be touched (grouped by reason) and exits with an error. Export has no confirmation gate (non-destructive).Live verification (NV10P-EX0-u via proxy)
Hand-crafted YAML with one
SKIP=YESkey (MB_address) and one DATA-level-hidden key (CodeNum):Tests
pkg/configio/apply_test.go— new tests cover: SKIP key classified-and-skipped by default; SKIP key written whenIncludeSkip=true(and still surfaced inInSkipCategoryfor the CLI banner); hidden key classified via parent-group cascade; hidden key written whenIncludeHidden=true; single dry-run pass surfaces all four buckets.pkg/session/filter_test.go—TestFilterSkipNeverOverridableupdated to the new semantics (it's now overridable viaIncludeSkip), default-exclude behaviour preserved.testdata/us/TEST-VB0-agains two minimal DATA:TEST_IP_Address(SKIP=YES) andTEST_EnableBoard(inside a VISIBILITY=3 group). Picked names that mirror the real-world failure pattern.Scope notes
🤖 Generated with Claude Code