codeant-ai-for-open-source[bot] commented on code in PR #38647:
URL: https://github.com/apache/superset/pull/38647#discussion_r2969394455
##########
superset/views/datasource/views.py:
##########
@@ -182,6 +184,14 @@ def external_metadata_by_name(self, **kwargs: Any) ->
FlaskResponse:
.filter_by(database_name=params["database_name"])
.one()
)
+ security_manager.raise_for_access(
+ database=database,
+ table=Table(
+ params["table_name"],
+ params["schema_name"],
+ params.get("catalog_name"),
+ ),
+ )
external_metadata = get_physical_table_metadata(
database=database,
table=Table(params["table_name"], params["schema_name"]),
Review Comment:
**Suggestion:** Access is validated against a `Table` that includes
`catalog_name`, but metadata is fetched using a different `Table` without
catalog, so the authorization target and fetched target can diverge. Build one
`Table` object and reuse it for both checks and metadata retrieval to avoid
returning/looking up the wrong table in multi-catalog databases. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ `/external_metadata_by_name` may fetch wrong catalog metadata.
- ❌ Dataset column sync can reflect unintended table structure.
- ⚠️ Multi-catalog authorization and retrieval become inconsistent.
```
</details>
```suggestion
table = Table(
params["table_name"],
params["schema_name"],
params.get("catalog_name"),
)
security_manager.raise_for_access(
database=database,
table=table,
)
external_metadata = get_physical_table_metadata(
database=database,
table=table,
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Call `GET /datasource/external_metadata_by_name/?q=...` (endpoint at
`superset/views/datasource/views.py:155`; widely exercised in integration
tests at
`tests/integration_tests/datasource_tests.py:187`, `:213`, `:234`, `:253`,
`:268`, `:286`,
`:300`).
2. Use parameters that take the "no saved datasource" branch
(`SqlaTable.get_datasource_by_name(...)` returns `None` at
`superset/views/datasource/views.py:169-181`), which is the branch
unit-tested in
`tests/unit_tests/views/datasource/views_test.py:186-216`.
3. In that branch, authorization checks `Table(table, schema, catalog)`
(`superset/views/datasource/views.py:187-194`) but metadata fetch uses
`Table(table,
schema)` without catalog (`superset/views/datasource/views.py:195-199`).
4. `get_physical_table_metadata()` calls `database.has_table(table)` and
`database.get_columns(table)` (`superset/connectors/sqla/utils.py:50-64`),
and those
database methods use `table.catalog` (`superset/models/core.py:1184-1193`,
`:1087-1091`),
so missing catalog can inspect a different/default catalog than the one
authorized.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/views/datasource/views.py
**Line:** 187:197
**Comment:**
*Logic Error: Access is validated against a `Table` that includes
`catalog_name`, but metadata is fetched using a different `Table` without
catalog, so the authorization target and fetched target can diverge. Build one
`Table` object and reuse it for both checks and metadata retrieval to avoid
returning/looking up the wrong table in multi-catalog databases.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38647&comment_hash=ce79452cdfd7dfb88046939e2b8f93000f958f03c0fc5a062f4df5cad5b28db8&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38647&comment_hash=ce79452cdfd7dfb88046939e2b8f93000f958f03c0fc5a062f4df5cad5b28db8&reaction=dislike'>👎</a>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
