Skip to content

Commit ed9cbef

Browse files
bobakemamianclaude
andauthored
fix(drawer): implicit webhook input + 'any' type for trigger drawers (#171)
* fix(drawer): implicit webhook input + 'any' type for trigger drawers Before: a drawer with a webhook trigger couldn't pass `buttons drawer NAME --summary` validation because refs like `${inputs.webhook.body.resource.defaultDatasetId}` hit "unknown drawer input 'webhook'" — the input is populated by the listener dispatcher, not declared in drawer.json. Now: when a drawer has kind=webhook in Triggers, the validator auto-declares an implicit InputDef{Name:"webhook", Type:"any"}. The "any" type compatibly matches string/int/bool at the arg-type check (fall-through to press-time best-effort resolution) so downstream steps reading webhook fields pass validation regardless of arg type. This was caught during the real-world Apify webhook setup — the on-apify-done drawer needed `${inputs.webhook.body.resource.defaultDatasetId}` to flow into an `apify-fetch-dataset` button that declares a string arg, and the validator blocked the press every time. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(init): comprehensive .gitignore + auto-add on batteries set Two linked fixes so secret-bearing files in .buttons/ can't accidentally ship in a commit: 1) `buttons init` now writes a .gitignore that covers everything in .buttons/ that can hold secrets or per-machine state: - batteries.json (API keys, 0600 on disk but 'git add -A' ignores perms) - webhook.json (tunnel hostname + id; machine-specific) - buttons/*/pressed/ + drawers/*/pressed/ (run history with press-time args) - idempotency/ (cached results keyed on args) - queues/ (file-lock state) 2) Older projects that already ran `buttons init` before this fix shipped get upgraded in place. The init path now reads an existing .gitignore and appends any missing secret patterns under an "Added by buttons upgrade" header, idempotent and append-only so user customisations are preserved. 3) `buttons batteries set --local` (or local-default in a project) now runs the same ensure-pattern check on .buttons/.gitignore before returning success, so a battery set in a project that somehow missed the init .gitignore still gets covered. Emits a "added batteries.json to .buttons/.gitignore" notice in stderr and the JSON payload so the user knows we touched their repo. Global scope (~/.buttons/) skips the check since it's outside any repo. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(gosec): annotate .gitignore writes as not-path-traversable gosec G703 flagged os.WriteFile calls on paths ending in .buttons/.gitignore because the path traces back to config.DataDir() which reads BUTTONS_HOME. The filename is a literal and the parent directory is constrained to BUTTONS_HOME / discovered .buttons/ / ~/.buttons — no user-tainted path-traversal vector. #nosec with a rationale comment at each site. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 38f718c commit ed9cbef

3 files changed

Lines changed: 185 additions & 6 deletions

File tree

cmd/batteries.go

Lines changed: 73 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"errors"
55
"fmt"
66
"os"
7+
"path/filepath"
78

89
"github.com/spf13/cobra"
910

@@ -68,18 +69,88 @@ var batteriesSetCmd = &cobra.Command{
6869
return handleBatteryError(err)
6970
}
7071

72+
// Creation-time git-leak check: if the file we just wrote
73+
// is project-local (under .buttons/), verify git treats it as
74+
// ignored. If not, auto-add the pattern to .buttons/.gitignore
75+
// and surface a notice so the user knows we touched their
76+
// repo on their behalf. For --global scope the file is under
77+
// ~/.buttons/ which is outside any repo — skip the check.
78+
gitignoreNotice := ""
79+
if scope == battery.ScopeLocal {
80+
notice, gerr := ensureBatteriesGitignored()
81+
if gerr != nil {
82+
// Not fatal — the battery was saved. Warn so the user
83+
// can check manually.
84+
fmt.Fprintf(os.Stderr, "warning: could not verify .buttons/batteries.json is gitignored: %v\n", gerr)
85+
}
86+
gitignoreNotice = notice
87+
}
88+
7189
if jsonOutput {
72-
return config.WriteJSON(map[string]any{
90+
out := map[string]any{
7391
"key": key,
7492
"scope": string(scope),
75-
})
93+
}
94+
if gitignoreNotice != "" {
95+
out["gitignore"] = gitignoreNotice
96+
}
97+
return config.WriteJSON(out)
7698
}
7799
fmt.Fprintf(os.Stderr, "set %s (%s)\n", key, scope)
100+
if gitignoreNotice != "" {
101+
fmt.Fprintf(os.Stderr, " %s\n", gitignoreNotice)
102+
}
78103
printNextHint("use $BUTTONS_BAT_%s in any shell/code button", key)
79104
return nil
80105
},
81106
}
82107

