Skip to content

fix: validate CDC access against configured streams to prevent zero LSN poisoning#77998

Open
Yarden Carmeli (yardencarmeli) wants to merge 8 commits into
masterfrom
yarden/source-mssql-permission-issues-zero-lsn
Open

fix: validate CDC access against configured streams to prevent zero LSN poisoning#77998
Yarden Carmeli (yardencarmeli) wants to merge 8 commits into
masterfrom
yarden/source-mssql-permission-issues-zero-lsn

Conversation

@yardencarmeli
Copy link
Copy Markdown
Contributor

@yardencarmeli Yarden Carmeli (yardencarmeli) commented May 11, 2026

What

Fixes 11451

How

  • Added getAccessibleCaptureInstances() which calls sys.sp_cdc_help_change_data_capture to discover the capture instances the connection user can actually read.
  • Added validateConfiguredStreamsAreAccessible() which compares the configured incremental streams against the accessible capture instances. If any configured stream is missing, throws ConfigErrorException naming the missing table(s) and pointing to the MSSQL setup docs.
  • Updated validateLsnStillAvailable() to limit the MIN LSN query to only configured streams that are also captured, via a WHERE capture_instance IN (...) clause. This avoids using unrelated CDC-enabled tables that the user happens to have permissions on but did not configure.
  • Wired the three steps together in deserializeState(): discover → validate access → check LSN range. ConfigErrorException is rethrown so CdcPartitionsCreator aborts the sync with a config error rather than routing through abortCdcSync()'s reset behavior (a reset wouldn't fix a permissions issue).
  • Injected ConfiguredAirbyteCatalog into MsSqlServerDebeziumOperations so the validation can read the user's configured streams.

Review guide

User Impact

Can this PR be safely reverted and rolled back?

  • YES 💚
  • NO ❌

Important

Active progressive rollout warning for source-mssql.

  • (Click to Approve:) Bypass the active progressive rollout warning for source-mssql in the PR comment here.

@github-actions
Copy link
Copy Markdown
Contributor

👋 Greetings, Airbyte Team Member!

Here are some helpful tips and reminders for your convenience.

💡 Show Tips and Tricks

PR Slash Commands

