feat(REQ-925): clonedInformationSchema mirror-schema support for restricted-access SDR - #176
feat(REQ-925): clonedInformationSchema mirror-schema support for restricted-access SDR#176vaibhavatlan wants to merge 3 commits into
Conversation
…cess SDR Re-implements the internal-ref feature on the current main (the original branch predated a handler/client refactor + a history rewrite, so it could not be rebased mechanically). Sourced from the preserved branch and re-fitted to the current structure + SDK 3.15.1; unit suite green (218 passed). Lets customers route every INFORMATION_SCHEMA query through a customer-managed mirror schema (e.g. atlan_meta), solving the restricted-access blocker where granting SELECT on INFORMATION_SCHEMA implicitly grants SELECT on every underlying user table. Backward compatible: with no control-config, rendered SQL is byte-identical to today. - app/utils.py: resolve_information_schema / resolve_excluded_schemas / extract_control_config, with hard identifier validation (SQL-injection guard). - app/sql/*: parametrize information_schema + system-schema exclusion list. - app/mysql.py: _prepare_sql + build_task_input + _init_sql_client overrides thread control-config into each extract activity. - app/handler.py: per-request resolution for preflight + fetch_metadata. - contract/app.pkl + regenerated app/generated/*: Advanced Config UI fields. - scripts/setup_information_schema_mirror.sql: one-time DBA setup. - tests: unit (utils, handler, app) + integration (mirror schema). Co-Authored-By: AtMrun <121743130+AtMrun@users.noreply.github.com> Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
👋 Mothership PR Reviewer is ready. Comment |
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
Tests Summary
Overall: ✅ All passed — view run |
🛡️ Security Gate PassedTrivy: 0 | Total: 0 CRITICAL/HIGH | All allowlisted ✅ Run: 27271846045 | Wed, 10 Jun 2026 11:09:31 GMT |
There was a problem hiding this comment.
🔍 Mothership PR Reviewer — Approve
| Score | Decision | Risk | Blocking | Critical | High |
|---|---|---|---|---|---|
| 74/100 | ✅ Approve | 🔺 Elevated | 0 | 0 | 0 |
Elevated risk — critical/high findings or weak safeguards noted below. Worth a closer look before merging.
This PR implements the clonedInformationSchema mirror-schema feature cleanly. SQL injection is well-guarded by _IDENTIFIER_RE. The Temporal worker-boundary problem (activity vs workflow context) is correctly solved by carrying control-config on the task input, with end-to-end verification in source-side-behavior-verification.md. Test coverage is excellent: unit, integration with testcontainers, and privilege isolation. The handler/activity precedence divergence is intentional and documented. One INFO: allow_unbounded_fields=True is a dead SDK keyword (removed in SDK 3.7.0); the real gu
🔎 Findings (1)
ℹ️ INFO allow_unbounded_fields=True dead SDK keyword; comment misidentifies the actual guard
- Pattern:
dead-sdk-keyword-misleading-comment - Category: defensive
- Confidence: 90%
- Location:
app/mysql.py:150 - Reachability: internal-only — class definition, not an entry point
- By-design check: no prior discussion; code is correct via ConfigDict(extra='allow'); dead keyword is documented with type: ignore[call-arg]
- Verifier: downgraded — Downgraded LOW->INFO: zero runtime impact; 218 tests pass; ConfigDict(extra='allow') is the real guard; type: ignore[call-arg] signals author awareness.
Evidence
class MySQLExtractionInput(ExtractionInput, allow_unbounded_fields=True): # type: ignore[call-arg]
Suggested fix: Remove allow_unbounded_fields=True (ignored since SDK 3.7.0); update docstring to credit ConfigDict(extra='allow') as the actual safety guard. Also applies to MySQLExtractionTaskInput at line 172.
💪 Strengths (3)
- Excellent test coverage: unit, integration via testcontainers, and privilege-isolation class validating the security premise end-to-end.
- SQL injection guard
_IDENTIFIER_REcorrectly enforces MySQL identifier rules: rejects hyphens, semicolons, backticks, and digit-leading names. - Temporal worker-boundary problem is documented in code, docstrings, and source-side-behavior-verification.md with a conclusive drop-probe.
📊 Score Breakdown
| Dimension | Score |
|---|---|
| Test quality | 95% |
| Defensive coding | 88% |
| Open findings (impact) | 97% |
📄 Review Metadata
commit: 7f82158, mode: standard, reviewer version: rover-v2, generated: 2026-06-10T11:20:52.850691Z, dispatch: e66405bf-1f2a-4e10-894e-fb0a0623d580
ea213f1 to
09a0659
Compare
7f82158 to
d323ce2
Compare
REQ-925 Support custom INFORMATION_SCHEMA mirror schema for MySQL/MariaDB connector (SDR)
ContextBandwidth's infosec team is blocking SDR adoption for their RDBMS connectors (MariaDB 10.11, SQLDB) because the MySQL connector requires Slack thread: https://atlanhq.slack.com/archives/CSC46LFAB/p1763583889911889 Customer ProposalBandwidth proposed a custom curated view workaround: create views wrapping the Current StateThe MySQL connector ( Precedent: Redshift Cloned SchemaThe Redshift connector ( What the MySQL Connector QueriesThe connector queries these
Implementation Scope (Estimated)Following the Redshift pattern:
RequestorBandwidth (via David Fisher, Prerna Sharma, Akshay Dhingra) |
🛡️ Security Gate PassedTrivy: 0 | Total: 0 CRITICAL/HIGH | All allowlisted ✅ Run: 27290743591 | Wed, 10 Jun 2026 16:38:45 GMT |
d323ce2 to
10457d3
Compare
🛡️ Security Gate PassedTrivy: 0 | Total: 0 CRITICAL/HIGH | All allowlisted ✅ Run: 27291951531 | Wed, 10 Jun 2026 16:58:29 GMT |
10457d3 to
728783c
Compare
🛡️ Security Gate PassedTrivy: 0 | Total: 0 CRITICAL/HIGH | All allowlisted ✅ Run: 27292857371 | Wed, 10 Jun 2026 17:14:19 GMT |
…nformation-schema
🛡️ Security Gate PassedTrivy: 0 | Total: 0 CRITICAL/HIGH | All allowlisted ✅ Run: 27294433961 | Wed, 10 Jun 2026 17:41:19 GMT |
- Close unterminated string literal in test_mirror_schema.py xfail reason - Apply ruff-format line wrap to warning log call in mysql.py Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
🛡️ Security Gate PassedTrivy: 0 | Total: 0 CRITICAL/HIGH | All allowlisted ✅ Run: 27295183783 | Wed, 10 Jun 2026 17:54:17 GMT |
Summary
Re-raises the REQ-925 feature (originally closed PR #121) on the current
main. The original branch could not be rebased —mainwas refactored since it branched (app/clients.py→app/client.py,app/handlers/mysql.py→app/handler.py) and the repo history was rewritten — so this is a clean re-implementation sourced from the preserved branch and re-fitted to the current structure + SDK 3.15.1.Adds a Custom Control Config option that routes every
INFORMATION_SCHEMAquery through a customer-managed mirror schema (e.g.atlan_meta). Solves the restricted-access SDR blocker: in MySQL, grantingSELECTonINFORMATION_SCHEMAimplicitly grantsSELECTon every underlying user table, which violates some customers' security policies.Backward compatible: with no control-config supplied, rendered SQL is byte-identical to today.
How it works
scripts/setup_information_schema_mirror.sqlonce — creates a mirror schema with passthrough views overINFORMATION_SCHEMAand a reader user withSELECTonly on the mirror.{ "clonedInformationSchema": "atlan_meta" }.atlan_meta.*instead ofinformation_schema.*.Changes
app/utils.py(new) —resolve_information_schema/resolve_excluded_schemas/extract_control_config; hard identifier validation as a SQL-injection guard.app/sql/*— parametrizeinformation_schemaand the system-schema exclusion list.app/mysql.py—_prepare_sql/build_task_input/_init_sql_clientoverrides thread control-config into each extract activity.app/handler.py— per-request resolution for preflight + fetch_metadata.contract/app.pkl+ regeneratedapp/generated/*— Advanced Config UI fields.scripts/setup_information_schema_mirror.sql(new) — one-time DBA setup.test_utils,test_handler,test_mysql_app) + integration (test_mirror_schema).Notes for reviewers
ae-workflow.shDAG-contract edits +test_dag_contracts.pyand atenant-deploy.yaml/app/templates/workflow.json— those were not part of this feature (and touched files since pruned/scrubbed), so they're intentionally omitted.Co-Authored-By.Test plan
pytest tests/unit— 218 passed locallypoe generateleaves the tree cleanruff check(repo rule set) passes🤖 Generated with Claude Code