108+
// ensureBatteriesGitignored verifies that the project-local
109+
// .buttons/batteries.json path is covered by a .gitignore rule. If
110+
// not, appends `batteries.json` to .buttons/.gitignore (creating the
111+
// file if it's absent) and returns a human-readable notice. Returns
112+
// "" when the pattern was already present.
113+
//
114+
// We don't shell out to git — some users run `buttons batteries set`
115+
// outside a repo, or before `git init`. A pure .gitignore read is
116+
// enough since our gitignore template has always put batteries.json
117+
// at the project-root-relative location.
118+
func ensureBatteriesGitignored() (string, error) {
119+
base, err := config.DataDir()
120+
if err != nil {
121+
return "", err
122+
}
123+
// Only act when the data dir is a project-local .buttons/. The
124+
// global path (~/.buttons/) isn't inside a repo so git concerns
125+
// don't apply.
126+
if !config.IsProjectLocal() {
127+
return "", nil
128+
}
129+
gitignorePath := filepath.Join(base, ".gitignore")
130+
data, rerr := os.ReadFile(gitignorePath) // #nosec G304 -- fixed .gitignore path
131+
if rerr != nil && !os.IsNotExist(rerr) {
132+
return "", rerr
133+
}
134+
body := string(data)
135+
if gitignoreContainsPattern(body, "batteries.json") {
136+
return "", nil
137+
}
138+
if len(body) > 0 && body[len(body)-1] != '\n' {
139+
body += "\n"
140+
}
141+
if body == "" {
142+
body = "# Buttons-managed patterns — never commit secret-bearing files.\n"
143+
} else {
144+
body += "\n# Added by `buttons batteries set` — never commit.\n"
145+
}
146+
body += "batteries.json\n"
147+
// #nosec G304 G306 G703 -- gitignorePath is filepath.Join(config.DataDir(), ".gitignore"). The filename is a literal and DataDir is constrained to BUTTONS_HOME, a discovered .buttons/ parent, or ~/.buttons. No user-tainted segment.
148+
if err := os.WriteFile(gitignorePath, []byte(body), 0600); err != nil {
149+
return "", err
150+
}
151+
return "added batteries.json to .buttons/.gitignore", nil
152+
}
153+
83154
var batteriesGetCmd = &cobra.Command{
84155
Use: "get KEY",
85156
Short: "Print a battery value",

cmd/init.go

Lines changed: 92 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"fmt"
55
"os"
66
"path/filepath"
7+
"strings"
78

89
"github.com/charmbracelet/huh"
910
"github.com/spf13/cobra"
@@ -120,16 +121,103 @@ func ensureButtonsDir(buttonsDir string) error {
120121

121122
gitignorePath := filepath.Join(buttonsDir, ".gitignore")
122123
if !fileExists(gitignorePath) {
123-
gitignore := "# Button specs, code files, and AGENT.md are committed.\n" +
124-
"# Run history is per-machine and excluded.\n" +
125-
"buttons/*/pressed/\n"
126-
if err := os.WriteFile(gitignorePath, []byte(gitignore), 0600); err != nil {
124+
if err := os.WriteFile(gitignorePath, []byte(defaultButtonsGitignore()), 0600); err != nil {
127125
return fmt.Errorf("failed to write .gitignore: %w", err)
128126
}
127+
} else {
128+
// Existing .gitignore — ensure the secret-bearing patterns are
129+
// present. Idempotent: only appends missing ones so a user who
130+
// customised their .gitignore keeps their additions.
131+
if err := ensureSecretPatterns(gitignorePath); err != nil {
132+
return fmt.Errorf("failed to update .gitignore: %w", err)
133+
}
129134
}
130135
return nil
131136
}
132137

138+
// defaultButtonsGitignore is the template written on init. Lists
139+
// every path in .buttons/ that can hold secrets or per-machine state
140+
// and must never be committed.
141+
//
142+
// batteries.json — holds plaintext API keys. 0600 on disk, but that
143+
// doesn't stop an accidental `git add -A`.
144+
// webhook.json — Cloudflare tunnel config (hostname + tunnel id).
145+
// Not secret per se, but machine-specific.
146+
// buttons/*/pressed/ — run history including stdin args; can leak
147+
// sensitive values passed at press time.
148+
// drawers/*/pressed/ — same, for drawer runs.
149+
// idempotency/ — cached successful results keyed on args. Can
150+
// contain secret-derived data.
151+
// queues/ — file-lock semaphore state; machine-local.
152+
func defaultButtonsGitignore() string {
153+
return `# Files that hold secrets — never commit.
154+
batteries.json
155+
# Machine-specific tunnel / listener config.
156+
webhook.json
157+
# Run history (per-machine, may contain sensitive args).
158+
buttons/*/pressed/
159+
drawers/*/pressed/
160+
# Local execution state.
161+
idempotency/
162+
queues/
163+
`
164+
}
165+
166+
// secretPatterns is the set of .gitignore lines we consider
167+
// load-bearing for secret hygiene. ensureSecretPatterns appends any
168+
// missing ones to an existing .buttons/.gitignore so an older project
169+
// upgrading past the init that introduced the pattern gets the
170+
// coverage retroactively.
171+
var secretPatterns = []string{
172+
"batteries.json",
173+
"webhook.json",
174+
}
175+
176+
func ensureSecretPatterns(path string) error {
177+
data, err := os.ReadFile(path) // #nosec G304 -- path is .buttons/.gitignore
178+
if err != nil {
179+
return err
180+
}
181+
existing := string(data)
182+
var toAppend []string
183+
for _, pat := range secretPatterns {
184+
if !gitignoreContainsPattern(existing, pat) {
185+
toAppend = append(toAppend, pat)
186+
}
187+
}
188+
if len(toAppend) == 0 {
189+
return nil
190+
}
191+
// Preserve the user's trailing newline, then append our additions
192+
// with a header so it's clear they came from buttons upgrade.
193+
if len(existing) > 0 && existing[len(existing)-1] != '\n' {
194+
existing += "\n"
195+
}
196+
existing += "\n# Added by buttons upgrade — never commit these.\n"
197+
for _, p := range toAppend {
198+
existing += p + "\n"
199+
}
200+
// #nosec G304 G306 G703 -- path is the caller's .buttons/.gitignore; filename is a literal, parent is our project-local data dir. Not user-tainted in any path-traversal sense.
201+
return os.WriteFile(path, []byte(existing), 0600)
202+
}
203+
204+
// gitignoreContainsPattern scans a .gitignore body for a literal line
205+
// matching `pattern`. Doesn't handle globs / negations — we only use
206+
// it for our own fixed patterns so a simple line-equality check is
207+
// fine and avoids pulling a .gitignore parser in.
208+
func gitignoreContainsPattern(body, pattern string) bool {
209+
for _, line := range strings.Split(body, "\n") {
210+
line = strings.TrimSpace(line)
211+
if line == "" || strings.HasPrefix(line, "#") {
212+
continue
213+
}
214+
if line == pattern {
215+
return true
216+
}
217+
}
218+
return false
219+
}
220+
133221
// resolveAgentTargets picks the agent skill files to install based on
134222
// (in priority order):
135223
//

internal/drawer/validate.go

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,18 @@ func Validate(d *Drawer, btnSvc *button.Service) ValidationReport {
6262
for _, in := range d.Inputs {
6363
inputNames[in.Name] = in
6464
}
65+
// When a drawer has a webhook trigger, the listener dispatches
66+
// with inputs.webhook populated from the incoming POST (body,
67+
// headers, query, etc.). Treat that as an implicit declared input
68+
// so `${inputs.webhook.body.foo}` refs validate cleanly — without
69+
// this, every webhook-triggered drawer reports a spurious
70+
// "unknown drawer input 'webhook'" error.
71+
for _, t := range d.Triggers {
72+
if t.Kind == "webhook" {
73+
inputNames["webhook"] = InputDef{Name: "webhook", Type: "any", Description: "Incoming webhook payload (implicit from kind=webhook trigger)"}
74+
break
75+
}
76+
}
6577

6678
// Cache button specs as we look them up.
6779
btnCache := map[string]*button.Button{}
@@ -453,6 +465,14 @@ func typesCompatible(src, dst string) bool {
453465
if src == "enum" && dst == "string" {
454466
return true
455467
}
468+
// "any" is used for the implicit `webhook` input on webhook-
469+
// triggered drawers — the payload's field types are shaped by
470+
// the sending service (Apify, Stripe, etc.) and aren't worth
471+
// statically re-encoding in drawer.json. Fall through to a
472+
// best-effort match at press time.
473+
if src == "any" {
474+
return true
475+
}
456476
return false
457477
}
458478

0 commit comments

Comments
 (0)