Airbyte Maintainers (that's you!) can execute the following slash commands on your PR:

  • 🛠️ Quick Fixes
    • /format-fix - Fixes most formatting issues.
    • /bump-version - Bumps connector versions, scraping changelog description from the PR title.
      • Bump types: patch (default), minor, major, major_rc, rc, promote.
      • The rc type is a smart default: applies minor_rc if stable, or bumps the RC number if already RC.
      • The promote type strips the RC suffix to finalize a release.
      • Example: /bump-version type=rc or /bump-version type=minor
    • /bump-progressive-rollout-version - Alias for /bump-version type=rc. Bumps with an RC suffix and enables progressive rollout.
  • ❇️ AI Testing and Review (internal link: AI-SDLC Docs):
    • /ai-prove-fix - Runs prerelease readiness checks, including testing against customer connections.
    • /ai-canary-prerelease - Rolls out prerelease to 5-10 connections for canary testing.
    • /ai-review - AI-powered PR review for connector safety and quality gates.
  • 📝 AI Documentation:
    • /ai-docs-review - AI-powered documentation review for PRs with connector changes.
    • /ai-create-docs-pr - Creates a documentation PR for connector changes, stacked on the current PR.
  • 🚀 Connector Releases:
    • /publish-connectors-prerelease - Publishes pre-release connector builds (tagged as {version}-preview.{git-sha}) for all modified connectors in the PR.
  • ☕️ JVM connectors:
    • /update-connector-cdk-version connector=<CONNECTOR_NAME> - Updates the specified connector to the latest CDK version.
      Example: /update-connector-cdk-version connector=destination-bigquery
  • 🐍 Python connectors:
    • /poe connector source-example lock - Run the Poe lock task on the source-example connector, committing the results back to the branch.
    • /poe source example lock - Alias for /poe connector source-example lock.
    • /poe source example use-cdk-branch my/branch - Pin the source-example CDK reference to the branch name specified.
    • /poe source example use-cdk-latest - Update the source-example CDK dependency to the latest available version.
  • ⚙️ Admin commands:
    • /force-merge reason="<REASON>" - Force merges the PR using admin privileges, bypassing CI checks. Requires a reason.
      Example: /force-merge reason="CI is flaky, tests pass locally"
📚 Show Repo Guidance

Helpful Resources

📝 Edit this welcome message.

@yardencarmeli
Copy link
Copy Markdown
Contributor Author

Yarden Carmeli (yardencarmeli) commented May 11, 2026

/format-fix

Format-fix job started... Check job output.

✅ Changes applied successfully. (696ccb0)

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 11, 2026

source-mssql Connector Test Results

 12 files   12 suites   45s ⏱️
116 tests 116 ✅ 0 💤 0 ❌
226 runs  226 ✅ 0 💤 0 ❌

Results for commit 258e12b.

♻️ This comment has been updated with latest results.

@yardencarmeli
Copy link
Copy Markdown
Contributor Author

Yarden Carmeli (yardencarmeli) commented May 11, 2026

/publish-connectors-prerelease

Pre-release Connector Publish Started

Publishing pre-release build for connector source-mssql.
PR: #77998

Pre-release versions will be tagged as {version}-preview.fc21138
and are available for version pinning via the scoped_configuration API.

View workflow run
Pre-release Publish: SUCCESS

Docker image (pre-release):
airbyte/source-mssql:4.4.7-preview.fc21138

Docker Hub: https://hub.docker.com/layers/airbyte/source-mssql/4.4.7-preview.fc21138

Registry JSON:

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 11, 2026

Deploy preview for airbyte-docs ready!

Project:airbyte-docs
Status: ✅  Deploy successful!
Preview URL:https://airbyte-docs-q1fhe6862-airbyte-growth.vercel.app
Latest Commit:258e12b

Deployed with vercel-action

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 13, 2026

Detected source-mssql Active Rollout: true

Important

Active progressive rollout warning for source-mssql.

To bypass this warning, click on the matching checkbox in the PR description. Look for the checkbox text:

(Click to Approve:) Bypass the active progressive rollout warning for source-mssql in the PR comment

Version on master Branch: 4.4.7

  • RC marker on master branch: false

PR Description Checkbox Status

  • Bypass checkbox checked: false

ℹ️ More Information

Show/hide details...

🤔 What happens if this PR is merged

Checking the checkbox will allow the PR to merge, but it does not necessarily stop the active rollout by itself. The result of the PR merging depends on what connector version is published.

Expected outcomes by type of version number change:

If connector version is not modified in this PR...

No new connector version should be released, and the active rollout should continue unchanged.

If the connector version increments to a higher `-rc` version...

After this PR is merged, the new RC will be published and registered, replacing the active RC marker. When the new RC is registered, the platform cancels any existing non-terminal rollout for this connector without unpinning actors.

After merging, you still need to start the new rollout. During start, pinned actors from the previous rollout can be moved to the new RC.

If the connector version changes from RC to non-RC (GA) version...

You should not merge the PR unless/until the RC has been finalized as canceled. See above Rollout state for detected status.

[!Warning]
This PR should not be merged if the RC rollout is still active. First finalize the active rollout as successful or cancel it in Connector Rollout Manager.

When you finalize an RC rollout as successful, the platform triggers a promotion workflow that strips the -rc suffix, removes stable-version registryOverrides, disables progressive rollout, force-merges that promotion, and unpins actors.

🔁 How to rerun this check

To rerun the check, simply check and uncheck the box, or else modify the PR description and/or title in any way.

Alternatively, you can find the Active Progressive Rollout CI workflow and manually rerun it (although this is generally slower than the above methods).


This comment will be updated as PR and/or rollout status changes.

Workflow run

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 13, 2026

Note

Detected that there are differences in the Gradle dependencies.

@rodireich Rodi Reich Zilberman (rodireich) marked this pull request as ready for review May 21, 2026 21:51
@octavia-bot
Copy link
Copy Markdown
Contributor

octavia-bot Bot commented May 22, 2026

AI PR Review starting...

Reviewing PR for connector safety and quality.
View playbook

Devin AI session created successfully!

@devin-ai-integration
Copy link
Copy Markdown
Contributor

I’m reviewing this PR with the AI PR Review gate playbook.

Session: https://app.devin.ai/sessions/c97dfdc4c3b944788e768fcd0274b882

@devin-ai-integration
Copy link
Copy Markdown
Contributor

↪️ Triggering /ai-review per Hands-Free AI Triage Project triage next step.

Reason: PR is ready for review and has no complete AI review result on the current head. The active rollout warning remains a human approval gate.

https://github.com/airbytehq/oncall/issues/11451
Devin session

@devin-ai-integration
Copy link
Copy Markdown
Contributor

AI PR Review Report

Review Action: APPROVE
Risk Level: 3 — Logic change to CDC LSN validation in source-mssql; adds pre-sync permission check and scopes existing MIN LSN query to configured streams.

Gate Status
PR Hygiene PASS
Code Hygiene WARNING
Test Coverage PASS
Code Security PASS
Per-Record Performance PASS
Breaking Dependencies WARNING
Backwards Compatibility PASS
Forwards Compatibility PASS
Behavioral Changes PASS
Out-of-Scope Changes PASS
CI Checks PASS
Live / E2E Tests PASS

📋 PR Details

Connector & PR Info

Connector(s): source-mssql
PR: #77998
HEAD SHA: 258e12bd868d66281892886838625e619f0af91b
Session: https://app.devin.ai/sessions/c97dfdc4c3b944788e768fcd0274b882

Risk Level

Level: 3 — Medium (typical connector change)
Rationale: Logic change to CDC LSN validation in MsSqlServerDebeziumOperations.kt; adds a new pre-sync permission check (getAccessibleCaptureInstances + validateConfiguredStreamsAreAccessible) and scopes the existing validateLsnStillAvailable MIN LSN query to only configured streams. Also bumps CDK 1.1.3 → 1.1.7. Changes are confined to a single connector and are well-tested by CI (116/116 tests passing).

Risk Level is reported for downstream consumers (e.g. auto-merge policy, reviewer routing). It does not change the review action — APPROVE here means "no blocking objection," not "safe to merge unattended."

Review Action Details

APPROVE - All enforced gates pass. This does NOT authorize auto-merge; the merge decision remains with humans or a separate policy.

Note: This report returns APPROVE whenever all enforced gates PASS, regardless of what the PR changes. APPROVE is a quality signal, not a merge authorization; any auto-merge behavior is governed by separate policy reading decision and risk_level from the metrics marker.

🔍 Gate Evaluation Details

Gate-by-Gate Analysis

Gate Status Enforced? Details
PR Hygiene PASS Yes Description present (1519 raw chars, 1635 visible chars). Changelog updated in docs/integrations/sources/mssql.md. No unresolved human reviewer comments.
Code Hygiene WARNING WARNING Source code modified (MsSqlServerDebeziumOperations.kt) with test file also modified (MsSqlServerDatatypeIntegrationTest.kt). However, the test change adds only CDC scan setup, not a dedicated unit test for the new validation logic. Note: Live validation evidence present (Pre-Release Checks pass) — risk is mitigated but unit test coverage still recommended.
Test Coverage PASS Yes Behavioral change detected (title contains "fix", linked to oncall issue 11451). Test file MsSqlServerDatatypeIntegrationTest.kt is modified with new test content (adds EXEC sys.sp_cdc_scan setup for CDC validation). CI integration tests pass (116/116).
Code Security PASS Yes No auth/credential file patterns matched. metadata.yaml diff only contains dockerImageTag version bump — no security keywords in diff hunks.
Per-Record Performance PASS WARNING New methods (getAccessibleCaptureInstances, validateConfiguredStreamsAreAccessible) run during deserializeState() — once per sync initialization, not per-record. No per-record processing code paths affected.
Breaking Dependencies WARNING WARNING CDK version bumped from 1.1.3 → 1.1.7 in gradle.properties. This is a minor version bump (patch level change). Gradle dependency diff was detected by CI but the report job concluded as neutral (no alarming changes flagged). Recommend verifying CDK 1.1.7 release notes for behavioral changes.
Backwards Compatibility PASS Warning (elevates Risk Level) No spec changes. metadata.yaml only changes dockerImageTag (4.4.7 → 4.4.8). No required field additions, no property removals, no stream changes. No rolloutConfiguration in metadata — progressive rollout is not enabled for this connector.
Forwards Compatibility PASS Warning (elevates Risk Level) No state/cursor/pagination keywords in functional diff hunks. The changes modify the LSN validation path (initialization-time), but do not alter state format, cursor format, or pagination logic. The new configuredCatalog parameter has a null default, so rollback to the prior version would skip the new validation gracefully.
Behavioral Changes PASS Warning (elevates Risk Level) No rate-limit, retry, backoff, timeout, sleep, error_handler, or resource keywords found in diff hunks. The statement keyword matches are false positives (SQL statement objects, not behavioral keywords).
Out-of-Scope Changes PASS Skip All 5 changed files are within airbyte-integrations/connectors/source-mssql/ or docs/integrations/sources/mssql.md. All in-scope.
CI Checks PASS Yes All core CI checks pass: Connector CI Checks Summary ✅, Test source-mssql Connector ✅ (116/116 tests), Lint source-mssql Connector ✅, Build and Verify Artifacts (source-mssql) ✅, Format Check ✅. The 2 failures (Connector Active Progressive Rollout Checks Summary and source-mssql Progressive Rollout Gate) are non-core progressive rollout governance checks, not CI test/lint/build failures.
Live / E2E Tests PASS Yes Validation required: PR is a bug fix (title "fix:", linked to oncall 11451), and modifies sync behavior code (MsSqlServerDebeziumOperations.kt — CDC state deserialization). Evidence: source-mssql Pre-Release Checks CI check-run passed ✅. Pre-release published successfully (airbyte/source-mssql:4.4.7-preview.fc21138). MCP verification unavailable (Cloud SQL Proxy not running), falling back to CI check-run evidence.

Spec Comparison:

  • Master dockerImageTag: 4.4.7
  • PR dockerImageTag: 4.4.8
  • No spec file changes. No required array changes. No property removals.
  • Result: Only version bump — PASS (no breaking changes)
📚 Evidence Consulted

Evidence

  • Changed files: 5 files
    • airbyte-integrations/connectors/source-mssql/gradle.properties (CDK 1.1.3 → 1.1.7)
    • airbyte-integrations/connectors/source-mssql/metadata.yaml (4.4.7 → 4.4.8)
    • airbyte-integrations/connectors/source-mssql/src/main/kotlin/.../MsSqlServerDebeziumOperations.kt (+135/-17)
    • airbyte-integrations/connectors/source-mssql/src/test/kotlin/.../MsSqlServerDatatypeIntegrationTest.kt (+5)
    • docs/integrations/sources/mssql.md (+1 changelog row)
  • CI checks: 40 passed, 2 failed (progressive rollout gate only), 0 pending
    • Core: Connector CI Checks Summary ✅, Test source-mssql Connector ✅, Lint source-mssql Connector ✅, Build and Verify Artifacts
    • Pre-release: source-mssql Pre-Release Checks
    • Non-core failures: Connector Active Progressive Rollout Checks Summary ❌, source-mssql Progressive Rollout Gate ❌ (governance, not code quality)
  • PR labels: connectors/source/mssql, dependencies-change
  • PR description: Present (1519 chars raw, meaningful content describing the fix)
  • Existing bot reviews: None for this HEAD SHA
  • Pre-release: airbyte/source-mssql:4.4.7-preview.fc21138 published successfully
  • Linked issue: airbytehq/oncall#11451

@devin-ai-integration
Copy link
Copy Markdown
Contributor

AI PR Review Report

Review Action: APPROVE
Risk Level: 3 — Logic change to CDC LSN validation in source-mssql; adds pre-sync permission check and scopes existing MIN LSN query to configured streams.

Gate Status
PR Hygiene PASS
Code Hygiene WARNING
Test Coverage PASS
Code Security PASS
Per-Record Performance PASS
Breaking Dependencies WARNING
Backwards Compatibility PASS
Forwards Compatibility PASS
Behavioral Changes PASS
Out-of-Scope Changes PASS
CI Checks PASS
Live / E2E Tests PASS

📋 PR Details

Connector & PR Info

Connector(s): source-mssql
PR: #77998
HEAD SHA: 258e12bd868d66281892886838625e619f0af91b
Session: https://app.devin.ai/sessions/c97dfdc4c3b944788e768fcd0274b882

Risk Level

Level: 3 — Medium (typical connector change)
Rationale: Logic change to CDC LSN validation in MsSqlServerDebeziumOperations.kt; adds a new pre-sync permission check (getAccessibleCaptureInstances + validateConfiguredStreamsAreAccessible) and scopes the existing validateLsnStillAvailable MIN LSN query to only configured streams. Also bumps CDK 1.1.3 → 1.1.7. Changes are confined to a single connector and are well-tested by CI (116/116 tests passing).

Risk Level is reported for downstream consumers (e.g. auto-merge policy, reviewer routing). It does not change the review action — APPROVE here means "no blocking objection," not "safe to merge unattended."

Review Action Details

APPROVE - All enforced gates pass. This does NOT authorize auto-merge; the merge decision remains with humans or a separate policy.

Note: This report returns APPROVE whenever all enforced gates PASS, regardless of what the PR changes. APPROVE is a quality signal, not a merge authorization; any auto-merge behavior is governed by separate policy reading decision and risk_level from the metrics marker.

🔍 Gate Evaluation Details

Gate-by-Gate Analysis

Gate Status Enforced? Details
PR Hygiene PASS Yes Description present (1519 raw chars, 1635 visible chars). Changelog updated in docs/integrations/sources/mssql.md. No unresolved human reviewer comments.
Code Hygiene WARNING WARNING Source code modified (MsSqlServerDebeziumOperations.kt) with test file also modified (MsSqlServerDatatypeIntegrationTest.kt). However, the test change adds only CDC scan setup, not a dedicated unit test for the new validation logic. Note: Live validation evidence present (Pre-Release Checks pass) — risk is mitigated but unit test coverage still recommended.
Test Coverage PASS Yes Behavioral change detected (title contains "fix", linked to oncall issue 11451). Test file MsSqlServerDatatypeIntegrationTest.kt is modified with new test content (adds EXEC sys.sp_cdc_scan setup for CDC validation). CI integration tests pass (116/116).
Code Security PASS Yes No auth/credential file patterns matched. metadata.yaml diff only contains dockerImageTag version bump — no security keywords in diff hunks.
Per-Record Performance PASS WARNING New methods (getAccessibleCaptureInstances, validateConfiguredStreamsAreAccessible) run during deserializeState() — once per sync initialization, not per-record. No per-record processing code paths affected.
Breaking Dependencies WARNING WARNING CDK version bumped from 1.1.3 → 1.1.7 in gradle.properties. This is a minor version bump (patch level change). Gradle dependency diff was detected by CI but the report job concluded as neutral (no alarming changes flagged). Recommend verifying CDK 1.1.7 release notes for behavioral changes.
Backwards Compatibility PASS Warning (elevates Risk Level) No spec changes. metadata.yaml only changes dockerImageTag (4.4.7 → 4.4.8). No required field additions, no property removals, no stream changes. No rolloutConfiguration in metadata — progressive rollout is not enabled for this connector.
Forwards Compatibility PASS Warning (elevates Risk Level) No state/cursor/pagination keywords in functional diff hunks. The changes modify the LSN validation path (initialization-time), but do not alter state format, cursor format, or pagination logic. The new configuredCatalog parameter has a null default, so rollback to the prior version would skip the new validation gracefully.
Behavioral Changes PASS Warning (elevates Risk Level) No rate-limit, retry, backoff, timeout, sleep, error_handler, or resource keywords found in diff hunks. The statement keyword matches are false positives (SQL statement objects, not behavioral keywords).
Out-of-Scope Changes PASS Skip All 5 changed files are within airbyte-integrations/connectors/source-mssql/ or docs/integrations/sources/mssql.md. All in-scope.
CI Checks PASS Yes All core CI checks pass: Connector CI Checks Summary ✅, Test source-mssql Connector ✅ (116/116 tests), Lint source-mssql Connector ✅, Build and Verify Artifacts (source-mssql) ✅, Format Check ✅. The 2 failures (Connector Active Progressive Rollout Checks Summary and source-mssql Progressive Rollout Gate) are non-core progressive rollout governance checks, not CI test/lint/build failures.
Live / E2E Tests PASS Yes Validation required: PR is a bug fix (title "fix:", linked to oncall 11451), and modifies sync behavior code (MsSqlServerDebeziumOperations.kt — CDC state deserialization). Evidence: source-mssql Pre-Release Checks CI check-run passed ✅. Pre-release published successfully (airbyte/source-mssql:4.4.7-preview.fc21138). MCP verification unavailable (Cloud SQL Proxy not running), falling back to CI check-run evidence.

Spec Comparison:

  • Master dockerImageTag: 4.4.7
  • PR dockerImageTag: 4.4.8
  • No spec file changes. No required array changes. No property removals.
  • Result: Only version bump — PASS (no breaking changes)
📚 Evidence Consulted

Evidence

  • Changed files: 5 files
    • airbyte-integrations/connectors/source-mssql/gradle.properties (CDK 1.1.3 → 1.1.7)
    • airbyte-integrations/connectors/source-mssql/metadata.yaml (4.4.7 → 4.4.8)
    • airbyte-integrations/connectors/source-mssql/src/main/kotlin/.../MsSqlServerDebeziumOperations.kt (+135/-17)
    • airbyte-integrations/connectors/source-mssql/src/test/kotlin/.../MsSqlServerDatatypeIntegrationTest.kt (+5)
    • docs/integrations/sources/mssql.md (+1 changelog row)
  • CI checks: 40 passed, 2 failed (progressive rollout gate only), 0 pending
    • Core: Connector CI Checks Summary ✅, Test source-mssql Connector ✅, Lint source-mssql Connector ✅, Build and Verify Artifacts
    • Pre-release: source-mssql Pre-Release Checks
    • Non-core failures: Connector Active Progressive Rollout Checks Summary ❌, source-mssql Progressive Rollout Gate ❌ (governance, not code quality)
  • PR labels: connectors/source/mssql, dependencies-change
  • PR description: Present (1519 chars raw, meaningful content describing the fix)
  • Existing bot reviews: None for this HEAD SHA
  • Pre-release: airbyte/source-mssql:4.4.7-preview.fc21138 published successfully
  • Linked issue: airbytehq/oncall#11451

Devin session

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants