feat(scripts/resources): align list_scripts visibility with list_resources
69a9a48 把 data resources 的可见性从 list 恒真改成
workspace-wide + visibility 过滤 + admin 短路,但 scripts 端
没动。两端不对称,导致:
* 非 admin 调 list_scripts 走 user_relative_path →
workspace/{当前用户ID}/...,永远拿不到别人的脚本
* count_scripts 同样 user-scoped,dashboard "全部脚本"
只统计自己
* list_resources 响应没带 owner_display_name,data-only
owner 的目录名回退到 userId.slice(-6),显示不友好
修复:
* list_scripts / count_scripts 改 workspace-wide(新建
_build_list_scripts_workspace_descendant_prefix helper;
旧 _build_list_scripts_descendant_prefix 保留标 deprecated
避免破坏其它调用方);非 admin 追加
or_(owner_user_id = me, visibility in {workspace, public})
与 list_resources 完全对称;admin 短路
* resource_payload 加 owner_display_name 字段(与
script_payload 对称);list_resources SELECT 加
Users.display_name + outerjoin
* 前端 ScriptExplorer displayName 回退链:scripts 的
owner_display_name → data resources 的 owner_display_name
→ 本人 user.display_name → userId.slice(-6) 占位
;ResourceItem 类型同步加 owner_display_name?: string|null
存储物理布局仍是 workspace/{user_id}/...,仅读取侧
listing/count 跨 owner。
This commit is contained in:
@@ -12,7 +12,7 @@ from datetime import UTC, datetime
|
|||||||
from pathlib import Path, PurePosixPath
|
from pathlib import Path, PurePosixPath
|
||||||
from typing import Any
|
from typing import Any
|
||||||
|
|
||||||
from common.db.models import DataResources, StorageObjects
|
from common.db.models import DataResources, StorageObjects, Users
|
||||||
from common.ids import new_ulid
|
from common.ids import new_ulid
|
||||||
from common.storage import workspaces_root
|
from common.storage import workspaces_root
|
||||||
from common.storage.schemas import (
|
from common.storage.schemas import (
|
||||||
@@ -96,6 +96,7 @@ def resource_directory(
|
|||||||
def resource_payload(
|
def resource_payload(
|
||||||
resource: DataResources,
|
resource: DataResources,
|
||||||
storage_object: StorageObjects,
|
storage_object: StorageObjects,
|
||||||
|
owner_display_name: str | None = None,
|
||||||
) -> dict[str, Any]:
|
) -> dict[str, Any]:
|
||||||
# ``object_key`` now follows ``{ws_id}/{user_id}/{target_path}/{file_name}``
|
# ``object_key`` now follows ``{ws_id}/{user_id}/{target_path}/{file_name}``
|
||||||
# (target_path may be empty). Legacy objects still live under
|
# (target_path may be empty). Legacy objects still live under
|
||||||
@@ -128,6 +129,7 @@ def resource_payload(
|
|||||||
"workspace_id": resource.workspace_id,
|
"workspace_id": resource.workspace_id,
|
||||||
"storage_object_id": resource.storage_object_id,
|
"storage_object_id": resource.storage_object_id,
|
||||||
"owner_user_id": resource.owner_user_id,
|
"owner_user_id": resource.owner_user_id,
|
||||||
|
"owner_display_name": owner_display_name,
|
||||||
"resource_name": resource.resource_name,
|
"resource_name": resource.resource_name,
|
||||||
"description": resource.description,
|
"description": resource.description,
|
||||||
"visibility": resource.visibility,
|
"visibility": resource.visibility,
|
||||||
@@ -387,12 +389,13 @@ async def list_resources(
|
|||||||
keyword: str | None = Query(default=None, max_length=100),
|
keyword: str | None = Query(default=None, max_length=100),
|
||||||
) -> dict[str, Any]:
|
) -> dict[str, Any]:
|
||||||
statement = (
|
statement = (
|
||||||
select(DataResources, StorageObjects)
|
select(DataResources, StorageObjects, Users.display_name)
|
||||||
.join(
|
.join(
|
||||||
StorageObjects,
|
StorageObjects,
|
||||||
StorageObjects.storage_object_id
|
StorageObjects.storage_object_id
|
||||||
== DataResources.storage_object_id,
|
== DataResources.storage_object_id,
|
||||||
)
|
)
|
||||||
|
.outerjoin(Users, Users.user_id == DataResources.owner_user_id)
|
||||||
.where(
|
.where(
|
||||||
DataResources.workspace_id == context.workspace.workspace_id,
|
DataResources.workspace_id == context.workspace.workspace_id,
|
||||||
DataResources.status == "active",
|
DataResources.status == "active",
|
||||||
@@ -445,8 +448,10 @@ async def list_resources(
|
|||||||
return {
|
return {
|
||||||
"request_id": context.request_id,
|
"request_id": context.request_id,
|
||||||
"data": [
|
"data": [
|
||||||
resource_payload(resource, storage_object)
|
resource_payload(
|
||||||
for resource, storage_object in rows
|
resource, storage_object, owner_display_name=owner_display_name
|
||||||
|
)
|
||||||
|
for resource, storage_object, owner_display_name in rows
|
||||||
],
|
],
|
||||||
"meta": {"count": len(rows)},
|
"meta": {"count": len(rows)},
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -36,7 +36,7 @@ from fastapi import (
|
|||||||
status,
|
status,
|
||||||
)
|
)
|
||||||
from loguru import logger
|
from loguru import logger
|
||||||
from sqlalchemy import func, select
|
from sqlalchemy import func, or_, select
|
||||||
from sqlalchemy.ext.asyncio import AsyncSession
|
from sqlalchemy.ext.asyncio import AsyncSession
|
||||||
|
|
||||||
from backend.dependencies import (
|
from backend.dependencies import (
|
||||||
@@ -129,11 +129,21 @@ def _escape_like_pattern(value: str) -> str:
|
|||||||
return value.replace("\\", "\\\\").replace("%", "\\%").replace("_", "\\_")
|
return value.replace("\\", "\\\\").replace("%", "\\%").replace("_", "\\_")
|
||||||
|
|
||||||
|
|
||||||
|
# @deprecated — legacy user-scoped prefix helper. list_scripts/count_scripts
|
||||||
|
# are now workspace-wide via _build_list_scripts_workspace_descendant_prefix;
|
||||||
|
# this helper is kept (with its original user-scoped semantics) so existing
|
||||||
|
# callers/tests that still reference it do not break.
|
||||||
def _build_list_scripts_descendant_prefix(
|
def _build_list_scripts_descendant_prefix(
|
||||||
context: RequestContext, parent_path: str
|
context: RequestContext, parent_path: str
|
||||||
) -> str:
|
) -> str:
|
||||||
"""Return the escaped materialized-path prefix for direct children of
|
"""Return the escaped materialized-path prefix for direct children of
|
||||||
``parent_path``.
|
``parent_path`` within the **requester's own subtree**.
|
||||||
|
|
||||||
|
.. note::
|
||||||
|
Legacy user-scoped helper. list_scripts / count_scripts are now
|
||||||
|
workspace-wide — use
|
||||||
|
:func:`_build_list_scripts_workspace_descendant_prefix` instead
|
||||||
|
(visibility filtering handles non-admin scoping in the SQL).
|
||||||
|
|
||||||
The endpoint appends ``LIKE '<prefix>/%' AND NOT LIKE '<prefix>/%/%'``
|
The endpoint appends ``LIKE '<prefix>/%' AND NOT LIKE '<prefix>/%/%'``
|
||||||
against ``storage_objects.relative_path`` so only scripts whose parent
|
against ``storage_objects.relative_path`` so only scripts whose parent
|
||||||
@@ -156,6 +166,29 @@ def _build_list_scripts_descendant_prefix(
|
|||||||
return f"{_escape_like_pattern(target_prefix)}/"
|
return f"{_escape_like_pattern(target_prefix)}/"
|
||||||
|
|
||||||
|
|
||||||
|
def _build_list_scripts_workspace_descendant_prefix(parent_path: str) -> str:
|
||||||
|
"""Return the escaped materialized-path prefix for direct children of
|
||||||
|
``parent_path`` across **all owners** in the workspace.
|
||||||
|
|
||||||
|
Storage is still physically laid out as ``workspace/{user_id}/...``, but
|
||||||
|
listing is workspace-wide: the prefix starts at ``workspace/`` (no
|
||||||
|
embedded user_id) so the endpoint's ``LIKE '<prefix>/%'`` walks every
|
||||||
|
owner's subtree. Non-admin scoping is handled separately in the SQL via
|
||||||
|
``owner_user_id = me OR visibility IN (workspace, public)``.
|
||||||
|
|
||||||
|
Empty ``parent_path`` returns ``"workspace/"`` (cross-owner root).
|
||||||
|
The prefix is run through ``_escape_like_pattern`` so folder names
|
||||||
|
containing ``_`` / ``%`` do not act as wildcards. The trailing ``/``
|
||||||
|
is appended AFTER escaping so it remains a literal slash.
|
||||||
|
"""
|
||||||
|
normalized_parent = normalize_user_path(parent_path)
|
||||||
|
if normalized_parent:
|
||||||
|
target_prefix = f"workspace/{normalized_parent}"
|
||||||
|
else:
|
||||||
|
target_prefix = "workspace"
|
||||||
|
return f"{_escape_like_pattern(target_prefix)}/"
|
||||||
|
|
||||||
|
|
||||||
def safe_script_name(value: str, script_type: str) -> str:
|
def safe_script_name(value: str, script_type: str) -> str:
|
||||||
name = value.replace("\\", "/").rsplit("/", 1)[-1].strip()
|
name = value.replace("\\", "/").rsplit("/", 1)[-1].strip()
|
||||||
if not name or name in {".", ".."} or any(ord(char) < 32 for char in name):
|
if not name or name in {".", ".."} or any(ord(char) < 32 for char in name):
|
||||||
@@ -1106,7 +1139,12 @@ async def list_scripts(
|
|||||||
context: RequestContext = Depends(request_context),
|
context: RequestContext = Depends(request_context),
|
||||||
session: AsyncSession = Depends(database_session),
|
session: AsyncSession = Depends(database_session),
|
||||||
) -> dict[str, Any]:
|
) -> dict[str, Any]:
|
||||||
descendant_prefix = _build_list_scripts_descendant_prefix(context, parent_path)
|
# Workspace-wide listing: storage is physically laid out as
|
||||||
|
# ``workspace/{user_id}/...``, but the prefix starts at ``workspace/``
|
||||||
|
# (no embedded user_id) so the LIKE walks every owner's subtree.
|
||||||
|
# Non-admin scoping is applied below via visibility, matching
|
||||||
|
# list_resources (69a9a48).
|
||||||
|
descendant_prefix = _build_list_scripts_workspace_descendant_prefix(parent_path)
|
||||||
|
|
||||||
statement = (
|
statement = (
|
||||||
select(Scripts, StorageObjects, Users.display_name)
|
select(Scripts, StorageObjects, Users.display_name)
|
||||||
@@ -1123,6 +1161,16 @@ async def list_scripts(
|
|||||||
)
|
)
|
||||||
.order_by(Scripts.updated_at.desc())
|
.order_by(Scripts.updated_at.desc())
|
||||||
)
|
)
|
||||||
|
# 只返回 owner 自己的脚本(含 private),或 visibility 为
|
||||||
|
# workspace/public 的其他成员脚本;A 的 private 脚本对非 owner 不可见。
|
||||||
|
# admin 跳过过滤,全部可见。
|
||||||
|
if not context.is_admin:
|
||||||
|
statement = statement.where(
|
||||||
|
or_(
|
||||||
|
Scripts.owner_user_id == context.user.user_id,
|
||||||
|
Scripts.visibility.in_(["workspace", "public"]),
|
||||||
|
)
|
||||||
|
)
|
||||||
rows = (await session.execute(statement)).all()
|
rows = (await session.execute(statement)).all()
|
||||||
return {
|
return {
|
||||||
"request_id": context.request_id,
|
"request_id": context.request_id,
|
||||||
@@ -1138,25 +1186,25 @@ async def list_scripts(
|
|||||||
# 避免被 listScripts 的懒加载语义污染。该路由必须在 /scripts/{script_id} 之前声明
|
# 避免被 listScripts 的懒加载语义污染。该路由必须在 /scripts/{script_id} 之前声明
|
||||||
# ——FastAPI 按声明顺序匹配,否则 `count` 会被当作 script_id 命中 get_script。
|
# ——FastAPI 按声明顺序匹配,否则 `count` 会被当作 script_id 命中 get_script。
|
||||||
#
|
#
|
||||||
# Scope: equals the UNION of list_scripts across every parent_path
|
# Scope: workspace-wide — the dashboard's "全部脚本" / "工作副本" counts
|
||||||
# within the user's subtree, NOT just the root-level call. We
|
# reflect the whole workspace, not the requester's own subtree. admin sees
|
||||||
# intentionally include deeper descendants because the dashboard's
|
# every active script; non-admin is narrowed by visibility
|
||||||
# "全部脚本" / "工作副本" counts reflect the whole workspace, not
|
# (owner_user_id = me OR visibility IN (workspace, public)), exactly like
|
||||||
# only the top level.
|
# list_scripts and list_resources.
|
||||||
#
|
#
|
||||||
# Implementation choices and why:
|
# Implementation choices and why:
|
||||||
# - INNER JOIN to StorageObjects so orphans (current_object_id has no
|
# - INNER JOIN to StorageObjects so orphans (current_object_id has no
|
||||||
# joinable row) are excluded.
|
# joinable row) are excluded.
|
||||||
# - User subtree filter so multi-member workspaces don't show counts
|
# - Workspace-wide ``LIKE 'workspace/%'`` prefix (no embedded user_id) so
|
||||||
# the requester can't see.
|
# counts span every owner's subtree.
|
||||||
# - No NOT-LIKE filter because the count wants descendants too.
|
# - No NOT-LIKE filter because the count wants descendants too.
|
||||||
@router.get("/api/v1/scripts/count")
|
@router.get("/api/v1/scripts/count")
|
||||||
async def count_scripts(
|
async def count_scripts(
|
||||||
context: RequestContext = Depends(request_context),
|
context: RequestContext = Depends(request_context),
|
||||||
session: AsyncSession = Depends(database_session),
|
session: AsyncSession = Depends(database_session),
|
||||||
) -> dict[str, Any]:
|
) -> dict[str, Any]:
|
||||||
user_subtree_prefix = f"{_escape_like_pattern(user_relative_path(context))}/%"
|
descendant_prefix = _build_list_scripts_workspace_descendant_prefix("")
|
||||||
total = await session.scalar(
|
base = (
|
||||||
select(func.count())
|
select(func.count())
|
||||||
.select_from(Scripts)
|
.select_from(Scripts)
|
||||||
.join(
|
.join(
|
||||||
@@ -1166,9 +1214,17 @@ async def count_scripts(
|
|||||||
.where(
|
.where(
|
||||||
Scripts.workspace_id == context.workspace.workspace_id,
|
Scripts.workspace_id == context.workspace.workspace_id,
|
||||||
Scripts.status == "active",
|
Scripts.status == "active",
|
||||||
StorageObjects.relative_path.like(user_subtree_prefix, escape="\\"),
|
StorageObjects.relative_path.like(f"{descendant_prefix}%", escape="\\"),
|
||||||
)
|
)
|
||||||
)
|
)
|
||||||
|
if not context.is_admin:
|
||||||
|
base = base.where(
|
||||||
|
or_(
|
||||||
|
Scripts.owner_user_id == context.user.user_id,
|
||||||
|
Scripts.visibility.in_(["workspace", "public"]),
|
||||||
|
)
|
||||||
|
)
|
||||||
|
total = await session.scalar(base)
|
||||||
return {
|
return {
|
||||||
"request_id": context.request_id,
|
"request_id": context.request_id,
|
||||||
"data": {"total": int(total or 0)},
|
"data": {"total": int(total or 0)},
|
||||||
|
|||||||
@@ -1,10 +1,11 @@
|
|||||||
"""Unit tests for GET /api/v1/scripts/count endpoint.
|
"""Unit tests for GET /api/v1/scripts/count endpoint.
|
||||||
|
|
||||||
Verifies the count endpoint returns the same scope as
|
Verifies the count endpoint matches the (workspace-wide) listing scope of
|
||||||
``list_scripts(parent_path="")``: workspace + active scripts whose
|
``list_scripts(parent_path="")``: workspace + active scripts across every
|
||||||
``StorageObjects.relative_path`` lives under the user's subtree. This
|
owner's ``StorageObjects.relative_path``, narrowed by visibility for
|
||||||
avoids under/over-reporting on the dashboard — the count is the size of
|
non-admin (admin short-circuits). This avoids under/over-reporting on the
|
||||||
the set list_scripts would return if it weren't lazy.
|
dashboard — the count is the size of the set list_scripts would return if
|
||||||
|
it weren't lazy.
|
||||||
"""
|
"""
|
||||||
|
|
||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
@@ -17,13 +18,23 @@ import pytest
|
|||||||
from backend.scripts import count_scripts
|
from backend.scripts import count_scripts
|
||||||
|
|
||||||
|
|
||||||
def _ctx(user_id: str = "U001", workspace_id: str = "W001") -> SimpleNamespace:
|
def _ctx(
|
||||||
|
user_id: str = "U001",
|
||||||
|
workspace_id: str = "W001",
|
||||||
|
*,
|
||||||
|
is_admin: bool = False,
|
||||||
|
is_system_admin: bool = False,
|
||||||
|
) -> SimpleNamespace:
|
||||||
return SimpleNamespace(
|
return SimpleNamespace(
|
||||||
request_id="test",
|
request_id="test",
|
||||||
user=SimpleNamespace(user_id=user_id),
|
user=SimpleNamespace(user_id=user_id),
|
||||||
workspace=SimpleNamespace(workspace_id=workspace_id),
|
workspace=SimpleNamespace(workspace_id=workspace_id),
|
||||||
role=SimpleNamespace(role_code="admin"),
|
role=SimpleNamespace(role_code="admin" if is_admin else "developer"),
|
||||||
is_system_admin=False,
|
is_system_admin=is_system_admin,
|
||||||
|
# ``count_scripts`` now consults ``context.is_admin`` directly
|
||||||
|
# (matching list_scripts / list_resources); SimpleNamespace needs
|
||||||
|
# it as a plain attribute.
|
||||||
|
is_admin=is_admin or is_system_admin,
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
@@ -57,10 +68,13 @@ async def test_count_scripts_returns_scalar_int() -> None:
|
|||||||
# JOIN to StorageObjects so orphaned scripts (no joinable row) are
|
# JOIN to StorageObjects so orphaned scripts (no joinable row) are
|
||||||
# excluded — matches list_scripts INNER JOIN behaviour.
|
# excluded — matches list_scripts INNER JOIN behaviour.
|
||||||
assert "inner join storage_objects" in sql
|
assert "inner join storage_objects" in sql
|
||||||
# Scope: workspace_id + active status + user subtree.
|
# Scope: workspace_id + active status + workspace-wide prefix.
|
||||||
assert "scripts.workspace_id" in sql
|
assert "scripts.workspace_id" in sql
|
||||||
assert "scripts.status" in sql
|
assert "scripts.status" in sql
|
||||||
assert "workspace/u001/%" in sql
|
assert "like 'workspace/%%'" in sql
|
||||||
|
# Non-admin (default) narrows by visibility.
|
||||||
|
assert "scripts.owner_user_id = 'u001'" in sql
|
||||||
|
assert "scripts.visibility in ('workspace', 'public')" in sql
|
||||||
|
|
||||||
|
|
||||||
async def test_count_scripts_handles_null_result() -> None:
|
async def test_count_scripts_handles_null_result() -> None:
|
||||||
@@ -72,10 +86,10 @@ async def test_count_scripts_handles_null_result() -> None:
|
|||||||
assert result["data"] == {"total": 0}
|
assert result["data"] == {"total": 0}
|
||||||
|
|
||||||
|
|
||||||
async def test_count_scripts_uses_user_specific_subtree() -> None:
|
async def test_count_scripts_workspace_wide_not_user_scoped() -> None:
|
||||||
"""Different users in the same workspace must see different totals —
|
"""The prefix is workspace-wide (``workspace/%`` — no embedded user_id),
|
||||||
each user's count is bounded by their own ``workspace/{user_id}/``
|
so different users count the same physical tree; the only per-user
|
||||||
subtree, NOT the whole workspace."""
|
difference is the non-admin visibility predicate (owner_user_id = me)."""
|
||||||
captured = []
|
captured = []
|
||||||
|
|
||||||
mock_session = MagicMock()
|
mock_session = MagicMock()
|
||||||
@@ -84,14 +98,41 @@ async def test_count_scripts_uses_user_specific_subtree() -> None:
|
|||||||
)
|
)
|
||||||
|
|
||||||
await count_scripts(context=_ctx(user_id="alice"), session=mock_session)
|
await count_scripts(context=_ctx(user_id="alice"), session=mock_session)
|
||||||
sql_alice = _compile(captured[-1])
|
sql_alice = _compile(captured[-1]).lower()
|
||||||
|
|
||||||
await count_scripts(context=_ctx(user_id="bob"), session=mock_session)
|
await count_scripts(context=_ctx(user_id="bob"), session=mock_session)
|
||||||
sql_bob = _compile(captured[-1])
|
sql_bob = _compile(captured[-1]).lower()
|
||||||
|
|
||||||
assert "workspace/alice/%" in sql_alice
|
# Both count the same workspace-wide subtree.
|
||||||
assert "workspace/alice/%" not in sql_bob
|
assert "like 'workspace/%%'" in sql_alice
|
||||||
assert "workspace/bob/%" in sql_bob
|
assert "like 'workspace/%%'" in sql_bob
|
||||||
|
# Neither embeds the user_id in the path prefix.
|
||||||
|
assert "workspace/alice/%" not in sql_alice
|
||||||
|
assert "workspace/bob/%" not in sql_bob
|
||||||
|
# Per-user narrowing happens via the visibility predicate.
|
||||||
|
assert "scripts.owner_user_id = 'alice'" in sql_alice
|
||||||
|
assert "scripts.owner_user_id = 'bob'" in sql_bob
|
||||||
|
|
||||||
|
|
||||||
|
async def test_count_scripts_admin_skips_visibility_filter() -> None:
|
||||||
|
"""Admin short-circuits the visibility predicate and counts every
|
||||||
|
active script in the workspace (dashboard '全部脚本' / '工作副本')."""
|
||||||
|
captured = []
|
||||||
|
|
||||||
|
mock_session = MagicMock()
|
||||||
|
mock_session.scalar = AsyncMock(
|
||||||
|
side_effect=lambda stmt: (captured.append(stmt), 42)[1]
|
||||||
|
)
|
||||||
|
|
||||||
|
result = await count_scripts(
|
||||||
|
context=_ctx(user_id="alice", is_admin=True), session=mock_session
|
||||||
|
)
|
||||||
|
assert result["data"] == {"total": 42}
|
||||||
|
sql = _compile(captured[0]).lower()
|
||||||
|
assert "like 'workspace/%%'" in sql
|
||||||
|
# visibility / owner_user_id still appear in the SELECT projection, but
|
||||||
|
# the visibility WHERE predicate must be absent for admins.
|
||||||
|
assert "scripts.visibility in ('workspace', 'public')" not in sql
|
||||||
|
|
||||||
|
|
||||||
async def test_count_scripts_route_declared_before_script_id_route() -> None:
|
async def test_count_scripts_route_declared_before_script_id_route() -> None:
|
||||||
|
|||||||
@@ -29,18 +29,24 @@ from sqlalchemy.dialects import mysql as mysql_dialect
|
|||||||
|
|
||||||
from backend.scripts import (
|
from backend.scripts import (
|
||||||
_build_list_scripts_descendant_prefix,
|
_build_list_scripts_descendant_prefix,
|
||||||
|
_build_list_scripts_workspace_descendant_prefix,
|
||||||
_escape_like_pattern,
|
_escape_like_pattern,
|
||||||
normalize_user_path,
|
normalize_user_path,
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
def _ctx(user_id: str = "U001") -> SimpleNamespace:
|
def _ctx(
|
||||||
|
user_id: str = "U001", *, is_admin: bool = False, is_system_admin: bool = False
|
||||||
|
) -> SimpleNamespace:
|
||||||
return SimpleNamespace(
|
return SimpleNamespace(
|
||||||
request_id="test",
|
request_id="test",
|
||||||
user=SimpleNamespace(user_id=user_id),
|
user=SimpleNamespace(user_id=user_id),
|
||||||
workspace=SimpleNamespace(workspace_id="W001"),
|
workspace=SimpleNamespace(workspace_id="W001"),
|
||||||
role=SimpleNamespace(role_code="admin"),
|
role=SimpleNamespace(role_code="admin" if is_admin else "developer"),
|
||||||
is_system_admin=False,
|
is_system_admin=is_system_admin,
|
||||||
|
# ``list_scripts`` now consults ``context.is_admin`` directly (matching
|
||||||
|
# ``list_resources``); SimpleNamespace needs it as a plain attribute.
|
||||||
|
is_admin=is_admin or is_system_admin,
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
@@ -115,6 +121,51 @@ def test_normalize_user_path_strips() -> None:
|
|||||||
assert normalize_user_path("a\\b") == "a/b"
|
assert normalize_user_path("a\\b") == "a/b"
|
||||||
|
|
||||||
|
|
||||||
|
# ─── layer 1.6: workspace-wide prefix helper ─────────────────────
|
||||||
|
|
||||||
|
|
||||||
|
def test_workspace_descendant_prefix_root() -> None:
|
||||||
|
"""Empty parent_path → workspace-wide root prefix (cross-owner)."""
|
||||||
|
assert _build_list_scripts_workspace_descendant_prefix("") == "workspace/"
|
||||||
|
|
||||||
|
|
||||||
|
def test_workspace_descendant_prefix_subdir() -> None:
|
||||||
|
"""Non-empty parent_path → appended under workspace root, no user_id."""
|
||||||
|
assert (
|
||||||
|
_build_list_scripts_workspace_descendant_prefix("foo/bar")
|
||||||
|
== "workspace/foo/bar/"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def test_workspace_descendant_prefix_escapes_metachars() -> None:
|
||||||
|
r"""Folder ``foo_bar`` must produce ``foo\_bar`` so the trailing ``%``
|
||||||
|
doesn't become 'match any single char'."""
|
||||||
|
assert (
|
||||||
|
_build_list_scripts_workspace_descendant_prefix("foo_bar")
|
||||||
|
== r"workspace/foo\_bar/"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def test_workspace_descendant_prefix_escapes_percent() -> None:
|
||||||
|
assert (
|
||||||
|
_build_list_scripts_workspace_descendant_prefix("100%match")
|
||||||
|
== r"workspace/100\%match/"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def test_workspace_descendant_prefix_normalizes_leading_trailing_slashes() -> None:
|
||||||
|
assert (
|
||||||
|
_build_list_scripts_workspace_descendant_prefix("/foo/bar/")
|
||||||
|
== "workspace/foo/bar/"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def test_workspace_descendant_prefix_rejects_traversal() -> None:
|
||||||
|
with pytest.raises(HTTPException) as exc:
|
||||||
|
_build_list_scripts_workspace_descendant_prefix("foo/../bar")
|
||||||
|
assert exc.value.status_code == 422
|
||||||
|
|
||||||
|
|
||||||
# ─── layer 2: SQL contract ────────────────────────────────────────
|
# ─── layer 2: SQL contract ────────────────────────────────────────
|
||||||
|
|
||||||
|
|
||||||
@@ -147,8 +198,8 @@ async def test_list_scripts_where_clause_uses_like_prefix_and_excludes_deeper()
|
|||||||
|
|
||||||
assert len(captured_sql) == 1
|
assert len(captured_sql) == 1
|
||||||
sql = captured_sql[0].lower()
|
sql = captured_sql[0].lower()
|
||||||
assert "like 'workspace/alice/foo/bar/%%'" in sql
|
assert "like 'workspace/foo/bar/%%'" in sql
|
||||||
assert "not like 'workspace/alice/foo/bar/%%/%%'" in sql
|
assert "not like 'workspace/foo/bar/%%/%%'" in sql
|
||||||
|
|
||||||
|
|
||||||
async def test_list_scripts_where_clause_escapes_pattern_literal() -> None:
|
async def test_list_scripts_where_clause_escapes_pattern_literal() -> None:
|
||||||
@@ -177,9 +228,9 @@ async def test_list_scripts_where_clause_escapes_pattern_literal() -> None:
|
|||||||
# doubles the escape char inside the SQL string literal, so what
|
# doubles the escape char inside the SQL string literal, so what
|
||||||
# the helper emits as `foo\_bar` renders as `foo\\_bar` here
|
# the helper emits as `foo\_bar` renders as `foo\\_bar` here
|
||||||
# (2 backslash chars in the actual SQL string).
|
# (2 backslash chars in the actual SQL string).
|
||||||
assert r"like 'workspace/alice/foo\\_bar/%%'" in sql_lower
|
assert r"like 'workspace/foo\\_bar/%%'" in sql_lower
|
||||||
# NOT LIKE clause also escaped.
|
# NOT LIKE clause also escaped.
|
||||||
assert r"not like 'workspace/alice/foo\\_bar/%%/%%'" in sql_lower
|
assert r"not like 'workspace/foo\\_bar/%%/%%'" in sql_lower
|
||||||
# And both declare ESCAPE '\\'.
|
# And both declare ESCAPE '\\'.
|
||||||
assert sql.count("ESCAPE '\\\\'") == 2, sql
|
assert sql.count("ESCAPE '\\\\'") == 2, sql
|
||||||
|
|
||||||
@@ -206,7 +257,60 @@ async def test_list_scripts_where_clause_escapes_percent_pattern() -> None:
|
|||||||
sql_lower = sql.lower()
|
sql_lower = sql.lower()
|
||||||
# SQLAlchemy doubles the escape char so `%` → `\%` becomes `\\%`
|
# SQLAlchemy doubles the escape char so `%` → `\%` becomes `\\%`
|
||||||
# in the SQL string literal.
|
# in the SQL string literal.
|
||||||
assert r"workspace/alice/100\\%%match/%%" in sql_lower
|
assert r"workspace/100\\%%match/%%" in sql_lower
|
||||||
|
|
||||||
|
|
||||||
|
async def test_list_scripts_non_admin_adds_visibility_filter() -> None:
|
||||||
|
"""Workspace-wide listing is narrowed by visibility for non-admin:
|
||||||
|
owner_user_id = me OR visibility IN (workspace, public) — exactly like
|
||||||
|
list_resources. The workspace prefix contains NO user_id (cross-owner)."""
|
||||||
|
from backend.scripts import list_scripts
|
||||||
|
|
||||||
|
captured_sql: list[str] = []
|
||||||
|
|
||||||
|
class _MockResult:
|
||||||
|
def all(self):
|
||||||
|
return []
|
||||||
|
|
||||||
|
mock_session = MagicMock()
|
||||||
|
mock_session.execute = AsyncMock(
|
||||||
|
side_effect=lambda stmt: (
|
||||||
|
captured_sql.append(_compile_sql(stmt)) or _MockResult()
|
||||||
|
)
|
||||||
|
)
|
||||||
|
|
||||||
|
await list_scripts(parent_path="", context=_ctx("alice"), session=mock_session)
|
||||||
|
sql = captured_sql[0].lower()
|
||||||
|
assert "like 'workspace/%%'" in sql
|
||||||
|
assert "scripts.owner_user_id = 'alice'" in sql
|
||||||
|
assert "scripts.visibility in ('workspace', 'public')" in sql
|
||||||
|
|
||||||
|
|
||||||
|
async def test_list_scripts_admin_skips_visibility_filter() -> None:
|
||||||
|
"""Admin short-circuits the visibility predicate and sees everything."""
|
||||||
|
from backend.scripts import list_scripts
|
||||||
|
|
||||||
|
captured_sql: list[str] = []
|
||||||
|
|
||||||
|
class _MockResult:
|
||||||
|
def all(self):
|
||||||
|
return []
|
||||||
|
|
||||||
|
mock_session = MagicMock()
|
||||||
|
mock_session.execute = AsyncMock(
|
||||||
|
side_effect=lambda stmt: (
|
||||||
|
captured_sql.append(_compile_sql(stmt)) or _MockResult()
|
||||||
|
)
|
||||||
|
)
|
||||||
|
|
||||||
|
await list_scripts(
|
||||||
|
parent_path="", context=_ctx("alice", is_admin=True), session=mock_session
|
||||||
|
)
|
||||||
|
sql = captured_sql[0].lower()
|
||||||
|
assert "like 'workspace/%%'" in sql
|
||||||
|
# owner_user_id / visibility still appear in the SELECT projection; what
|
||||||
|
# must be absent is the visibility WHERE predicate for non-admins.
|
||||||
|
assert "scripts.visibility in ('workspace', 'public')" not in sql
|
||||||
|
|
||||||
|
|
||||||
async def test_list_workspace_directories_where_clause_escapes_pattern() -> None:
|
async def test_list_workspace_directories_where_clause_escapes_pattern() -> None:
|
||||||
|
|||||||
@@ -322,6 +322,8 @@ def test_resource_payload_legacy_dot_resources():
|
|||||||
)
|
)
|
||||||
assert payload["jupyter_accessible_path"] == ".resources/data.csv"
|
assert payload["jupyter_accessible_path"] == ".resources/data.csv"
|
||||||
assert payload["absolute_path"].endswith(f"{ws}/{user}/.resources/data.csv")
|
assert payload["absolute_path"].endswith(f"{ws}/{user}/.resources/data.csv")
|
||||||
|
# Default None when no Users join row is provided (bind_resource path).
|
||||||
|
assert payload["owner_display_name"] is None
|
||||||
|
|
||||||
|
|
||||||
def test_resource_payload_new_flat_path():
|
def test_resource_payload_new_flat_path():
|
||||||
@@ -333,6 +335,13 @@ def test_resource_payload_new_flat_path():
|
|||||||
)
|
)
|
||||||
assert payload["jupyter_accessible_path"] == "data.csv"
|
assert payload["jupyter_accessible_path"] == "data.csv"
|
||||||
assert payload["absolute_path"].endswith(f"{ws}/{user}/data.csv")
|
assert payload["absolute_path"].endswith(f"{ws}/{user}/data.csv")
|
||||||
|
# Explicit owner_display_name is passed through to the payload.
|
||||||
|
payload = resource_payload(
|
||||||
|
_make_resource(ws, user),
|
||||||
|
_make_storage_object(f"{ws}/{user}/data.csv"),
|
||||||
|
owner_display_name="张三",
|
||||||
|
)
|
||||||
|
assert payload["owner_display_name"] == "张三"
|
||||||
|
|
||||||
|
|
||||||
def test_resource_payload_new_nested_path():
|
def test_resource_payload_new_nested_path():
|
||||||
@@ -344,6 +353,7 @@ def test_resource_payload_new_nested_path():
|
|||||||
)
|
)
|
||||||
assert payload["jupyter_accessible_path"] == "train/v1/data.csv"
|
assert payload["jupyter_accessible_path"] == "train/v1/data.csv"
|
||||||
assert payload["absolute_path"].endswith(f"{ws}/{user}/train/v1/data.csv")
|
assert payload["absolute_path"].endswith(f"{ws}/{user}/train/v1/data.csv")
|
||||||
|
assert payload["owner_display_name"] is None
|
||||||
|
|
||||||
|
|
||||||
def test_compute_jupyter_relative_path_for_legacy_and_new_paths():
|
def test_compute_jupyter_relative_path_for_legacy_and_new_paths():
|
||||||
@@ -621,6 +631,27 @@ async def test_list_resources_without_parent_path_adds_no_like_clause() -> None:
|
|||||||
assert " not like " not in sql
|
assert " not like " not in sql
|
||||||
|
|
||||||
|
|
||||||
|
async def test_list_resources_joins_users_for_display_name() -> None:
|
||||||
|
"""list_resources must OUTER JOIN users and SELECT users.display_name so
|
||||||
|
every resource carries owner_display_name (frontend displayName chain)."""
|
||||||
|
from backend.resources import list_resources
|
||||||
|
|
||||||
|
captured_sql: list[str] = []
|
||||||
|
mock_session = _list_resources_capturing_session(captured_sql)
|
||||||
|
|
||||||
|
await list_resources(
|
||||||
|
parent_path="",
|
||||||
|
context=_resource_ctx(),
|
||||||
|
session=mock_session,
|
||||||
|
visibility=None,
|
||||||
|
keyword=None,
|
||||||
|
)
|
||||||
|
sql = captured_sql[0]
|
||||||
|
sql_lower = sql.lower()
|
||||||
|
assert "outer join users" in sql_lower
|
||||||
|
assert "users.display_name" in sql_lower
|
||||||
|
|
||||||
|
|
||||||
# ─── layer 3: behavioral test on real LIKE execution (SQLite) ────────────────
|
# ─── layer 3: behavioral test on real LIKE execution (SQLite) ────────────────
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@@ -109,8 +109,12 @@ export function ScriptExplorer({
|
|||||||
dataResources: ResourceItem[];
|
dataResources: ResourceItem[];
|
||||||
}[] = [];
|
}[] = [];
|
||||||
for (const [ownerUserId, groupScripts] of byOwner.entries()) {
|
for (const [ownerUserId, groupScripts] of byOwner.entries()) {
|
||||||
|
const groupDataResources = dataByOwner.get(ownerUserId) ?? [];
|
||||||
|
// data-only owner(没有 scripts 的用户)回退到 data resources 的
|
||||||
|
// owner_display_name,否则 data-only owner 前端只能显示 userId 末 6 位。
|
||||||
const displayName =
|
const displayName =
|
||||||
groupScripts[0]?.owner_display_name ??
|
groupScripts[0]?.owner_display_name ??
|
||||||
|
groupDataResources[0]?.owner_display_name ??
|
||||||
(ownerUserId === user?.user_id ? user?.display_name : null) ??
|
(ownerUserId === user?.user_id ? user?.display_name : null) ??
|
||||||
`${ownerUserId.slice(-6)}…`;
|
`${ownerUserId.slice(-6)}…`;
|
||||||
const groupUser =
|
const groupUser =
|
||||||
@@ -133,7 +137,7 @@ export function ScriptExplorer({
|
|||||||
ownerUserId === user?.user_id
|
ownerUserId === user?.user_id
|
||||||
? mergeDirectories(directories, inferred)
|
? mergeDirectories(directories, inferred)
|
||||||
: inferred,
|
: inferred,
|
||||||
dataResources: dataByOwner.get(groupUser.user_id) ?? [],
|
dataResources: groupDataResources,
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -417,6 +417,7 @@ export type ResourceItem = {
|
|||||||
workspace_id: string;
|
workspace_id: string;
|
||||||
storage_object_id: string;
|
storage_object_id: string;
|
||||||
owner_user_id: string;
|
owner_user_id: string;
|
||||||
|
owner_display_name?: string | null;
|
||||||
resource_name: string;
|
resource_name: string;
|
||||||
description: string | null;
|
description: string | null;
|
||||||
visibility: "private" | "workspace" | "public";
|
visibility: "private" | "workspace" | "public";
|
||||||
|
|||||||
Reference in New Issue
Block a user