Skip to content

Harden output destinations and CLI process lifecycle - #98

Open
apstndb wants to merge 3 commits into
mainfrom
fix/codebase-review-2026-09-17
Open

apstndb wants to merge 3 commits into
mainfrom
fix/codebase-review-2026-09-17

Conversation

@apstndb

@apstndb apstndb commented Sep 16, 2026

Copy link
Copy Markdown
Owner

Summary

Harden output destinations and CLI process lifecycle after a local codebase review.

  • Detect --output / --plan-output collisions when the parent directory is a symlink and the target does not exist yet. Temp files are created in the resolved directory.
  • Reject directories and other non-regular destinations before the query. /dev/null is mapped in-process like /dev/stdout and /dev/stderr.
  • Handle SIGTERM with the same sink abort path as SIGINT, so cancelled runs do not leave .execspansql-*.tmp files.
  • Published files keep mode 0600 (including replacements). README documents this as intentional for possibly sensitive rows.
  • CLI errors print without a timestamp or a full usage dump. --version is wired through GoReleaser ldflags. Exit 2 is usage/parse; exit 3 is output failure after a committed statement.
  • Drop unused jqresult helpers and the unused Compile input-mode argument.
  • Bump google.golang.org/grpc to v1.83.2, golang.org/x/text to v0.41.0, and github.com/apstndb/spanemuboost to v0.4.7.

Behavior changes

  • Existing-file overwrite still allowed; the published inode is always 0600.
  • Two /dev/null destinations are allowed (unlike two stdout destinations).
  • Scripts that treated every failure as exit 1 now see 2 or 3 for usage errors and post-commit output failures.

Tests run

Command Exit
go vet ./... 0
GOTOOLCHAIN=go1.25.13 golangci-lint run ./... 0 (0 issues)
go test ./params/... ./jqresult/... ./resultset/... ./internal/... 0
DOCKER_HOST=unix://$HOME/.colima/default/docker.sock TESTCONTAINERS_DOCKER_SOCKET_OVERRIDE=/var/run/docker.sock go test ./... 0
go run golang.org/x/vuln/cmd/govulncheck@v1.8.0 ./... 0 reachable

google.golang.org/grpc v1.81.1 had two reachable vulnerabilities
fixed in v1.83.1; pin v1.83.2. golang.org/x/text moves to v0.41.0
with that grpc requirement (above the GO-2026-5970 floor).
spanemuboost v0.4.7 picks up the newer emulator image.
golang.org/x/term also moves to v0.45.0 from the same go get.
Symlinked parent directories of not-yet-created files compared
unequal, so --output and --plan-output could publish onto the
same path. Directories and other non-regular targets were only
rejected at rename, after DML had committed.

Resolve the deepest existing ancestor before comparing paths or
creating temps, reject non-regular destinations up front, and
map /dev/null in-process like /dev/stdout. SIGTERM now runs the
same sink abort path as SIGINT. Published files keep mode 0600.
Errors no longer dump usage or a log timestamp; --version and
distinct exit codes cover parse vs post-commit output failure.
stripLeadingComments is removed in favor of the memefish DML
lexer. jqresult.Compile no longer takes an unused input mode.
ResultSetMapFromRowIterator had no callers. RowIter.Prime, Next,
and Drain were exported wrappers around unexported methods that
only Lazy uses. NeedsGraphviz now selects the SVG/PNG renderer
path instead of sitting unused beside a test.
@apstndb apstndb added bug Something isn't working enhancement New feature or request labels Sep 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant