Skip to content

replace _ALLOWED_EXTRA whitelist with _RESERVED_EXTRA_FIELDS blocklist#33

Merged
wangxingjun778 merged 1 commit into
mainfrom
fix/hub_compat_issue
Jul 2, 2026
Merged

replace _ALLOWED_EXTRA whitelist with _RESERVED_EXTRA_FIELDS blocklist#33
wangxingjun778 merged 1 commit into
mainfrom
fix/hub_compat_issue

Conversation

@wangxingjun778

@wangxingjun778 wangxingjun778 commented Jul 2, 2026

Copy link
Copy Markdown
Member
  • Removed _ALLOWED_EXTRA whitelist that blocked unknown extra fields in create_repo
  • Added _RESERVED_EXTRA_FIELDS blocklist (Path, Owner, Name) to only reject fields controlled by method params
  • All other extra fields now pass through to server (server handles validation)
  • Eliminates need to update SDK when server adds new fields (e.g. aigc_model)

…S blocklist

- Removed _ALLOWED_EXTRA whitelist that blocked unknown extra fields in create_repo
- Added _RESERVED_EXTRA_FIELDS blocklist (Path, Owner, Name) to only reject fields controlled by method params
- All other extra fields now pass through to server (server handles validation)
- Eliminates need to update SDK when server adds new fields (e.g. aigc_model)
@wangxingjun778 wangxingjun778 merged commit 06ea8ba into main Jul 2, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request transitions the create_repo method in src/modelscope_hub/api.py from a whitelist-based approach (_ALLOWED_EXTRA) to a blocklist-based approach (_RESERVED_EXTRA_FIELDS) for handling extra keyword arguments. This allows passing arbitrary extra fields while protecting reserved fields like Path, Owner, and Name. The review feedback correctly identifies a potential vulnerability where the OpenAPI creation path still accepts unfiltered **extra arguments, which could allow overriding critical identity fields like owner, repo_name, and skill_name. It is recommended to implement a corresponding blocklist for the OpenAPI path to prevent silent conflicts.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread src/modelscope_hub/api.py
Comment on lines +95 to +96
_RESERVED_EXTRA_FIELDS: frozenset[str] = frozenset(
{"Path", "Owner", "Name"}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The blocklist _RESERVED_EXTRA_FIELDS only covers the legacy API's PascalCase fields (Path, Owner, Name). However, the OpenAPI creation path (for Studios and Skills) on lines 637-664 also accepts **extra and updates the payload directly (payload.update(extra)) without any blocklist filtering. This allows users to silently override critical identity fields like owner, repo_name, and skill_name via extra kwargs, which can lead to unexpected behavior or repository creation under incorrect namespaces.\n\nConsider defining a corresponding blocklist for OpenAPI fields (e.g., _OPENAPI_RESERVED_FIELDS = {"owner", "repo_name", "skill_name"}) and filtering them out of extra in the OpenAPI block before updating the payload.

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.

1 participant