From 4c553986fb3baac2b35755f2e3821f81b263e6b6 Mon Sep 17 00:00:00 2001 From: Ravi Pina Date: Tue, 16 Jun 2026 15:00:09 -0700 Subject: [PATCH] feat: emit stored secret on import, drop secrets keyword Import now always emits WLAN secrets so the config is ready to apply: the stored enc: value by default (apply decrypts it), or plaintext with the decrypt keyword. A clear-text cache value or wrong password masks as *secret*. The secrets keyword is gone. --- cmd/import_api_site.go | 49 +++++++++++++---------- cmd/import_api_site_test.go | 38 +++++++++--------- cmd/import_api_templates.go | 10 ++--- cmd/import_api_templates_test.go | 6 +-- cmd/import_roundtrip_test.go | 2 +- docs/configuration.md | 19 ++++----- internal/cmdutils/positional_args.go | 19 ++++----- internal/cmdutils/positional_args_test.go | 8 ++-- 8 files changed, 75 insertions(+), 76 deletions(-) diff --git a/cmd/import_api_site.go b/cmd/import_api_site.go index 249c5e8..c14941b 100644 --- a/cmd/import_api_site.go +++ b/cmd/import_api_site.go @@ -54,7 +54,7 @@ const ( // importAPISiteCmd represents the "import api site" command var importAPISiteCmd = &cobra.Command{ - Use: "site [config|inventory|all] [full|type ] [source ] [secrets|decrypt] [compare] [save] [file ]", + Use: "site [config|inventory|all] [full|type ] [source ] [decrypt] [compare] [save] [file ]", Short: "Import site configuration from API cache", Long: `Import site configuration from the API cache. @@ -90,7 +90,7 @@ With Custom Output File: Combined Options: wifimgr import api site US-SFO-LAB source mist-prod save file import/sfo.json - wifimgr import api site US-SFO-LAB type wlans secrets save + wifimgr import api site US-SFO-LAB type wlans decrypt save wifimgr import api site US-SFO-LAB | jq '.config' Arguments: @@ -107,9 +107,10 @@ Arguments: - ap Access point configurations only - switch Switch configurations only - gateway Gateway/firewall configurations only - secrets Optional. Include PSK/RADIUS secrets, masked as "*secret*" - decrypt Optional. Include secrets decrypted to plaintext (implies secrets); - needs the encryption password (WIFIMGR_PASSWORD or prompt) + decrypt Optional. Emit secrets (PSK, RADIUS) decrypted to plaintext; + needs the encryption password (WIFIMGR_PASSWORD or prompt). + Without it, the stored enc: value is emitted as-is (applyable), + or "*secret*" if the cache somehow holds it in the clear. compare Optional. Compare API state with existing import file (using jsondiff) save Optional. Write to import file (default: print to STDOUT) file Optional. Keyword followed by output filename (relative to config_dir or absolute) @@ -117,7 +118,7 @@ Arguments: What it Does: 1. Retrieves site configuration from the specified API cache 2. Optionally filters to specific scope (wlans, profiles, ap, switch, gateway) - 3. Omits secrets by default; 'secrets' masks them, 'decrypt' reveals plaintext + 3. Emits secrets as their stored enc: value by default; 'decrypt' reveals plaintext 4. Prints to STDOUT or saves to a single import file if 'save' specified 5. With 'compare': shows diff between API and existing local import file @@ -282,7 +283,7 @@ func runImportAPISite(cmd *cobra.Command, args []string) error { // Build the combined ImportFile envelope (site config + site-local WLAN // templates live side-by-side so the file self-describes). - reveal, err := resolveSecretReveal(parsed.IncludeSecrets, parsed.Decrypt) + reveal, err := resolveSecretReveal(parsed.Decrypt) if err != nil { return err } @@ -589,23 +590,23 @@ type templatesEnvelope struct { Device map[string]map[string]any `json:"device,omitempty"` } -// maskedSecret is the placeholder emitted for a secret the caller asked to -// include but not decrypt. +// maskedSecret stands in for a secret that can't be emitted safely — one the +// cache holds in the clear (so echoing it would leak plaintext) or one a decrypt +// failed on. const maskedSecret = "*secret*" -// secretReveal controls how WLAN secrets are emitted on import. include turns -// the secret fields on at all; decrypt swaps the masked placeholder for the -// plaintext, which needs the encryption password. +// secretReveal controls how WLAN secrets are emitted on import. By default the +// stored (encrypted) value is printed verbatim so the config applies as-is; +// decrypt swaps it for the plaintext, which needs the encryption password. type secretReveal struct { - include bool decrypt bool password string } // resolveSecretReveal builds the reveal policy from the parsed keywords, // prompting for the encryption password when decrypt was requested. -func resolveSecretReveal(include, decrypt bool) (secretReveal, error) { - r := secretReveal{include: include || decrypt, decrypt: decrypt} +func resolveSecretReveal(decrypt bool) (secretReveal, error) { + r := secretReveal{decrypt: decrypt} if decrypt { pw, err := encryption.GetPasswordOrPrompt("Enter encryption password to decrypt secrets: ") if err != nil { @@ -616,12 +617,16 @@ func resolveSecretReveal(include, decrypt bool) (secretReveal, error) { return r, nil } -// revealSecret renders a stored (encrypted) secret for output: the masked -// placeholder unless decrypt was requested, in which case it's decrypted with -// the supplied password. A decrypt failure falls back to the mask so a wrong +// revealSecret renders a stored secret for output. Default: emit the encrypted +// value as-is (so the imported config stays applyable) — but mask anything not +// actually encrypted so a clear-text secret never leaks. With decrypt: return +// the plaintext, falling back to the mask if decryption fails so a wrong // password neither leaks ciphertext nor aborts the export. func revealSecret(stored string, reveal secretReveal) string { if !reveal.decrypt { + if encryption.IsEncrypted(stored) { + return stored + } return maskedSecret } if !encryption.IsEncrypted(stored) { @@ -1232,8 +1237,8 @@ func merakiSlot(w *vendors.WLAN) (int, bool) { // (network IDs, ipAssignmentMode, etc.) is deliberately dropped here — this is // the portable half. The one binding worth keeping, the Meraki SSID slot, rides // alongside in a vendor block (see vendorBlockForWLAN), not in the profile. PSK -// and RADIUS secrets honor the reveal policy: omitted unless requested, masked -// as a placeholder by default, and decrypted only when decrypt was passed. +// and RADIUS secrets emit per revealSecret: the encrypted value by default +// (applyable as-is), or plaintext when decrypt was passed. func convertVendorWLANToProfile(w *vendors.WLAN, reveal secretReveal) *config.WLANProfile { profile := &config.WLANProfile{ SSID: w.SSID, @@ -1246,7 +1251,7 @@ func convertVendorWLANToProfile(w *vendors.WLAN, reveal secretReveal) *config.WL }, } - if reveal.include && w.PSK != "" { + if w.PSK != "" { profile.Auth.PSK = revealSecret(w.PSK, reveal) } @@ -1258,7 +1263,7 @@ func convertVendorWLANToProfile(w *vendors.WLAN, reveal secretReveal) *config.WL servers := make([]config.RADIUSServer, 0, len(w.RadiusServers)) for _, rs := range w.RadiusServers { cs := config.RADIUSServer{Host: rs.Host, Port: rs.Port} - if reveal.include && rs.Secret != "" { + if rs.Secret != "" { cs.Secret = revealSecret(rs.Secret, reveal) } servers = append(servers, cs) diff --git a/cmd/import_api_site_test.go b/cmd/import_api_site_test.go index de1d93a..9dd4bcf 100644 --- a/cmd/import_api_site_test.go +++ b/cmd/import_api_site_test.go @@ -195,7 +195,8 @@ func TestConvertVendorWLANToProfile_Meraki(t *testing.T) { "splashPage": "Click-through", }, } - got := convertVendorWLANToProfile(w, secretReveal{include: true, decrypt: true}) + // Seed PSK is plaintext, so decrypt echoes it back unchanged. + got := convertVendorWLANToProfile(w, secretReveal{decrypt: true}) if got.SSID != "Scale Guest" || !got.Enabled || got.VLANID != 10 || got.Band != "dual" { t.Errorf("basic fields wrong: %+v", got) @@ -213,15 +214,16 @@ func TestConvertVendorWLANToProfile_Meraki(t *testing.T) { t.Errorf("portal wrong: %+v", got.Portal) } - // With includeSecrets=false, PSK must be stripped. + // Default (no decrypt) on a plaintext-stored secret masks it, never leaks it. gotNoSec := convertVendorWLANToProfile(w, secretReveal{}) - if gotNoSec.Auth.PSK != "" { - t.Errorf("PSK leaked when includeSecrets=false: %q", gotNoSec.Auth.PSK) + if gotNoSec.Auth.PSK != maskedSecret { + t.Errorf("plaintext PSK should mask, got %q", gotNoSec.Auth.PSK) } } -// TestConvertVendorWLANToProfile_SecretReveal checks the three output states for -// a secret stored encrypted in the cache: omitted, masked, and decrypted. +// TestConvertVendorWLANToProfile_SecretReveal checks the output states for a +// secret stored encrypted in the cache: the enc: value by default, plaintext on +// decrypt, and the mask on a bad password. func TestConvertVendorWLANToProfile_SecretReveal(t *testing.T) { const password = "correct horse battery staple" encPSK, err := encryption.Encrypt("s3cr3t-pass", password) @@ -230,24 +232,19 @@ func TestConvertVendorWLANToProfile_SecretReveal(t *testing.T) { } w := &vendors.WLAN{SSID: "Scale", Enabled: true, AuthType: "psk", PSK: encPSK} - // Omitted: no secrets keyword. - if got := convertVendorWLANToProfile(w, secretReveal{}); got.Auth.PSK != "" { - t.Errorf("PSK should be omitted, got %q", got.Auth.PSK) - } - - // Masked: secrets without decrypt never exposes ciphertext or plaintext. - if got := convertVendorWLANToProfile(w, secretReveal{include: true}); got.Auth.PSK != maskedSecret { - t.Errorf("PSK = %q, want %q", got.Auth.PSK, maskedSecret) + // Default: emit the encrypted value verbatim so the config applies as-is. + if got := convertVendorWLANToProfile(w, secretReveal{}); got.Auth.PSK != encPSK { + t.Errorf("PSK = %q, want the stored enc: value", got.Auth.PSK) } // Decrypted: correct password yields the plaintext. - got := convertVendorWLANToProfile(w, secretReveal{include: true, decrypt: true, password: password}) + got := convertVendorWLANToProfile(w, secretReveal{decrypt: true, password: password}) if got.Auth.PSK != "s3cr3t-pass" { t.Errorf("decrypted PSK = %q, want %q", got.Auth.PSK, "s3cr3t-pass") } // Wrong password falls back to the mask rather than leaking ciphertext. - bad := convertVendorWLANToProfile(w, secretReveal{include: true, decrypt: true, password: "wrong"}) + bad := convertVendorWLANToProfile(w, secretReveal{decrypt: true, password: "wrong"}) if bad.Auth.PSK != maskedSecret { t.Errorf("bad-password PSK = %q, want %q", bad.Auth.PSK, maskedSecret) } @@ -302,7 +299,8 @@ func TestConvertVendorWLANToProfile_MistEnterprise(t *testing.T) { "portal": map[string]any{"enabled": true, "auth": "sponsor"}, }, } - got := convertVendorWLANToProfile(w, secretReveal{include: true, decrypt: true}) + // Seed RADIUS secret is plaintext, so decrypt echoes it back unchanged. + got := convertVendorWLANToProfile(w, secretReveal{decrypt: true}) if got.Auth.Type != "eap" { t.Errorf("expected Auth.Type=eap, got %q", got.Auth.Type) @@ -320,10 +318,10 @@ func TestConvertVendorWLANToProfile_MistEnterprise(t *testing.T) { t.Errorf("portal wrong: %+v", got.Portal) } - // includeSecrets=false strips RADIUS secret but keeps host/port. + // Default (no decrypt) on a plaintext-stored RADIUS secret masks it. gotNoSec := convertVendorWLANToProfile(w, secretReveal{}) - if gotNoSec.Auth.RADIUSServers[0].Secret != "" { - t.Errorf("RADIUS secret leaked: %q", gotNoSec.Auth.RADIUSServers[0].Secret) + if gotNoSec.Auth.RADIUSServers[0].Secret != maskedSecret { + t.Errorf("plaintext RADIUS secret should mask, got %q", gotNoSec.Auth.RADIUSServers[0].Secret) } if gotNoSec.Auth.RADIUSServers[0].Host != "radius1.example.com" { t.Errorf("host should survive secret strip: %q", gotNoSec.Auth.RADIUSServers[0].Host) diff --git a/cmd/import_api_templates.go b/cmd/import_api_templates.go index b2c9794..e20fce6 100644 --- a/cmd/import_api_templates.go +++ b/cmd/import_api_templates.go @@ -48,7 +48,7 @@ const ( // into a standalone ImportFile. Scoped to a specific API because template // namespaces live at the API/org boundary. var importAPITemplatesCmd = &cobra.Command{ - Use: "templates [target ] [type wlan|rf|device|gateway|all] [save] [file ] [secrets|decrypt]", + Use: "templates [target ] [type wlan|rf|device|gateway|all] [save] [file ] [decrypt]", Short: "Import vendor-level templates from API cache", Long: `Import vendor-level (org-scoped) templates from a specific API. @@ -70,9 +70,9 @@ Arguments: type Optional. Template kind to export (default: wlan). Currently only "wlan" is supported; "rf", "device", "gateway", "all" are reserved for later coverage. - secrets Optional. Include PSK/RADIUS secrets, masked as "*secret*". - decrypt Optional. Include secrets decrypted to plaintext (implies - secrets); needs WIFIMGR_PASSWORD or an interactive prompt. + decrypt Optional. Emit secrets decrypted to plaintext; needs + WIFIMGR_PASSWORD or an interactive prompt. Without it, the + stored enc: value is emitted as-is. save Optional. Write to import file (default: print to STDOUT). file Optional. Keyword followed by output filename (relative to config_dir or absolute). @@ -189,7 +189,7 @@ func runImportAPITemplates(cmd *cobra.Command, args []string) error { } // Build the envelope. For now only WLANs are supported. - reveal, err := resolveSecretReveal(parsed.IncludeSecrets, parsed.Decrypt) + reveal, err := resolveSecretReveal(parsed.Decrypt) if err != nil { return err } diff --git a/cmd/import_api_templates_test.go b/cmd/import_api_templates_test.go index bf578e3..f1825db 100644 --- a/cmd/import_api_templates_test.go +++ b/cmd/import_api_templates_test.go @@ -19,18 +19,18 @@ func TestParseImportTemplatesArgs_Defaults(t *testing.T) { if got.templateType != TemplateTypeWLAN { t.Errorf("templateType = %q, want default wlan", got.templateType) } - if got.SaveMode || got.IncludeSecrets || got.OutputFile != "" { + if got.SaveMode || got.Decrypt || got.OutputFile != "" { t.Errorf("unexpected non-default flags: %+v", got) } } func TestParseImportTemplatesArgs_Full(t *testing.T) { - got, err := parseImportTemplatesArgs([]string{"target", "mist-prod", "type", "wlan", "secrets", "save", "file", "import/org.json"}) + got, err := parseImportTemplatesArgs([]string{"target", "mist-prod", "type", "wlan", "decrypt", "save", "file", "import/org.json"}) if err != nil { t.Fatalf("unexpected error: %v", err) } if got.apiLabel != "mist-prod" || got.templateType != TemplateTypeWLAN || - !got.SaveMode || !got.IncludeSecrets || got.OutputFile != "import/org.json" { + !got.SaveMode || !got.Decrypt || got.OutputFile != "import/org.json" { t.Errorf("parsed args incorrect: %+v", got) } } diff --git a/cmd/import_roundtrip_test.go b/cmd/import_roundtrip_test.go index 3cd1412..5139187 100644 --- a/cmd/import_roundtrip_test.go +++ b/cmd/import_roundtrip_test.go @@ -94,7 +94,7 @@ func runRoundTripCase(t *testing.T, vendor, siteName string, seedWLANs []*vendor siteSlug := slug(siteName) // Step 1+2: export pipeline (no cache accessor needed for this slice). - labels, profiles, _ := synthesizeWLANLabels(seedWLANs, siteSlug, secretReveal{include: true, decrypt: true}) + labels, profiles, _ := synthesizeWLANLabels(seedWLANs, siteSlug, secretReveal{decrypt: true}) if len(labels) != len(seedWLANs) { t.Fatalf("label count = %d, want %d", len(labels), len(seedWLANs)) } diff --git a/docs/configuration.md b/docs/configuration.md index 89c0644..d676162 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -389,19 +389,20 @@ Because the password protects the cache, use the **same** password across refres secret encrypted with one password can't be decrypted with another. If you refresh with a different password, re-run `refresh` to re-encrypt under the intended one. -The `import api site` and `import api templates` commands read these encrypted secrets: +The `import api site` and `import api templates` commands emit these secrets so the +imported file is ready to apply: -| Keyword | Output for a stored secret | -|-------------|---------------------------------------------------------------| -| *(none)* | Secret field omitted | -| `secrets` | Field present, masked as `*secret*` | -| `decrypt` | Field present, decrypted to plaintext (implies `secrets`) | +| Keyword | Output for a stored secret | +|-----------|-----------------------------------------------------------------------------| +| *(none)* | The stored `enc:` value, verbatim — applies as-is (apply decrypts it) | +| `decrypt` | Decrypted plaintext, using the encryption password | -`decrypt` needs the encryption password (`WIFIMGR_PASSWORD` or prompt). A wrong password -falls back to the `*secret*` mask rather than emitting ciphertext. +If the cache somehow holds a secret in the clear, the default output masks it as +`*secret*` rather than echoing plaintext. `decrypt` needs the encryption password +(`WIFIMGR_PASSWORD` or prompt); a wrong password also falls back to the mask. ```bash -wifimgr import api site US-SFO-LAB type wlans secrets # PSK shown as *secret* +wifimgr import api site US-SFO-LAB type wlans # PSK kept as its enc: value WIFIMGR_PASSWORD=… wifimgr import api site US-SFO-LAB type wlans decrypt # plaintext PSK ``` diff --git a/internal/cmdutils/positional_args.go b/internal/cmdutils/positional_args.go index 6a821bc..eb7f92d 100644 --- a/internal/cmdutils/positional_args.go +++ b/internal/cmdutils/positional_args.go @@ -219,15 +219,15 @@ func ParseApplyArgs(args []string) (*ParsedApplyArgs, error) { } // ImportOutputArgs carries the emit-control keywords every `import api …` -// subcommand shares: secrets (include redacted-by-default fields), save (write -// to disk vs print), and file (output path). Each importer embeds this -// and runs Consume before its own grammar switch so the shared keywords behave -// identically across commands; the command-specific keywords stay local. +// subcommand shares: decrypt (reveal plaintext secrets vs the encrypted value), +// save (write to disk vs print), and file (output path). Each importer +// embeds this and runs Consume before its own grammar switch so the shared +// keywords behave identically across commands; the command-specific keywords +// stay local. type ImportOutputArgs struct { - IncludeSecrets bool - Decrypt bool // reveal plaintext secrets; implies IncludeSecrets - SaveMode bool - OutputFile string + Decrypt bool // emit decrypted plaintext secrets instead of the stored enc: value + SaveMode bool + OutputFile string } // Consume handles the shared import keyword at args[i]. It reports whether the @@ -236,11 +236,8 @@ type ImportOutputArgs struct { // through to its own keywords. func (o *ImportOutputArgs) Consume(args []string, i int) (matched bool, last int, err error) { switch strings.ToLower(args[i]) { - case "secrets": - o.IncludeSecrets = true case "decrypt": o.Decrypt = true - o.IncludeSecrets = true case "save": o.SaveMode = true case "file": diff --git a/internal/cmdutils/positional_args_test.go b/internal/cmdutils/positional_args_test.go index 6c637ea..d1b633e 100644 --- a/internal/cmdutils/positional_args_test.go +++ b/internal/cmdutils/positional_args_test.go @@ -112,14 +112,12 @@ func TestImportOutputArgsConsume(t *testing.T) { args []string wantMatch bool wantLast int - wantSecrets bool wantDecrypt bool wantSave bool wantFile string wantErr string }{ - {name: "secrets", args: []string{"secrets"}, wantMatch: true, wantLast: 0, wantSecrets: true}, - {name: "decrypt implies secrets", args: []string{"decrypt"}, wantMatch: true, wantLast: 0, wantSecrets: true, wantDecrypt: true}, + {name: "decrypt", args: []string{"decrypt"}, wantMatch: true, wantLast: 0, wantDecrypt: true}, {name: "save", args: []string{"save"}, wantMatch: true, wantLast: 0, wantSave: true}, {name: "file consumes operand", args: []string{"file", "x.json"}, wantMatch: true, wantLast: 1, wantFile: "x.json"}, {name: "file strips quotes", args: []string{"file", `"q.json"`}, wantMatch: true, wantLast: 1, wantFile: "q.json"}, @@ -142,8 +140,8 @@ func TestImportOutputArgsConsume(t *testing.T) { if matched != tc.wantMatch || last != tc.wantLast { t.Errorf("matched=%v last=%d, want matched=%v last=%d", matched, last, tc.wantMatch, tc.wantLast) } - if o.IncludeSecrets != tc.wantSecrets || o.Decrypt != tc.wantDecrypt || o.SaveMode != tc.wantSave || o.OutputFile != tc.wantFile { - t.Errorf("state = %+v, want secrets=%v decrypt=%v save=%v file=%q", o, tc.wantSecrets, tc.wantDecrypt, tc.wantSave, tc.wantFile) + if o.Decrypt != tc.wantDecrypt || o.SaveMode != tc.wantSave || o.OutputFile != tc.wantFile { + t.Errorf("state = %+v, want decrypt=%v save=%v file=%q", o, tc.wantDecrypt, tc.wantSave, tc.wantFile) } }) }