Skip to content

Improve SQL parameter binding and escaping#965

Merged
kvch merged 4 commits into
mainfrom
fix-sql-escaping-parameters
Jul 6, 2026
Merged

Improve SQL parameter binding and escaping#965
kvch merged 4 commits into
mainfrom
fix-sql-escaping-parameters

Conversation

@kvch

@kvch kvch commented Jul 6, 2026

Copy link
Copy Markdown
Collaborator

Description

This PR addresses multiple SQL issues in pgstream.

  • parameters are binded where possible
  • source.postgresql.replication.replication_slot is escaped correctly
  • IsQuotedIdentifier not only checks if the quotes but validates the whole token

Type of Change

Please select the relevant option(s):

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 💥 Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • 📚 Documentation update
  • 🔧 Refactoring (no functional changes)
  • ⚡ Performance improvement
  • 🧪 Test coverage improvement
  • 🔨 Build/CI changes
  • 🧹 Code cleanup

Testing

  • Unit tests added/updated
  • Integration tests added/updated
  • Manual testing performed
  • All existing tests pass

Checklist

  • Code follows project style guidelines
  • Self-review completed
  • Code is well-commented
  • Documentation updated where necessary

@kvch kvch requested a review from tsg July 6, 2026 09:32
@kvch kvch force-pushed the fix-sql-escaping-parameters branch from 7ee759a to a0ad65d Compare July 6, 2026 09:36
@kvch kvch enabled auto-merge (squash) July 6, 2026 09:55
@kvch kvch merged commit 96da911 into main Jul 6, 2026
8 checks passed
@kvch kvch deleted the fix-sql-escaping-parameters branch July 6, 2026 10:00
kvch added a commit that referenced this pull request Jul 6, 2026
Backports #965 ("Improve SQL parameter binding and escaping") from
`main` to the `v1.1.x` release branch.
@kvch kvch added the backport-0.9.x Backport this PR to v0.9.x release label Jul 6, 2026
@xata-bot

xata-bot commented Jul 6, 2026

Copy link
Copy Markdown

Backport failed for v0.9.x, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin v0.9.x
git worktree add -d .worktree/backport-965-to-v0.9.x origin/v0.9.x
cd .worktree/backport-965-to-v0.9.x
git switch --create backport-965-to-v0.9.x
git cherry-pick -x 96da911d02464df77d6994ea18233bc20ca04bcb

@kvch kvch removed the backport-0.9.x Backport this PR to v0.9.x release label Jul 6, 2026
@github-actions

github-actions Bot commented Jul 6, 2026

Copy link
Copy Markdown

Merging this branch will not change overall coverage

Impacted Packages Coverage Δ 🤖
github.com/xataio/pgstream/internal/postgres 29.79% (ø)
github.com/xataio/pgstream/pkg/snapshot/generator/postgres/schema/pgdumprestore 89.30% (ø)
github.com/xataio/pgstream/pkg/snapshot/store/postgres 85.71% (ø)
github.com/xataio/pgstream/pkg/stream 40.23% (ø)
github.com/xataio/pgstream/pkg/wal/replication/postgres 65.07% (ø)

Coverage by file

Changed files (no unit tests)

Changed File Coverage Δ Total Covered Missed 🤖
github.com/xataio/pgstream/internal/postgres/pg_utils.go 49.61% (ø) 129 64 65
github.com/xataio/pgstream/pkg/snapshot/generator/postgres/schema/pgdumprestore/snapshot_pg_dump_restore_generator.go 84.65% (ø) 404 342 62
github.com/xataio/pgstream/pkg/snapshot/store/postgres/pg_snapshot_store.go 85.71% (ø) 56 48 8
github.com/xataio/pgstream/pkg/stream/stream_init.go 5.97% (ø) 134 8 126
github.com/xataio/pgstream/pkg/wal/replication/postgres/pg_replication_handler.go 64.29% (ø) 140 90 50

Please note that the "Total", "Covered", and "Missed" counts above refer to code statements instead of lines of code. The value in brackets refers to the test coverage of that file in the old version of the code.

Changed unit test files

  • github.com/xataio/pgstream/internal/postgres/pg_utils_test.go
  • github.com/xataio/pgstream/pkg/snapshot/store/postgres/pg_snapshot_store_test.go

kvch added a commit that referenced this pull request Jul 6, 2026
…#968)

Backport of #965 to `v0.9.x`.

#### Description

This addresses several SQL escaping issues, adapted for the v0.9.x
branch:

- Bind parameters where possible: the snapshot-request status query and
the replication slot create/drop statements now use `$1` placeholders
instead of `fmt.Sprintf`.
- Validate the replication slot name (`IsValidReplicationSlotName`)
before use.
- `IsQuotedIdentifier` now validates the whole token rather than just
checking the surrounding quotes, so malicious lookalikes (e.g. `"a" ;
DROP TABLE users; --"`) are escaped rather than trusted as
already-quoted.
- Quote the role name when granting `rds_replication` during
pg_dump/restore.

#### Adaptations for v0.9.x

Parts of #965 target functionality that doesn't exist on v0.9.x and were
intentionally skipped:

- **add-tables/filter-tables plugin argument escaping** — configurable
plugin arguments don't exist on v0.9.x.
- **`cleanupV09xState` event-trigger quoting** — that helper is
v1.x-only.
- **`UnquoteIdentifier` tests** — the function is v1.x-only.

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants