feat(wfctl): secrets setup --plugin reads plugin-declared required_secrets[]#737
Closed
intel352 wants to merge 1 commit into
Closed
feat(wfctl): secrets setup --plugin reads plugin-declared required_secrets[]#737intel352 wants to merge 1 commit into
intel352 wants to merge 1 commit into
Conversation
Implements T3+T4 of docs/plans/2026-05-20-dns-providers.md. `wfctl secrets setup --plugin workflow-plugin-hover --scope org --org GoCodeAlone --token-env GH_PAT` reads plugin.json from the configured plugin install dir, prompts for each required_secrets[] entry (masked iff sensitive), and writes the values to the chosen GitHub scope (repo|env|org) via the new scoped GH provider. cmd/wfctl/secrets_setup_plugin.go: - pluginManifest + PluginRequiredSecret types (json:"required_secrets"). - runSecretsSetupPlugin + runSecretsSetupPluginWithIO (testable core accepts injectable io.Reader for piped input). - loadPluginManifest: resolves plugin install dir from --plugin-dir / $WFCTL_PLUGIN_DIR / ./data/plugins; reads plugin.json; errors hint at `wfctl plugin install` when missing. - promptOne: piped path returns the line as-is; sensitive interactive path uses term.ReadPassword; non-sensitive interactive uses bare Fscanln. - buildSecretWriter: switches on scope (org|env|repo) and mints the matching GitHub provider. Fails loud on missing --org/--env. cmd/wfctl/secrets.go: dispatch updated — when `secrets setup` sees a --plugin arg, route to runSecretsSetupPlugin instead of the env-based runSecretsSetup. 7 new tests: - LoadPluginManifest happy + missing-dir (hint) + bad JSON. - promptOne piped non-sensitive + sensitive. - RunSecretsSetupPlugin: piped reader writes through; emits prelude. - BuildSecretWriter scope routing: org happy, missing --org/--env errors, unknown scope errors. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a new wfctl secrets setup --plugin <name> workflow to read a plugin’s plugin.json.required_secrets[], prompt for each secret (masked when interactive + sensitive=true), and write values to a chosen GitHub secrets scope.
Changes:
- Dispatch
wfctl secrets setupto a new--plugin-driven path. - Implement
secrets setup --pluginmanifest loading, prompting, and scoped GitHub secret writing. - Add unit tests covering manifest parsing, prompt behavior, and basic scope routing.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| cmd/wfctl/secrets.go | Routes secrets setup to the plugin-based setup flow when --plugin is present. |
| cmd/wfctl/secrets_setup_plugin.go | New implementation for reading required_secrets[], prompting, and writing to GitHub repo/env/org secrets. |
| cmd/wfctl/secrets_setup_plugin_test.go | New tests for manifest loading, prompting, and writer scope selection. |
Comments suppressed due to low confidence (2)
cmd/wfctl/secrets_setup_plugin.go:171
- Non-sensitive interactive input uses
fmt.Fscanln(os.Stdin, &line), which tokenizes on whitespace and will truncate values that contain spaces. This also differs fromrunSecretsSetupwhich reads full lines viabufio.Reader.ReadString('\n'). Prefer line-based reading here too (and keep the empty-line skip behavior).
// Non-sensitive interactive — echo.
var line string
if _, err := fmt.Fscanln(os.Stdin, &line); err != nil && err.Error() != "unexpected newline" {
return "", err
}
return line, nil
cmd/wfctl/secrets_setup_plugin_test.go:136
TestRunSecretsSetupPlugin_PiperReadsRequiredSecretsdoesn't assert that each required secret is read separately and written via the provider; it only checks that a non-nil error occurs and that the prelude was printed. This test won't catch regressions like consuming multiple lines at once or skipping later secrets. Consider injecting a fakescopedWriter(or a factory) so the test can assertSetcalls/values without hitting the network.
func TestRunSecretsSetupPlugin_PiperReadsRequiredSecrets(t *testing.T) {
dir := t.TempDir()
writePluginManifestFile(t, dir, "wp-fake", `{
"name": "wp-fake",
"required_secrets": [
{"name": "A", "sensitive": false},
{"name": "B", "sensitive": true}
]
}`)
// Stub the writer side by setting the org via env so
// buildSecretWriter is short-circuited (we just want to exercise
// the prompt loop). Use --scope=org with a stub provider not
// reachable in tests; the call will fail at network → we assert
// we got at least to the writer construction.
in := io.Reader(strings.NewReader("alice\nhunter2\n"))
var out bytes.Buffer
t.Setenv("GITHUB_TOKEN", "stub")
// We can't actually hit the GH API; use --scope=org pointing
// at a non-resolvable token+org, then assert error returns from
// the network-side path (after the prompts succeed).
err := runSecretsSetupPluginWithIO([]string{
"--plugin", "wp-fake",
"--plugin-dir", dir,
"--scope", "org",
"--org", "test-org",
"--token-env", "GITHUB_TOKEN",
}, in, &out)
if err == nil {
t.Fatal("expected network-side error reaching GH (test runs offline)")
}
// The output buffer should still show that we entered the setup
// loop (prompt prelude).
if !strings.Contains(out.String(), "Setting up secrets for plugin") {
t.Errorf("setup prelude missing from output:\n%s", out.String())
}
}
Comment on lines
+145
to
+149
| // Test/piped path — read one line. | ||
| buf := make([]byte, 4096) | ||
| n, err := in.Read(buf) | ||
| if err != nil && err != io.EOF { | ||
| return "", err |
Comment on lines
+112
to
+120
| dir := dirOverride | ||
| if dir == "" { | ||
| dir = os.Getenv("WFCTL_PLUGIN_DIR") | ||
| } | ||
| if dir == "" { | ||
| dir = "./data/plugins" | ||
| } | ||
| path := filepath.Join(dir, name, "plugin.json") | ||
| data, err := os.ReadFile(path) |
| vis, err := parseGitHubOrgVisibility(visibility) | ||
| if err != nil { | ||
| return nil, "", err | ||
| } |
| // dispatcher that reads plugin.json required_secrets[]. The | ||
| // env-name flow stays on runSecretsSetup. | ||
| for _, a := range args[1:] { | ||
| if a == "--plugin" || strings.HasPrefix(a, "--plugin=") { |
Comment on lines
+35
to
+37
| // `secrets setup --plugin <name>` shells out to a separate | ||
| // dispatcher that reads plugin.json required_secrets[]. The | ||
| // env-name flow stays on runSecretsSetup. |
Comment on lines
+95
to
+100
| // TestRunSecretsSetupPlugin_PiperReadsRequiredSecrets exercises the | ||
| // full flow with a piped reader for input. Output goes to a buffer. | ||
| // | ||
| // We swap out stdin via the io.Reader arg + verify the buffered out | ||
| // reports each secret as "set". | ||
| func TestRunSecretsSetupPlugin_PiperReadsRequiredSecrets(t *testing.T) { |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
⏱ Benchmark Results✅ No significant performance regressions detected. benchstat comparison (baseline → PR)
|
Contributor
Author
|
Superseded by the consolidated PR with lint fixes. |
intel352
added a commit
that referenced
this pull request
May 20, 2026
Supersedes #736 + #737 with lint fixes applied: dyndns/ (T14..T16): - IP-detect multi-source quorum + diff + Update callback + exp-backoff with jitter. 12 tests. cmd/wfctl/secrets_setup_plugin.go (T3+T4): - secrets setup --plugin reads plugin.json required_secrets[], prompts each (masked iff sensitive), writes to chosen GH scope (repo|env|org). 7 tests. Lint fixes vs the original PRs: - dyndns/dyndns.go Run() — replaced empty-branch (SA9003) with explicit `_ = d.Tick(...)` ignoring the error. - dyndns/dyndns.go jitter — math/rand/v2 G404 false-positive silenced with nolint:gosec annotation (decorrelation, not crypto). - dyndns/dyndns.go timeAfter — replaced unlambda wrapper with direct `var timeAfter = time.After`. - cmd/wfctl/secrets.go — hoisted `args[1:]` into a local var to silence gosec G602 (already bounded by outer switch). All tests pass; golangci-lint clean. 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.
Per workflow#735 SPEC T3+T4. Reads plugin.json required_secrets[], prompts each (masked iff sensitive), writes to chosen GH scope. 7 tests.