Skip to content

Commit eac8814

Browse files
ale-rinaldiclaude
andcommitted
fix(import,export): scope-aware writes, refuse dangerous categories by 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>
1 parent 9f2f833 commit eac8814

10 files changed

Lines changed: 449 additions & 48 deletions

File tree

README.md

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -238,8 +238,18 @@ mythy import --host 192.0.2.10 sample.yaml
238238
# Different product → pass --force to skip the product-mismatch check.
239239
```
240240
241-
Read-only, hidden, and module-disabled DATA are excluded by default;
242-
`--include-readonly` and `--include-hidden` widen the export.
241+
Read-only, hidden, `SKIP="YES"`, and module-disabled DATA are excluded
242+
by default. `--include-readonly`, `--include-hidden`, and
243+
`--include-skip` widen the export; `--all` is shorthand for all three.
244+
245+
`mythy import` mirrors the same flags. By default it refuses to write
246+
`SKIP="YES"` (identification, IP/comm config — writing these usually
247+
drops the live connection) or `VISIBILITY="3"` keys (the Administrator
248+
subtree — wrong values there can leave the device in a state
249+
requiring a factory reset). To opt in, set the matching `--include-*`
250+
flag AND `--yes-i-understand`; `--all` is the shortcut for both
251+
opt-ins together. Without `--yes-i-understand`, the command prints a
252+
banner of the keys that would be touched and aborts.
243253
244254
### Raw escape hatches
245255

cmd/mythy/export.go

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import (
1313
func newExportCmd(cf *catalogFlags) *cobra.Command {
1414
var conn connFlags
1515
var scope string
16-
var includeHidden, includeReadOnly, noProgress bool
16+
var includeHidden, includeReadOnly, includeSkip, includeAll, noProgress bool
1717

1818
cmd := &cobra.Command{
1919
Use: "export <file>",
@@ -32,11 +32,17 @@ func newExportCmd(cf *catalogFlags) *cobra.Command {
3232
progress = makeReadProgress(cmd.ErrOrStderr(), false)
3333
}
3434

35+
if includeAll {
36+
includeHidden = true
37+
includeReadOnly = true
38+
includeSkip = true
39+
}
3540
b, err := configio.Export(ctx, s, configio.ExportOptions{
3641
Scope: scope,
3742
Filter: session.ExportFilter{
3843
IncludeHidden: includeHidden,
3944
IncludeReadOnly: includeReadOnly,
45+
IncludeSkip: includeSkip,
4046
},
4147
Progress: progress,
4248
})
@@ -54,6 +60,8 @@ func newExportCmd(cf *catalogFlags) *cobra.Command {
5460
cmd.Flags().StringVar(&scope, "scope", "", "menu path to export (default: whole device)")
5561
cmd.Flags().BoolVar(&includeHidden, "include-hidden", false, "include VISIBILITY=3 (Administrator) groups")
5662
cmd.Flags().BoolVar(&includeReadOnly, "include-readonly", false, "include READONLY=YES entries")
63+
cmd.Flags().BoolVar(&includeSkip, "include-skip", false, "include SKIP=YES entries (identification, IP/comm config)")
64+
cmd.Flags().BoolVar(&includeAll, "all", false, "shorthand for --include-hidden --include-readonly --include-skip")
5765
cmd.Flags().BoolVar(&noProgress, "no-progress", false, "suppress the progress indicator (auto-suppressed when stderr isn't a TTY)")
5866
return cmd
5967
}

cmd/mythy/import.go

Lines changed: 93 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"context"
55
"errors"
66
"fmt"
7+
"io"
78
"os"
89

910
"github.com/gridsociety/mythy/pkg/configio"
@@ -13,6 +14,7 @@ import (
1314
func newImportCmd(cf *catalogFlags) *cobra.Command {
1415
var conn connFlags
1516
var dryRun, force, noProgress bool
17+
var includeSkip, includeHidden, includeAll, yesIUnderstand bool
1618
var format string
1719

1820
cmd := &cobra.Command{
@@ -48,35 +50,119 @@ func newImportCmd(cf *catalogFlags) *cobra.Command {
4850
if !noProgress {
4951
progress = makeReadProgress(cmd.ErrOrStderr(), false)
5052
}
51-
applyOpts := configio.ApplyOptions{Progress: progress}
53+
optIns := resolveScopeOptIns(includeSkip, includeHidden, includeAll)
54+
applyOpts := configio.ApplyOptions{
55+
Progress: progress,
56+
IncludeSkip: optIns.skip,
57+
IncludeHidden: optIns.hidden,
58+
}
5259

5360
out := cmd.OutOrStdout()
61+
62+
// Opt-in without --yes-i-understand → pre-flight dry-run to
63+
// surface exactly which keys would be touched in each opted-
64+
// in category, then refuse to proceed. The Diff cost is only
65+
// paid in this safety path; the no-opt-in default flow goes
66+
// straight to Apply with one Diff pass.
67+
if optIns.anyRequested() && !yesIUnderstand {
68+
report, err := configio.ApplyDryRun(ctx, s, parsed, applyOpts)
69+
if err != nil {
70+
return err
71+
}
72+
printOptInBanner(out, report, optIns, "would write (rerun with --yes-i-understand to proceed)")
73+
return errors.New("scope opt-in requires --yes-i-understand to proceed")
74+
}
75+
5476
if dryRun {
5577
report, err := configio.ApplyDryRun(ctx, s, parsed, applyOpts)
5678
if err != nil {
5779
return err
5880
}
59-
fmt.Fprintf(out, "would write %d key(s): %v\n", len(report.WouldApply), report.WouldApply)
60-
if len(report.Skipped) > 0 {
61-
fmt.Fprintf(out, "would skip %d READONLY key(s): %v\n", len(report.Skipped), report.Skipped)
81+
if optIns.anyRequested() {
82+
printOptInBanner(out, report, optIns, "would write")
6283
}
84+
printDryRun(out, report, optIns)
6385
return nil
6486
}
87+
6588
report, err := configio.Apply(ctx, s, parsed, applyOpts)
6689
if err != nil {
6790
return err
6891
}
69-
fmt.Fprintf(out, "wrote %d key(s): %v\n", len(report.Applied), report.Applied)
70-
if len(report.Skipped) > 0 {
71-
fmt.Fprintf(out, "skipped %d READONLY key(s) the file changed: %v\n", len(report.Skipped), report.Skipped)
92+
if optIns.anyRequested() {
93+
printOptInBanner(out, report, optIns, "WRITING")
7294
}
95+
fmt.Fprintf(out, "wrote %d key(s): %v\n", len(report.Applied), report.Applied)
96+
printSkipSummary(out, report, optIns)
7397
return nil
7498
},
7599
}
76100
conn.bind(cmd)
77101
cmd.Flags().BoolVar(&dryRun, "dry-run", false, "show what would change; perform no writes")
78102
cmd.Flags().BoolVar(&force, "force", false, "skip the product-mismatch check")
79103
cmd.Flags().BoolVar(&noProgress, "no-progress", false, "suppress the progress indicator (auto-suppressed when stderr isn't a TTY)")
104+
cmd.Flags().BoolVar(&includeSkip, "include-skip", false, "allow writes to SKIP=YES keys (identification, IP/comm config)")
105+
cmd.Flags().BoolVar(&includeHidden, "include-hidden", false, "allow writes to VISIBILITY=3 keys (Administrator subtree)")
106+
cmd.Flags().BoolVar(&includeAll, "all", false, "shorthand for --include-skip --include-hidden")
107+
cmd.Flags().BoolVar(&yesIUnderstand, "yes-i-understand", false, "confirm intent when an --include-* flag is set")
80108
cmd.Flags().StringVar(&format, "format", "", "human|json|yaml (default: from MYTHY_FORMAT)")
81109
return cmd
82110
}
111+
112+
type scopeOptIns struct {
113+
skip bool
114+
hidden bool
115+
}
116+
117+
func (s scopeOptIns) anyRequested() bool { return s.skip || s.hidden }
118+
119+
func resolveScopeOptIns(skip, hidden, all bool) scopeOptIns {
120+
if all {
121+
return scopeOptIns{skip: true, hidden: true}
122+
}
123+
return scopeOptIns{skip: skip, hidden: hidden}
124+
}
125+
126+
// printOptInBanner lists keys that fall under an opt-in flag, grouped
127+
// by reason. Called for both the abort path (no --yes-i-understand)
128+
// and the proceed path (with --yes-i-understand) so the operator
129+
// always sees the full list before writes hit the device.
130+
func printOptInBanner(out io.Writer, rep *configio.Report, optIns scopeOptIns, label string) {
131+
if optIns.skip && len(rep.InSkipCategory) > 0 {
132+
fmt.Fprintf(out, "%s %d connection-disruptive key(s) (SKIP=YES — identification, IP/comm config): %v\n",
133+
label, len(rep.InSkipCategory), rep.InSkipCategory)
134+
}
135+
if optIns.hidden && len(rep.InHiddenCategory) > 0 {
136+
fmt.Fprintf(out, "%s %d hidden key(s) (VISIBILITY=3 — Administrator subtree): %v\n",
137+
label, len(rep.InHiddenCategory), rep.InHiddenCategory)
138+
}
139+
}
140+
141+
func printDryRun(out io.Writer, rep *configio.Report, optIns scopeOptIns) {
142+
fmt.Fprintf(out, "would write %d key(s): %v\n", len(rep.WouldApply), rep.WouldApply)
143+
if len(rep.Skipped) > 0 {
144+
fmt.Fprintf(out, "would skip %d READONLY key(s): %v\n", len(rep.Skipped), rep.Skipped)
145+
}
146+
if !optIns.skip && len(rep.InSkipCategory) > 0 {
147+
fmt.Fprintf(out, "would skip %d SKIP key(s) (use --include-skip to write): %v\n",
148+
len(rep.InSkipCategory), rep.InSkipCategory)
149+
}
150+
if !optIns.hidden && len(rep.InHiddenCategory) > 0 {
151+
fmt.Fprintf(out, "would skip %d hidden key(s) (use --include-hidden to write): %v\n",
152+
len(rep.InHiddenCategory), rep.InHiddenCategory)
153+
}
154+
}
155+
156+
func printSkipSummary(out io.Writer, rep *configio.Report, optIns scopeOptIns) {
157+
if len(rep.Skipped) > 0 {
158+
fmt.Fprintf(out, "skipped %d READONLY key(s) the file changed: %v\n", len(rep.Skipped), rep.Skipped)
159+
}
160+
if !optIns.skip && len(rep.InSkipCategory) > 0 {
161+
fmt.Fprintf(out, "skipped %d SKIP key(s) (use --include-skip to include): %v\n",
162+
len(rep.InSkipCategory), rep.InSkipCategory)
163+
}
164+
if !optIns.hidden && len(rep.InHiddenCategory) > 0 {
165+
fmt.Fprintf(out, "skipped %d hidden key(s) (use --include-hidden to include): %v\n",
166+
len(rep.InHiddenCategory), rep.InHiddenCategory)
167+
}
168+
}

pkg/catalog/walk_test.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,8 @@ func TestWalk(t *testing.T) {
3737
"MB_address",
3838
"MB_baudrate",
3939
"NomeLinea",
40+
"TEST_EnableBoard",
41+
"TEST_IP_Address",
4042
"UL1",
4143
}
4244
if !equalSlices(got, want) {

0 commit comments

Comments
 (0)