Hardening audit
Key hardening changes:
- Config validation now rejects paths.source: . and paths.assets: ., so source/asset roots cannot
resolve to the repository root.
- apply-hak-manifest now refuses to run when paths.source is unset or unsafe, instead of potentially
writing under a root-level module/.
- Inline flags across utility parsers now reject empty values consistently, e.g. --dataset=, --hak=,
--endpoint=, --output=.
- build-changelog now supports --flag=value inline syntax like the other utilities.
This commit is contained in:
+109
-24
@@ -645,19 +645,31 @@ func parseBuildHAKArgs(args []string) (buildHAKOptions, error) {
|
||||
}
|
||||
opts.sourceManifest = args[index]
|
||||
default:
|
||||
if value, ok := parseInlineFlagValue(arg, "--hak"); ok {
|
||||
if value, ok, err := requireInlineFlagValue(arg, "--hak"); ok || err != nil {
|
||||
if err != nil {
|
||||
return opts, err
|
||||
}
|
||||
opts.filteredHAKs = append(opts.filteredHAKs, value)
|
||||
continue
|
||||
}
|
||||
if value, ok := parseInlineFlagValue(arg, "--archive"); ok {
|
||||
if value, ok, err := requireInlineFlagValue(arg, "--archive"); ok || err != nil {
|
||||
if err != nil {
|
||||
return opts, err
|
||||
}
|
||||
opts.filteredArchives = append(opts.filteredArchives, value)
|
||||
continue
|
||||
}
|
||||
if value, ok := parseInlineFlagValue(arg, "--source-manifest"); ok {
|
||||
if value, ok, err := requireInlineFlagValue(arg, "--source-manifest"); ok || err != nil {
|
||||
if err != nil {
|
||||
return opts, err
|
||||
}
|
||||
opts.sourceManifest = value
|
||||
continue
|
||||
}
|
||||
if value, ok := parseInlineFlagValue(arg, "--music-dataset"); ok {
|
||||
if value, ok, err := requireInlineFlagValue(arg, "--music-dataset"); ok || err != nil {
|
||||
if err != nil {
|
||||
return opts, err
|
||||
}
|
||||
opts.musicDatasets = append(opts.musicDatasets, value)
|
||||
continue
|
||||
}
|
||||
@@ -679,6 +691,17 @@ func parseInlineFlagValue(arg, name string) (string, bool) {
|
||||
return strings.TrimSpace(strings.TrimPrefix(arg, prefix)), true
|
||||
}
|
||||
|
||||
func requireInlineFlagValue(arg, name string) (string, bool, error) {
|
||||
value, ok := parseInlineFlagValue(arg, name)
|
||||
if !ok {
|
||||
return "", false, nil
|
||||
}
|
||||
if value == "" {
|
||||
return "", true, fmt.Errorf("%s requires a value", name)
|
||||
}
|
||||
return value, true, nil
|
||||
}
|
||||
|
||||
type musicCommandOptions struct {
|
||||
datasets []string
|
||||
json bool
|
||||
@@ -765,7 +788,10 @@ func parseMusicCommandArgs(args []string) (musicCommandOptions, error) {
|
||||
case "--force":
|
||||
opts.force = true
|
||||
default:
|
||||
if value, ok := parseInlineFlagValue(arg, "--dataset"); ok {
|
||||
if value, ok, err := requireInlineFlagValue(arg, "--dataset"); ok || err != nil {
|
||||
if err != nil {
|
||||
return opts, err
|
||||
}
|
||||
opts.datasets = append(opts.datasets, value)
|
||||
continue
|
||||
}
|
||||
@@ -1417,29 +1443,56 @@ func parseDeployWikiArgs(commandName string, args []string) (topdata.DeployWikiO
|
||||
case "-h", "--help":
|
||||
return opts, fmt.Errorf("usage: %s [--source-dir <path>] [--endpoint <url>] [--token <token>] [--version <ver>] [--namespace <ns> ...] [--category <namespace=cid> ...] [--manifest <path>] [--dry-run] [--create] [--force]", commandName)
|
||||
default:
|
||||
if strings.HasPrefix(arg, "--source-dir=") {
|
||||
opts.SourceDir = strings.TrimPrefix(arg, "--source-dir=")
|
||||
} else if strings.HasPrefix(arg, "--endpoint=") {
|
||||
opts.Endpoint = strings.TrimPrefix(arg, "--endpoint=")
|
||||
} else if strings.HasPrefix(arg, "--username=") {
|
||||
opts.Username = strings.TrimPrefix(arg, "--username=")
|
||||
} else if strings.HasPrefix(arg, "--password=") {
|
||||
opts.Password = strings.TrimPrefix(arg, "--password=")
|
||||
} else if strings.HasPrefix(arg, "--token=") {
|
||||
opts.Token = strings.TrimPrefix(arg, "--token=")
|
||||
} else if strings.HasPrefix(arg, "--version=") {
|
||||
opts.Version = strings.TrimPrefix(arg, "--version=")
|
||||
} else if strings.HasPrefix(arg, "--namespace=") {
|
||||
opts.Namespaces = append(opts.Namespaces, strings.TrimPrefix(arg, "--namespace="))
|
||||
} else if strings.HasPrefix(arg, "--category=") {
|
||||
if value, ok, err := requireInlineFlagValue(arg, "--source-dir"); ok || err != nil {
|
||||
if err != nil {
|
||||
return opts, err
|
||||
}
|
||||
opts.SourceDir = value
|
||||
} else if value, ok, err := requireInlineFlagValue(arg, "--endpoint"); ok || err != nil {
|
||||
if err != nil {
|
||||
return opts, err
|
||||
}
|
||||
opts.Endpoint = value
|
||||
} else if value, ok, err := requireInlineFlagValue(arg, "--username"); ok || err != nil {
|
||||
if err != nil {
|
||||
return opts, err
|
||||
}
|
||||
opts.Username = value
|
||||
} else if value, ok, err := requireInlineFlagValue(arg, "--password"); ok || err != nil {
|
||||
if err != nil {
|
||||
return opts, err
|
||||
}
|
||||
opts.Password = value
|
||||
} else if value, ok, err := requireInlineFlagValue(arg, "--token"); ok || err != nil {
|
||||
if err != nil {
|
||||
return opts, err
|
||||
}
|
||||
opts.Token = value
|
||||
} else if value, ok, err := requireInlineFlagValue(arg, "--version"); ok || err != nil {
|
||||
if err != nil {
|
||||
return opts, err
|
||||
}
|
||||
opts.Version = value
|
||||
} else if value, ok, err := requireInlineFlagValue(arg, "--namespace"); ok || err != nil {
|
||||
if err != nil {
|
||||
return opts, err
|
||||
}
|
||||
opts.Namespaces = append(opts.Namespaces, value)
|
||||
} else if value, ok, err := requireInlineFlagValue(arg, "--category"); ok || err != nil {
|
||||
if err != nil {
|
||||
return opts, err
|
||||
}
|
||||
if opts.CategoryIDs == nil {
|
||||
opts.CategoryIDs = map[string]int{}
|
||||
}
|
||||
if err := parseWikiCategoryArg(strings.TrimPrefix(arg, "--category="), opts.CategoryIDs); err != nil {
|
||||
if err := parseWikiCategoryArg(value, opts.CategoryIDs); err != nil {
|
||||
return opts, err
|
||||
}
|
||||
} else if strings.HasPrefix(arg, "--manifest=") {
|
||||
opts.ManifestPath = strings.TrimPrefix(arg, "--manifest=")
|
||||
} else if value, ok, err := requireInlineFlagValue(arg, "--manifest"); ok || err != nil {
|
||||
if err != nil {
|
||||
return opts, err
|
||||
}
|
||||
opts.ManifestPath = value
|
||||
} else if arg == "--dry-run" {
|
||||
opts.DryRun = true
|
||||
} else if arg == "--force" {
|
||||
@@ -1546,7 +1599,39 @@ func parseBuildChangelogArgs(commandName string, args []string) (buildChangelogO
|
||||
case "-h", "--help":
|
||||
return opts, usage
|
||||
default:
|
||||
return opts, fmt.Errorf("unknown %s argument %q", commandName, arg)
|
||||
if value, ok, err := requireInlineFlagValue(arg, "--config"); ok || err != nil {
|
||||
if err != nil {
|
||||
return opts, err
|
||||
}
|
||||
opts.configPath = value
|
||||
} else if value, ok, err := requireInlineFlagValue(arg, "--output"); ok || err != nil {
|
||||
if err != nil {
|
||||
return opts, err
|
||||
}
|
||||
opts.outputPath = value
|
||||
} else if value, ok, err := requireInlineFlagValue(arg, "--current-tag"); ok || err != nil {
|
||||
if err != nil {
|
||||
return opts, err
|
||||
}
|
||||
opts.currentTag = value
|
||||
} else if value, ok, err := requireInlineFlagValue(arg, "--previous-tag"); ok || err != nil {
|
||||
if err != nil {
|
||||
return opts, err
|
||||
}
|
||||
opts.previousTag = value
|
||||
} else if value, ok, err := requireInlineFlagValue(arg, "--api-base-url"); ok || err != nil {
|
||||
if err != nil {
|
||||
return opts, err
|
||||
}
|
||||
opts.apiBaseURL = value
|
||||
} else if value, ok, err := requireInlineFlagValue(arg, "--token"); ok || err != nil {
|
||||
if err != nil {
|
||||
return opts, err
|
||||
}
|
||||
opts.token = value
|
||||
} else {
|
||||
return opts, fmt.Errorf("unknown %s argument %q", commandName, arg)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -391,6 +391,43 @@ paths:
|
||||
}
|
||||
}
|
||||
|
||||
func TestInlineFlagParsersRejectEmptyValues(t *testing.T) {
|
||||
if _, err := parseBuildHAKArgs([]string{"--hak="}); err == nil || !strings.Contains(err.Error(), "--hak requires a value") {
|
||||
t.Fatalf("expected empty --hak inline value error, got %v", err)
|
||||
}
|
||||
if _, err := parseMusicCommandArgs([]string{"--dataset="}); err == nil || !strings.Contains(err.Error(), "--dataset requires a value") {
|
||||
t.Fatalf("expected empty --dataset inline value error, got %v", err)
|
||||
}
|
||||
if _, err := parseDeployWikiArgs("deploy-wiki", []string{"--endpoint="}); err == nil || !strings.Contains(err.Error(), "--endpoint requires a value") {
|
||||
t.Fatalf("expected empty --endpoint inline value error, got %v", err)
|
||||
}
|
||||
if _, err := parseBuildChangelogArgs("build-changelog", []string{"--output="}); err == nil || !strings.Contains(err.Error(), "--output requires a value") {
|
||||
t.Fatalf("expected empty --output inline value error, got %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestParseBuildChangelogArgsAcceptsInlineValues(t *testing.T) {
|
||||
opts, err := parseBuildChangelogArgs("build-changelog", []string{
|
||||
"--config=scripts/changelog.json",
|
||||
"--output=CHANGELOG.md",
|
||||
"--current-tag=v2",
|
||||
"--previous-tag=v1",
|
||||
"--api-base-url=https://gitea.example/api/v1",
|
||||
"--token=secret",
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("parseBuildChangelogArgs failed: %v", err)
|
||||
}
|
||||
if opts.configPath != "scripts/changelog.json" ||
|
||||
opts.outputPath != "CHANGELOG.md" ||
|
||||
opts.currentTag != "v2" ||
|
||||
opts.previousTag != "v1" ||
|
||||
opts.apiBaseURL != "https://gitea.example/api/v1" ||
|
||||
opts.token != "secret" {
|
||||
t.Fatalf("unexpected changelog opts: %#v", opts)
|
||||
}
|
||||
}
|
||||
|
||||
func mkdirAll(t *testing.T, path string) {
|
||||
t.Helper()
|
||||
if err := os.MkdirAll(path, 0o755); err != nil {
|
||||
|
||||
@@ -5,6 +5,7 @@ import (
|
||||
"fmt"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
|
||||
"gitea.westgate.pw/ShadowsOverWestgate/sow-tools/internal/gff"
|
||||
"gitea.westgate.pw/ShadowsOverWestgate/sow-tools/internal/project"
|
||||
@@ -20,6 +21,13 @@ func ApplyHAKManifest(p *project.Project, manifestPath string) (ApplyManifestRes
|
||||
if manifestPath == "" {
|
||||
manifestPath = p.HAKManifestPath()
|
||||
}
|
||||
if strings.TrimSpace(p.EffectiveConfig().Paths.Source) == "" {
|
||||
return ApplyManifestResult{}, fmt.Errorf("cannot apply hak manifest: paths.source is not configured")
|
||||
}
|
||||
sourceRoot := filepath.Clean(p.SourceDir())
|
||||
if sourceRoot == "." || sourceRoot == string(filepath.Separator) || sourceRoot == filepath.Clean(p.Root) {
|
||||
return ApplyManifestResult{}, fmt.Errorf("cannot apply hak manifest: paths.source resolves to unsafe source root %s", sourceRoot)
|
||||
}
|
||||
|
||||
raw, err := os.ReadFile(manifestPath)
|
||||
if err != nil {
|
||||
@@ -31,7 +39,7 @@ func ApplyHAKManifest(p *project.Project, manifestPath string) (ApplyManifestRes
|
||||
return ApplyManifestResult{}, fmt.Errorf("parse hak manifest: %w", err)
|
||||
}
|
||||
|
||||
moduleSource := filepath.Join(p.SourceDir(), "module", "module.ifo.json")
|
||||
moduleSource := filepath.Join(sourceRoot, "module", "module.ifo.json")
|
||||
sourceRaw, err := os.ReadFile(moduleSource)
|
||||
if err != nil {
|
||||
return ApplyManifestResult{}, fmt.Errorf("read module ifo source: %w", err)
|
||||
|
||||
@@ -548,6 +548,43 @@ func TestExtractKeepsConsumedModuleArchiveWhenExtractionFails(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestApplyHAKManifestRefusesUnsetSourcePath(t *testing.T) {
|
||||
root := t.TempDir()
|
||||
mustMkdir(t, filepath.Join(root, "assets"))
|
||||
mustMkdir(t, filepath.Join(root, "build"))
|
||||
mustMkdir(t, filepath.Join(root, "module"))
|
||||
mustWriteFile(t, filepath.Join(root, "nwn-tool.yaml"), `
|
||||
module:
|
||||
name: Test Module
|
||||
resref: testmod
|
||||
paths:
|
||||
assets: assets
|
||||
build: build
|
||||
`)
|
||||
mustWriteFile(t, filepath.Join(root, "build", "haks.json"), `{"module_haks":["core"],"haks":[]}`)
|
||||
mustWriteFile(t, filepath.Join(root, "module", "module.ifo.json"), "{}\n")
|
||||
|
||||
p, err := project.Load(root)
|
||||
if err != nil {
|
||||
t.Fatalf("load project: %v", err)
|
||||
}
|
||||
|
||||
_, err = ApplyHAKManifest(p, filepath.Join(root, "build", "haks.json"))
|
||||
if err == nil {
|
||||
t.Fatal("expected apply manifest to fail")
|
||||
}
|
||||
if !strings.Contains(err.Error(), "paths.source is not configured") {
|
||||
t.Fatalf("expected missing source path error, got %v", err)
|
||||
}
|
||||
raw, readErr := os.ReadFile(filepath.Join(root, "module", "module.ifo.json"))
|
||||
if readErr != nil {
|
||||
t.Fatalf("read root module file: %v", readErr)
|
||||
}
|
||||
if string(raw) != "{}\n" {
|
||||
t.Fatalf("expected root module file to remain unchanged, got %q", string(raw))
|
||||
}
|
||||
}
|
||||
|
||||
func TestBuildSplitsConfiguredHAKs(t *testing.T) {
|
||||
root := t.TempDir()
|
||||
mustMkdir(t, filepath.Join(root, "src"))
|
||||
|
||||
@@ -470,6 +470,8 @@ func (p *Project) ValidateLayout() error {
|
||||
failures = append(failures, validateValidationConfig(p.Config.Validation)...)
|
||||
effective := p.EffectiveConfig()
|
||||
failures = append(failures, validateRelativePath("paths.cache", effective.Paths.Cache)...)
|
||||
failures = append(failures, validateTreeRootPath("paths.source", effective.Paths.Source)...)
|
||||
failures = append(failures, validateTreeRootPath("paths.assets", effective.Paths.Assets)...)
|
||||
failures = append(failures, validateRelativePath("outputs.module_archive", effective.Outputs.ModuleArchive)...)
|
||||
failures = append(failures, validateRelativePath("outputs.hak_manifest", effective.Outputs.HAKManifest)...)
|
||||
failures = append(failures, validateRelativePath("outputs.hak_archive", effective.Outputs.HAKArchive)...)
|
||||
@@ -1406,6 +1408,18 @@ func validateRelativePath(field, path string) []error {
|
||||
return failures
|
||||
}
|
||||
|
||||
func validateTreeRootPath(field, path string) []error {
|
||||
failures := validateRelativePath(field, path)
|
||||
trimmed := strings.TrimSpace(path)
|
||||
if trimmed == "" {
|
||||
return failures
|
||||
}
|
||||
if filepath.Clean(filepath.FromSlash(trimmed)) == "." {
|
||||
failures = append(failures, fmt.Errorf("%s must not resolve to the repository root: %q", field, path))
|
||||
}
|
||||
return failures
|
||||
}
|
||||
|
||||
func sameCleanPath(a, b string) bool {
|
||||
return filepath.Clean(a) == filepath.Clean(b)
|
||||
}
|
||||
|
||||
@@ -648,6 +648,30 @@ func TestValidateLayoutRejectsUnsafeGeneratedOutputPaths(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestValidateLayoutRejectsRootSourceAndAssetPaths(t *testing.T) {
|
||||
root := t.TempDir()
|
||||
mkdirAll(t, filepath.Join(root, "build"))
|
||||
|
||||
proj := &Project{
|
||||
Root: root,
|
||||
Config: Config{
|
||||
Module: ModuleConfig{Name: "Test", ResRef: "test"},
|
||||
Paths: PathConfig{Source: ".", Assets: ".", Build: "build"},
|
||||
},
|
||||
}
|
||||
|
||||
err := proj.ValidateLayout()
|
||||
if err == nil {
|
||||
t.Fatal("expected root source and asset path validation error")
|
||||
}
|
||||
if !strings.Contains(err.Error(), "paths.source") {
|
||||
t.Fatalf("expected source path validation error, got %v", err)
|
||||
}
|
||||
if !strings.Contains(err.Error(), "paths.assets") {
|
||||
t.Fatalf("expected assets path validation error, got %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestValidateLayoutRejectsInvalidAutogenConsumerConfig(t *testing.T) {
|
||||
root := t.TempDir()
|
||||
mkdirAll(t, filepath.Join(root, "src"))
|
||||
|
||||
Reference in New Issue
Block a user