fix: contain generated files and assets and harden SQL rendering
This commit is contained in:
@@ -191,6 +191,7 @@ jobs:
|
||||
# (file output = full DDL); set options.full_ddl to force full DDL
|
||||
overwrite: false # default false
|
||||
options:
|
||||
lowercase_identifiers: false # lowercase names/references; SQL expressions and data keep their case
|
||||
flatten_schema: false
|
||||
schema: public
|
||||
package: models # for gorm/bun output
|
||||
|
||||
+30
-2
@@ -96,11 +96,39 @@ INSERT INTO assets (message, payload)
|
||||
VALUES (:message, decode(:payload, 'base64')::bytea);
|
||||
```
|
||||
|
||||
- `path`: File path resolved relative to the SQL file containing the directive
|
||||
- `path`: Relative file path beneath the SQL file’s directory; traversal, absolute paths, and symlinks are rejected
|
||||
- `var`: Named placeholder to replace, such as `:message`
|
||||
- `mode`: `text` embeds an escaped SQL string literal; `base64` embeds a base64 string literal
|
||||
|
||||
The directive comment is removed from the SQL, and every matching placeholder is replaced before the script is listed or executed.
|
||||
A directive must be a standalone SQL line comment. Directive-like text inside
|
||||
quoted strings, quoted identifiers, block comments, or dollar-quoted bodies is
|
||||
ordinary SQL content. The directive is removed, and every matching placeholder in
|
||||
executable SQL is replaced once before listing or execution. Replacements are
|
||||
literal, preserving `$1`, `${2}`, `$HOME`, and other dollar text; inserted content
|
||||
is never scanned for another placeholder. Duplicate embed variables or variables
|
||||
used only inside strings/comments are errors. PostgreSQL casts (`::type`) remain
|
||||
unchanged. Asset-call named parameters follow these same lexical boundaries;
|
||||
repeated parameters share one positional argument.
|
||||
|
||||
Embed and manifest asset reads share root-scoped containment and a 64 MiB per-file
|
||||
limit. Only regular files are accepted. The SQL directory reader uses its selected
|
||||
script tree as an explicit asset root; embed paths remain relative to each script.
|
||||
Symlinks in asset paths are rejected, and containment is enforced again at open time.
|
||||
Manifest entries are checked at discovery and execution, so replacing a file with an
|
||||
escaping symlink after scanning cannot expose an external file. No per-directive
|
||||
external-file override is supported; copy required assets into the allowed tree.
|
||||
|
||||
Library callers can choose a trusted root and size limit with
|
||||
`assetloader.ProcessEmbedDirectivesWithOptions(..., assetloader.EmbedOptions{
|
||||
AssetRoot: root, MaxFileBytes: limit})`; zero means the 64 MiB default, and negative
|
||||
or unbounded limits are rejected. Manifest items retain `AssetRoot` and
|
||||
`MaxFileBytes` for execution-time validation.
|
||||
|
||||
For scripts loaded from a model rather than a SQL directory, the executor requires
|
||||
trusted writer configuration `Metadata["asset_root"]` before expanding directives.
|
||||
Model `source_path` metadata supplies resolution context only; it does not grant
|
||||
local-file access. Missing or out-of-root source paths fail before reading assets.
|
||||
|
||||
|
||||
## Commands
|
||||
|
||||
|
||||
@@ -570,3 +570,13 @@ COMMIT;
|
||||
- [RelSpec Documentation](../README.md)
|
||||
- [Model Structure Reference](../pkg/models/)
|
||||
- [Example Templates](../examples/templates/)
|
||||
|
||||
## Output path safety
|
||||
|
||||
In schema, domain, script, and table modes, `--output` always names a directory,
|
||||
including a nonexistent directory without a trailing slash. An empty value uses
|
||||
the current directory. Database mode uses a single output file as before.
|
||||
Generated paths must stay relative to that directory. Traversal, absolute or
|
||||
Windows-specific unsafe paths, symlinks, filename collisions (including case
|
||||
variants), and file/directory collisions are rejected before publishing artifacts.
|
||||
Nested patterns such as `{{.ParentSchema.Name}}/{{.Name}}.txt` remain supported.
|
||||
|
||||
@@ -14,6 +14,18 @@ Implementation batches (2026-10-12):
|
||||
- [x] **A01, shared execution results:** PostgreSQL, SQLite, SQL Server, and raw script execution now use `writers.ExecutionReport` / `ExecutionFailure`. Stop/continue modes always fail on SQL errors; counters/results survive early failure and aggregate across script schemas. Reports distinguish applied, failed, skipped, rolled-back, and uncertain operations. Generated DDL supports optional atomic execution; raw scripts report script-managed outcomes without promising statement-level commit/rollback. Requested report failures are errors in every executor. Live PostgreSQL, SQL Server, SQLite, and dependent-job regressions pass.
|
||||
- [x] **A02, conversion expressions and preflight:** column metadata `pgsql_using` configures an explicit SQL conversion expression; `pgsql_preflight` adds a row predicate. Full-DDL and diff plans evaluate every existing value before schema mutations, and live preflight failure stops even with continuation enabled. Tests cover later invalid rows, preservation of a constraint scheduled for removal, custom conversions, predicate rejection, invalid configuration, and block-delimiter collisions. Standalone schema generation also includes preflights.
|
||||
|
||||
- [x] **A03, contained template output:** multi-file output has an explicit directory root, portable relative filename validation, and preflight collision/symlink checks. Rendering/validation complete before writing; exclusive per-file staging and root-scoped rename prevent generated filenames from escaping the root. Multi-file atomic publication remains A16 work.
|
||||
- [x] **A04, contained asset reads:** embed directives and manifest execution use shared root-scoped reads, symlink checks, regular-file validation, and a bounded 64 MiB default. Manifest paths are rechecked at execution. Imported source-path metadata cannot authorize file reads without a trusted configured asset root. Library callers can explicitly configure a root/size limit; directives cannot opt out of containment.
|
||||
- [x] **A05, PostgreSQL quoting:** schema/full-DDL/diff paths and embedded templates always quote model identifiers, quote literal/catalog/regclass values, sanitize names in generated line comments, and choose collision-safe dollar delimiters for procedural and audit function bodies. NUL is rejected before output; migration comparisons preserve identifier case. Raw SQL expressions remain an explicit executable-input boundary.
|
||||
- [x] **Requested lowercase option:** `--lowercase` on convert/batch/merge/split/templ/job run, job `options.lowercase_identifiers`, and PostgreSQL `WriterOptions.LowercaseIdentifiers` normalize names and structured references on a copy, rejecting collisions. SQL expressions, defaults, comments, enum values, and data keep their case; callers must adjust references inside raw SQL themselves.
|
||||
- [x] **A08, SQL-aware substitution/execution:** embeds use one-pass literal substitution, preserving dollar characters and preventing replacement rescanning. Named parameters and embed directives respect SQL strings, quoted identifiers, nested comments, casts, and dollar bodies; duplicate/unused embed variables fail. MySQL executes structured generated statements without splitting multiline literals or stripping their contents.
|
||||
|
||||
A05–A08 verification: unit regressions and live PostgreSQL 18 checks cover unusual names, comments/defaults, rerunnable file/statement output, diff migrations, sequences, primary-key replacement, audit functions/triggers, and both standard-string modes. MySQL statement preservation is unit-tested; a live MySQL instance was not used for A08. `go test -mod=vendor -p 1 ./...` passed with `RELSPEC_TEST_PG_CONN` set to the disposable instance; `go vet -mod=vendor ./...`, executable CLI help checks, and `git diff --check` passed. SQL Server tests were not configured for this batch. The disposable instance was removed after verification.
|
||||
|
||||
Additional observation: a parallel live full-suite run hit a PostgreSQL reader catalog race (`could not open relation with OID`) while another package's test dropped schema objects. The writer safely fell back to full DDL and its preflight still stopped all mutations, but the existing test assumed no successful read-only preflights before failure. Running live packages serially passed. Reader resilience to concurrent catalog changes remains outside A05/A08; A10 specifically concerns cancellation/timeouts.
|
||||
|
||||
A03–A04 verification: `go test -mod=vendor ./...`, targeted regression tests, `go vet -mod=vendor ./...`, and `git diff --check` passed. Database-dependent tests were not configured for this filesystem-only batch. Windows test binaries were cross-compiled for the template and asset packages; Windows runtime filesystem tests remain unperformed.
|
||||
|
||||
Verification: `go test -mod=vendor ./...` passed with `RELSPEC_TEST_PG_CONN` and `RELSPEC_TEST_MSSQL_CONN` pointing to disposable PostgreSQL 18 and SQL Server 2022 Developer instances; SQLite regressions use temporary database files. A minimal public table was seeded for the existing PostgreSQL `ReadTable` test, which requires a nonempty default schema. `go vet -mod=vendor ./...` and `git diff --check` passed. Executable CLI checks verified `scripts execute --ignore-errors`, `--continue-on-error`, and `--coe` all attempt a later script after failure and exit 1. Raw script tests intentionally make no statement-level rollback claim. A09 remains open for concurrency, history, and migration recovery; sequence/external side effects are outside transaction rollback guarantees. Original audit evidence is retained for historical context.
|
||||
|
||||
## Assessment
|
||||
@@ -92,6 +104,8 @@ The broad exception handler also treats unrelated failures as permission to atte
|
||||
|
||||
### A03 — Generated template filenames escape the output directory
|
||||
|
||||
**Repair status: addressed.** Multi-file roots, generated paths, collisions, symlinks, and root-scoped writes are covered by regression tests. A nonexistent output path now consistently denotes the directory itself rather than its parent.
|
||||
|
||||
**P1 / High · Security · Reproduced**
|
||||
|
||||
Evidence: `pkg/writers/template/writer.go:249–308`.
|
||||
@@ -106,6 +120,8 @@ An imported model can therefore cause writes outside the intended destination, i
|
||||
|
||||
### A04 — SQL embed directives can read outside the script directory
|
||||
|
||||
**Repair status: addressed.** Shared bounded root-scoped reads protect directives and manifest assets, including revalidation after discovery. Model source-path metadata alone no longer grants local file access.
|
||||
|
||||
**P1 / High · Security · Reproduced**
|
||||
|
||||
Evidence: `pkg/assetloader/embed.go:73–74`; compare the containment checks in `pkg/assetloader/loader.go` and `pkg/jobs/jobs.go`.
|
||||
|
||||
Reference in New Issue
Block a user