Skip to content

Commit e5fe20e

Browse files
brianmcgeebrianmcgee
authored
Merge pull request #513 from numtide/feat/validate-formatter-name
Enforce formatter name doesn't contain bad characters
2 parents 5aefff9 + 43f943f commit e5fe20e

File tree

2 files changed

+92
-39
lines changed

2 files changed

+92
-39
lines changed

‎cmd/root_test.go‎

Lines changed: 71 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ func TestOnUnmatched(t *testing.T) {
7979

8080
// should exit with error when using fatal
8181
t.Run("fatal", func(t *testing.T) {
82-
errorFn := func(err error) {
82+
errorFn := func(as *require.Assertions, err error) {
8383
as.ErrorContains(err, "no formatter for path: "+expectedPaths[0])
8484
}
8585

@@ -114,8 +114,8 @@ func TestOnUnmatched(t *testing.T) {
114114

115115
t.Run("invalid", func(t *testing.T) {
116116
// test bad value
117-
errorFn := func(arg string) func(err error) {
118-
return func(err error) {
117+
errorFn := func(arg string) func(as *require.Assertions, err error) {
118+
return func(as *require.Assertions, err error) {
119119
as.ErrorContains(err, fmt.Sprintf(`invalid level: "%s"`, arg))
120120
}
121121
}
@@ -153,7 +153,7 @@ func TestQuiet(t *testing.T) {
153153
t.Setenv("TREEFMT_ALLOW_MISSING_FORMATTER", "false")
154154

155155
// check it doesn't suppress errors
156-
treefmt(t, withError(func(err error) {
156+
treefmt(t, withError(func(as *require.Assertions, err error) {
157157
as.ErrorContains(err, "error looking up 'foo-fmt'")
158158
}))
159159
}
@@ -183,8 +183,6 @@ func TestCpuProfile(t *testing.T) {
183183
}
184184

185185
func TestAllowMissingFormatter(t *testing.T) {
186-
as := require.New(t)
187-
188186
tempDir := test.TempExamples(t)
189187
configPath := filepath.Join(tempDir, "treefmt.toml")
190188

@@ -200,7 +198,7 @@ func TestAllowMissingFormatter(t *testing.T) {
200198

201199
t.Run("default", func(t *testing.T) {
202200
treefmt(t,
203-
withError(func(err error) {
201+
withError(func(as *require.Assertions, err error) {
204202
as.ErrorIs(err, format.ErrCommandNotFound)
205203
}),
206204
)
@@ -226,8 +224,6 @@ func TestAllowMissingFormatter(t *testing.T) {
226224
}
227225

228226
func TestSpecifyingFormatters(t *testing.T) {
229-
as := require.New(t)
230-
231227
// we use the test formatter to append some whitespace
232228
cfg := &config.Config{
233229
FormatterConfigs: map[string]*config.Formatter{
@@ -308,8 +304,8 @@ func TestSpecifyingFormatters(t *testing.T) {
308304
// bad name
309305
treefmt(t,
310306
withArgs("--formatters", "foo"),
311-
withError(func(err error) {
312-
as.Errorf(err, "formatter not found in config: foo")
307+
withError(func(as *require.Assertions, err error) {
308+
as.ErrorContains(err, "formatter foo not found in config")
313309
}),
314310
)
315311
})
@@ -331,11 +327,49 @@ func TestSpecifyingFormatters(t *testing.T) {
331327
t.Setenv("TREEFMT_FORMATTERS", "bar,foo")
332328

333329
treefmt(t,
334-
withError(func(err error) {
335-
as.Errorf(err, "formatter not found in config: bar")
330+
withError(func(as *require.Assertions, err error) {
331+
as.ErrorContains(err, "formatter bar not found in config")
336332
}),
337333
)
338334
})
335+
336+
t.Run("bad names", func(t *testing.T) {
337+
for _, name := range []string{"foo$", "/bar", "baz%"} {
338+
treefmt(t,
339+
withArgs("--formatters", name),
340+
withError(func(as *require.Assertions, err error) {
341+
as.ErrorContains(err, fmt.Sprintf("formatter name %q is invalid", name))
342+
}),
343+
)
344+
345+
t.Setenv("TREEFMT_FORMATTERS", name)
346+
347+
treefmt(t,
348+
withError(func(as *require.Assertions, err error) {
349+
as.ErrorContains(err, fmt.Sprintf("formatter name %q is invalid", name))
350+
}),
351+
)
352+
353+
t.Setenv("TREEFMT_FORMATTERS", "")
354+
355+
cfg.FormatterConfigs[name] = &config.Formatter{
356+
Command: "echo",
357+
Includes: []string{"*"},
358+
}
359+
360+
test.WriteConfig(t, configPath, cfg)
361+
362+
treefmt(t,
363+
withError(func(as *require.Assertions, err error) {
364+
as.ErrorContains(err, fmt.Sprintf("formatter name %q is invalid", name))
365+
}),
366+
)
367+
368+
delete(cfg.FormatterConfigs, name)
369+
370+
test.WriteConfig(t, configPath, cfg)
371+
}
372+
})
339373
}
340374

341375
func TestIncludesAndExcludes(t *testing.T) {
@@ -541,7 +575,7 @@ func TestConfigFile(t *testing.T) {
541575
withEnv(map[string]string{
542576
"PRJ_ROOT": configSubDir,
543577
}),
544-
withError(func(err error) {
578+
withError(func(as *require.Assertions, err error) {
545579
as.ErrorContains(err, "failed to find treefmt config file")
546580
}),
547581
)
@@ -550,8 +584,6 @@ func TestConfigFile(t *testing.T) {
550584
}
551585

552586
func TestCache(t *testing.T) {
553-
as := require.New(t)
554-
555587
tempDir := test.TempExamples(t)
556588
configPath := filepath.Join(tempDir, "treefmt.toml")
557589

@@ -657,7 +689,7 @@ func TestCache(t *testing.T) {
657689
// running should match but not format anything
658690

659691
treefmt(t,
660-
withError(func(err error) {
692+
withError(func(as *require.Assertions, err error) {
661693
as.ErrorIs(err, format.ErrFormattingFailures)
662694
}),
663695
withStats(t, map[stats.Type]int{
@@ -670,7 +702,7 @@ func TestCache(t *testing.T) {
670702

671703
// running again should provide the same result
672704
treefmt(t,
673-
withError(func(err error) {
705+
withError(func(as *require.Assertions, err error) {
674706
as.ErrorIs(err, format.ErrFormattingFailures)
675707
}),
676708
withStats(t, map[stats.Type]int{
@@ -733,7 +765,7 @@ func TestChangeWorkingDirectory(t *testing.T) {
733765

734766
treefmt(t,
735767
withConfig(configPath, cfg),
736-
withError(func(err error) {
768+
withError(func(as *require.Assertions, err error) {
737769
as.ErrorContains(err, "failed to find treefmt config file")
738770
}),
739771
)
@@ -804,8 +836,6 @@ func TestChangeWorkingDirectory(t *testing.T) {
804836
}
805837

806838
func TestFailOnChange(t *testing.T) {
807-
as := require.New(t)
808-
809839
t.Run("change size", func(t *testing.T) {
810840
tempDir := test.TempExamples(t)
811841
configPath := filepath.Join(tempDir, "treefmt.toml")
@@ -829,7 +859,7 @@ func TestFailOnChange(t *testing.T) {
829859
treefmt(t,
830860
withArgs("--fail-on-change"),
831861
withConfig(configPath, cfg),
832-
withError(func(err error) {
862+
withError(func(as *require.Assertions, err error) {
833863
as.ErrorIs(err, formatCmd.ErrFailOnChange)
834864
}),
835865
withStats(t, map[stats.Type]int{
@@ -890,7 +920,7 @@ func TestFailOnChange(t *testing.T) {
890920
},
891921
}
892922
}),
893-
withError(func(err error) {
923+
withError(func(as *require.Assertions, err error) {
894924
as.ErrorIs(err, formatCmd.ErrFailOnChange)
895925
}),
896926
withStats(t, map[stats.Type]int{
@@ -1428,7 +1458,7 @@ func TestGit(t *testing.T) {
14281458
treefmt(t,
14291459
withArgs("-C", tempDir, "haskell", "foo"),
14301460
withConfig(configPath, cfg),
1431-
withError(func(err error) {
1461+
withError(func(as *require.Assertions, err error) {
14321462
as.ErrorContains(err, "path foo not found")
14331463
}),
14341464
)
@@ -1555,8 +1585,8 @@ func TestPathsArg(t *testing.T) {
15551585
// specify a bad path
15561586
treefmt(t,
15571587
withArgs("elm/elm.json", "haskell/Nested/Bar.hs"),
1558-
withError(func(err error) {
1559-
as.Errorf(err, "path haskell/Nested/Bar.hs not found")
1588+
withError(func(as *require.Assertions, err error) {
1589+
as.ErrorContains(err, "path haskell/Nested/Bar.hs not found")
15601590
}),
15611591
)
15621592

@@ -1567,8 +1597,8 @@ func TestPathsArg(t *testing.T) {
15671597

15681598
treefmt(t,
15691599
withArgs(absoluteExternalPath),
1570-
withError(func(err error) {
1571-
as.Errorf(err, "path %s not found within the tree root", absoluteExternalPath)
1600+
withError(func(as *require.Assertions, err error) {
1601+
as.ErrorContains(err, fmt.Sprintf("path %s not inside the tree root", absoluteExternalPath))
15721602
}),
15731603
)
15741604

@@ -1578,8 +1608,8 @@ func TestPathsArg(t *testing.T) {
15781608

15791609
treefmt(t,
15801610
withArgs(relativeExternalPath),
1581-
withError(func(err error) {
1582-
as.Errorf(err, "path %s not found within the tree root", relativeExternalPath)
1611+
withError(func(as *require.Assertions, err error) {
1612+
as.ErrorContains(err, fmt.Sprintf("path %s not inside the tree root", relativeExternalPath))
15831613
}),
15841614
)
15851615
}
@@ -1607,7 +1637,7 @@ func TestStdin(t *testing.T) {
16071637
// we get an error about the missing filename parameter.
16081638
treefmt(t,
16091639
withArgs("--stdin"),
1610-
withError(func(err error) {
1640+
withError(func(as *require.Assertions, err error) {
16111641
as.EqualError(err, "exactly one path should be specified when using the --stdin flag")
16121642
}),
16131643
withStderr(func(out []byte) {
@@ -1640,8 +1670,8 @@ func TestStdin(t *testing.T) {
16401670

16411671
treefmt(t,
16421672
withArgs("--stdin", "../test.nix"),
1643-
withError(func(err error) {
1644-
as.Errorf(err, "path ../test.nix not inside the tree root %s", tempDir)
1673+
withError(func(as *require.Assertions, err error) {
1674+
as.ErrorContains(err, "path ../test.nix not inside the tree root "+tempDir)
16451675
}),
16461676
withStderr(func(out []byte) {
16471677
as.Contains(string(out), "Error: path ../test.nix not inside the tree root")
@@ -1804,7 +1834,7 @@ func TestRunInSubdir(t *testing.T) {
18041834
// this should not work, as we're in a subdirectory
18051835
treefmt(t,
18061836
withArgs("-c", "elm/elm.json", "haskell/Nested/Foo.hs"),
1807-
withError(func(err error) {
1837+
withError(func(as *require.Assertions, err error) {
18081838
as.ErrorContains(err, "path elm/elm.json not found")
18091839
}),
18101840
)
@@ -1836,7 +1866,7 @@ type options struct {
18361866
assertStdout func([]byte)
18371867
assertStderr func([]byte)
18381868

1839-
assertError func(error)
1869+
assertError func(*require.Assertions, error)
18401870
assertStats func(*stats.Stats)
18411871

18421872
bump struct {
@@ -1886,7 +1916,7 @@ func withStats(t *testing.T, expected map[stats.Type]int) option {
18861916
}
18871917
}
18881918

1889-
func withError(fn func(error)) option {
1919+
func withError(fn func(*require.Assertions, error)) option {
18901920
return func(o *options) {
18911921
o.assertError = fn
18921922
}
@@ -1896,8 +1926,8 @@ func withNoError(t *testing.T) option {
18961926
t.Helper()
18971927

18981928
return func(o *options) {
1899-
o.assertError = func(err error) {
1900-
require.NoError(t, err)
1929+
o.assertError = func(as *require.Assertions, err error) {
1930+
as.NoError(err)
19011931
}
19021932
}
19031933
}
@@ -1928,6 +1958,8 @@ func treefmt(
19281958
) {
19291959
t.Helper()
19301960

1961+
as := require.New(t)
1962+
19311963
// build options
19321964
opts := &options{}
19331965
for _, option := range opt {
@@ -2037,6 +2069,6 @@ func treefmt(
20372069
}
20382070

20392071
if opts.assertError != nil {
2040-
opts.assertError(cmdErr)
2072+
opts.assertError(as, cmdErr)
20412073
}
20422074
}

‎config/config.go‎

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

910
"github.com/numtide/treefmt/v2/walk"
@@ -210,12 +211,32 @@ func FromViper(v *viper.Viper) (*Config, error) {
210211
cfg.Excludes = cfg.Global.Excludes
211212
}
212213

214+
// validate formatter names do not contain invalid characters
215+
216+
nameRegex := regexp.MustCompile("^[a-zA-Z0-9_-]+$")
217+
218+
for name := range cfg.FormatterConfigs {
219+
if !nameRegex.MatchString(name) {
220+
return nil, fmt.Errorf(
221+
"formatter name %q is invalid, must be of the form %s",
222+
name, nameRegex.String(),
223+
)
224+
}
225+
}
226+
213227
// filter formatters based on provided names
214228
if len(cfg.Formatters) > 0 {
215229
filtered := make(map[string]*Formatter)
216230

217231
// check if the provided names exist in the config
218232
for _, name := range cfg.Formatters {
233+
if !nameRegex.MatchString(name) {
234+
return nil, fmt.Errorf(
235+
"formatter name %q is invalid, must be of the form %s",
236+
name, nameRegex.String(),
237+
)
238+
}
239+
219240
formatterCfg, ok := cfg.FormatterConfigs[name]
220241
if !ok {
221242
return nil, fmt.Errorf("formatter %v not found in config", name)

0 commit comments

Comments
 (0)