codeant-ai-for-open-source[bot] commented on code in PR #38174:
URL: https://github.com/apache/superset/pull/38174#discussion_r2844315313
##########
superset/databases/api.py:
##########
@@ -1079,15 +1079,23 @@ def table_metadata(self, pk: int) -> FlaskResponse:
parameters = QualifiedTableSchema().load(request.args)
except ValidationError as ex:
raise InvalidPayloadSchemaError(ex) from ex
-
- table = Table(parameters["name"], parameters["schema"],
parameters["catalog"])
+ table_name = str(parameters["name"])
+ table = Table(table_name, parameters["schema"], parameters["catalog"])
+ is_partitioned_table, partition_fields =
DatabaseDAO.is_odps_partitioned_table(
+ database, table_name
+ )
try:
security_manager.raise_for_access(database=database, table=table)
except SupersetSecurityException as ex:
# instead of raising 403, raise 404 to hide table existence
raise TableNotFoundException("No such table") from ex
Review Comment:
**Suggestion:** The ODPS partition detection is performed before checking
the user's table access, meaning an unauthorized caller can still trigger ODPS
API calls (and potentially learn about table existence or cause backend errors)
before a permission check is enforced; move the partition lookup after the
security check so that only authorized users can hit the ODPS backend for
metadata. [security]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Unauthorized users can trigger ODPS table metadata lookups.
- ⚠️ Table existence may leak via ODPS error behavior or timing.
- ⚠️ Extra ODPS calls for denied requests increase backend load.
```
</details>
```suggestion
try:
security_manager.raise_for_access(database=database, table=table)
except SupersetSecurityException as ex:
# instead of raising 403, raise 404 to hide table existence
raise TableNotFoundException("No such table") from ex
is_partitioned_table, partition_fields =
DatabaseDAO.is_odps_partitioned_table(
database, table_name
)
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Configure a Superset Database record pointing to an ODPS (MaxCompute)
instance so that
`Database.backend == "odps"` (used in
`DatabaseDAO.is_odps_partitioned_table` at
`superset/daos/database.py:245-257`).
2. Start Superset and authenticate as a user that has API access to the
Database REST API
but lacks table-level access for a specific ODPS table (table access is
enforced via
`security_manager.raise_for_access(database=database, table=table)` at
`superset/databases/api.py:1087-1091` and
`SupersetSecurityManager.can_access_table` at
`superset/security/manager.py:906-920`).
3. Call the table metadata endpoint implemented by
`DatabaseRestApi.table_metadata` in
`superset/databases/api.py:1023-1100` with `GET
/<int:pk>/table_metadata/?name=<table_name>&schema=<schema>&catalog=<catalog>`
where `pk`
is the ODPS database id and `<table_name>` is any table name (including ones
the user is
not allowed to access).
4. Observe that before the permission check runs, `table_metadata` invokes
`DatabaseDAO.is_odps_partitioned_table(database, table_name)` at
`superset/databases/api.py:1084-1085`, which in turn creates an ODPS client
and calls
`odps_client.get_table(table_name)` at `superset/daos/database.py:278-280`,
hitting the
external ODPS API with Superset's credentials; only after this remote call
does
`security_manager.raise_for_access` run and potentially raise
`TableNotFoundException` for
unauthorized users, meaning unauthorized callers can still trigger ODPS
metadata lookups
and, if `get_table` raises (e.g. non-existent table), cause backend errors
before access
is denied.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/databases/api.py
**Line:** 1084:1091
**Comment:**
*Security: The ODPS partition detection is performed before checking
the user's table access, meaning an unauthorized caller can still trigger ODPS
API calls (and potentially learn about table existence or cause backend errors)
before a permission check is enforced; move the partition lookup after the
security check so that only authorized users can hit the ODPS backend for
metadata.
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%2F38174&comment_hash=c8042e4928a1e9b1d0db71979a45dac9a8aec0cde40817fd56545208bd7907a5&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38174&comment_hash=c8042e4928a1e9b1d0db71979a45dac9a8aec0cde40817fd56545208bd7907a5&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]
