mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-10-29 22:50:37 +00:00
fix(skills): capture package digests without POSIX-only dir_fd (#7583)
* fix(skills): capture package digests without POSIX-only dir_fd `capture_package` rejected every non-POSIX host, so `SkillMutationRuntime.state` fell back to `"unsupported-" + uuid4().hex`. That digest differs on every call, so `repository.observe()` recorded a mutation for an untouched package: `mutation_seq` kept climbing and ABA detection could never match. Walk the tree with `scandir` + `lstat` where `dir_fd`/`O_NOFOLLOW` are missing, comparing the pre-open `lstat` against the opened descriptor before reading, and leave the POSIX descriptor walk unchanged. Windows 11 / Python 3.12: test_skill_asset_versions.py 5 failed -> 23 passed; reverting this file alone brings the 5 failures back. * docs(gateway): keep guidance below CI size budget --------- Co-authored-by: Felix Shi <204130665+shi-felix@users.noreply.github.com> Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
This commit is contained in:
Felix Shi
GitHub
parent
b3f23326a5
commit
44b88151be
@ -76,28 +76,32 @@ OIDC redirects retain existing endpoint query bytes (RFC 6749 section 3.1).
|
||||
Malformed IdP bodies raise `OIDCError` (502/`sso_failed`).
|
||||
`tests/test_oidc_auth.py` pins query, state-cookie, nonce and PKCE agreement.
|
||||
|
||||
Personal Access Tokens (`app.gateway.auth.pat`, `Authorization: Bearer dfp_...`) run as their owning user: an invalid Bearer is a hard 401 with no cookie fallback, which keeps `CSRFMiddleware`'s Bearer skip safe (origin checks still run). Scopes narrow within the allowlisted threads/runs/projects routes (including `POST /api/threads/{id}/move`); every other authenticated route 403s PAT callers (admin included). PAT management and `/change-password` require session auth; only SHA-256 digests are stored (`0017`).
|
||||
PATs (`app.gateway.auth.pat`, `Authorization: Bearer dfp_...`) act as their owner.
|
||||
Invalid Bearers return 401 without cookie fallback, keeping `CSRFMiddleware`'s
|
||||
Bearer skip safe; origin checks still run. Scopes narrow allowlisted thread/run/
|
||||
project routes, including `POST /api/threads/{id}/move`; other authenticated
|
||||
routes reject PATs with 403, even for admins. PAT management and `/change-password`
|
||||
require session auth. Store only SHA-256 digests (`0017`).
|
||||
|
||||
Thread→project membership is written by thread creation (`POST /api/threads` with
|
||||
a validated `project_id`), branch creation (the new row inherits the source
|
||||
thread's project; an archived/deleted project degrades the branch to unassigned
|
||||
instead of failing), and explicit moves (`POST /api/threads/{id}/move`); run
|
||||
admission never modifies membership; it only pins the resolved project context
|
||||
read-only into the run context. The server-reserved `deerflow_project_id`
|
||||
metadata key is a read-only exposure of the `threads_meta.project_id` column and
|
||||
is stripped from client writes.
|
||||
Thread→project membership is written only by thread creation (`POST /api/threads`
|
||||
with validated `project_id`), branch creation (inherit the source project;
|
||||
archived/deleted projects become unassigned rather than failing), and explicit
|
||||
`POST /api/threads/{id}/move`. Run admission pins resolved project context
|
||||
read-only and never changes membership. Server-reserved `deerflow_project_id`
|
||||
metadata exposes `threads_meta.project_id` read-only and is stripped from client writes.
|
||||
|
||||
Localhost persistence deliberately reads the direct request `Host` and ignores `Forwarded` / `X-Forwarded-Host`. Scheme and auth-origin reconstruction still consume forwarding headers. The bundled nginx sets `X-Forwarded-Proto`, but preserves an upstream HTTPS value and does not overwrite every forwarded header, so the outer trusted proxy must replace or strip client-supplied forwarding headers before traffic reaches DeerFlow.
|
||||
Localhost persistence trusts direct `Host`, not `Forwarded`/`X-Forwarded-Host`.
|
||||
Scheme/auth-origin reconstruction still uses forwarding headers. Nginx sets
|
||||
`X-Forwarded-Proto` but preserves upstream HTTPS and some other forwarded headers;
|
||||
the outer trusted proxy must replace/strip client copies before DeerFlow.
|
||||
|
||||
Standalone local LangGraph Studio is recognized only through the upstream
|
||||
`Auth.types.StudioUser` principal type, never by its reusable identity string.
|
||||
The type is resolved once at import; an older SDK without it degrades to normal
|
||||
owner scoping.
|
||||
For that principal's assistant reads/searches, `langgraph_auth.add_owner_filter`
|
||||
selects genuine server-registered assistants plus assistants owned by Studio;
|
||||
all other resources remain owner-scoped. Assistant create/update handlers make
|
||||
both `user_id` and `created_by=user` server-owned, because LangGraph gives
|
||||
`created_by=system` privileged ownership semantics during run creation. The
|
||||
Recognize standalone Studio only by upstream `Auth.types.StudioUser`, never its
|
||||
reusable identity string. Resolve the type once at import; older SDKs without
|
||||
it retain normal owner scoping. For its assistant reads/searches,
|
||||
`langgraph_auth.add_owner_filter` admits genuine server-registered assistants
|
||||
and Studio-owned rows; other resources stay owner-scoped. Assistant create/update
|
||||
forces server-owned `user_id` and `created_by=user`: LangGraph treats
|
||||
`created_by=system` as privileged run ownership. The
|
||||
custom application module in `langgraph_studio.py` is imported before the
|
||||
locked in-memory runtime lifespan. At that pre-runtime boundary it derives
|
||||
genuine system assistant IDs from the CLI-provided graph registry, removes
|
||||
|
||||
@ -1,4 +1,11 @@
|
||||
"""Bounded byte snapshots for managed custom packages (POSIX, no links)."""
|
||||
"""Bounded byte snapshots for managed custom packages (no links).
|
||||
|
||||
POSIX walks the tree with ``dir_fd`` descriptors so every path component is
|
||||
anchored. Hosts without ``dir_fd``/``O_NOFOLLOW`` (Windows) use
|
||||
:func:`_capture_package_portable`, which re-checks each component with ``lstat``
|
||||
before use and compares the pre-open and post-open ``stat`` of every file, so a
|
||||
swap during the walk is still rejected instead of followed.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
@ -47,7 +54,7 @@ class PackageSnapshot:
|
||||
def capture_package(root: Path) -> PackageSnapshot:
|
||||
"""Caller owns mutation guard. Never read outside the opened package tree."""
|
||||
if os.name != "posix":
|
||||
raise ValueError("UNSUPPORTED_ASSET")
|
||||
return _capture_package_portable(root)
|
||||
files: list[PackageFile] = []
|
||||
total = 0
|
||||
entries = 0
|
||||
@ -146,3 +153,101 @@ def open_directory_chain(path: Path) -> int:
|
||||
except BaseException:
|
||||
os.close(fd)
|
||||
raise
|
||||
|
||||
|
||||
def _resolve_unlinked_ancestors(path: Path) -> Path:
|
||||
"""Return *path* after rejecting a link in any component above it.
|
||||
|
||||
The non-POSIX twin of :func:`open_directory_chain`: there is no ``dir_fd``
|
||||
to anchor the walk, so each component is ``lstat``-ed in order and a link
|
||||
stops the walk instead of being traversed.
|
||||
"""
|
||||
path = path.absolute()
|
||||
if ".." in path.parts or len(path.parts) > 128 or len(os.fsencode(path)) > 4096:
|
||||
raise ValueError("UNSUPPORTED_ASSET")
|
||||
current = Path(path.anchor)
|
||||
for part in path.parts[1:]:
|
||||
current = current / part
|
||||
info = os.lstat(current)
|
||||
if stat.S_ISLNK(info.st_mode) or not stat.S_ISDIR(info.st_mode):
|
||||
raise ValueError("UNSUPPORTED_ASSET")
|
||||
return path
|
||||
|
||||
|
||||
def _capture_package_portable(root: Path) -> PackageSnapshot:
|
||||
"""Capture a package where ``dir_fd``/``O_NOFOLLOW`` are unavailable.
|
||||
|
||||
``os.lstat`` runs before every open and the opened descriptor is compared
|
||||
against that ``lstat`` afterwards, so a component replaced between the two
|
||||
calls raises ``REVISION_CONFLICT`` instead of publishing foreign bytes.
|
||||
Windows keeps ``st_dev``/``st_ino`` for NTFS volumes, which makes that
|
||||
comparison meaningful there; the executable bit stays unset because the
|
||||
platform has no mode bits, and that is consistent within a host.
|
||||
"""
|
||||
files: list[PackageFile] = []
|
||||
total = 0
|
||||
entries = 0
|
||||
open_flags = os.O_RDONLY | getattr(os, "O_BINARY", 0) | getattr(os, "O_NOINHERIT", 0)
|
||||
|
||||
def walk(directory: Path, prefix: str = "", depth: int = 0) -> None:
|
||||
nonlocal total, entries
|
||||
if depth > MAX_DEPTH:
|
||||
raise ValueError("QUOTA_EXCEEDED")
|
||||
names = []
|
||||
with os.scandir(directory) as iterator:
|
||||
for entry in iterator:
|
||||
entries += 1
|
||||
if entries > MAX_FILES * 2:
|
||||
raise ValueError("QUOTA_EXCEEDED")
|
||||
names.append(entry.name)
|
||||
for name in sorted(names):
|
||||
if "\\" in name or any(ord(c) < 32 for c in name):
|
||||
raise ValueError("UNSUPPORTED_ASSET")
|
||||
path = f"{prefix}/{name}" if prefix else name
|
||||
if len(path.encode("utf-8")) > 1024:
|
||||
raise ValueError("QUOTA_EXCEEDED")
|
||||
target = directory / name
|
||||
info = os.lstat(target)
|
||||
if stat.S_ISLNK(info.st_mode):
|
||||
raise ValueError("UNSUPPORTED_ASSET")
|
||||
if stat.S_ISDIR(info.st_mode):
|
||||
walk(target, path, depth + 1)
|
||||
continue
|
||||
if not stat.S_ISREG(info.st_mode):
|
||||
raise ValueError("UNSUPPORTED_ASSET")
|
||||
if len(files) >= MAX_FILES:
|
||||
raise ValueError("QUOTA_EXCEEDED")
|
||||
limit = min(MAX_PACKAGE_BYTES - total, MAX_MAIN_BYTES if path == "SKILL.md" else MAX_PACKAGE_BYTES)
|
||||
if info.st_size > limit:
|
||||
raise ValueError("QUOTA_EXCEEDED")
|
||||
fd = os.open(target, open_flags)
|
||||
try:
|
||||
opened = os.fstat(fd)
|
||||
if (info.st_dev, info.st_ino, info.st_mode) != (opened.st_dev, opened.st_ino, opened.st_mode):
|
||||
raise ValueError("REVISION_CONFLICT")
|
||||
data = bytearray()
|
||||
while True:
|
||||
chunk = os.read(fd, min(65536, limit + 1 - len(data)))
|
||||
if not chunk:
|
||||
break
|
||||
data.extend(chunk)
|
||||
if len(data) > limit:
|
||||
raise ValueError("QUOTA_EXCEEDED")
|
||||
after = os.fstat(fd)
|
||||
if (opened.st_size, opened.st_mtime_ns, opened.st_ctime_ns) != (after.st_size, after.st_mtime_ns, after.st_ctime_ns):
|
||||
raise ValueError("REVISION_CONFLICT")
|
||||
total += len(data)
|
||||
files.append(PackageFile(path, bytes(data), bool(opened.st_mode & 0o111)))
|
||||
finally:
|
||||
os.close(fd)
|
||||
|
||||
try:
|
||||
walk(_resolve_unlinked_ancestors(root))
|
||||
except OSError as exc:
|
||||
raise ValueError("UNSUPPORTED_ASSET") from exc
|
||||
result = PackageSnapshot(tuple(sorted(files, key=lambda item: item.path)))
|
||||
try:
|
||||
result.main_content
|
||||
except (StopIteration, UnicodeError) as exc:
|
||||
raise ValueError("UNSUPPORTED_ASSET") from exc
|
||||
return result
|
||||
|
||||
@ -24,7 +24,7 @@ def assets(tmp_path, monkeypatch):
|
||||
storage = UserScopedSkillStorage("owner", host_path=str(tmp_path / "skills"))
|
||||
root = storage.get_custom_skill_dir("example")
|
||||
root.mkdir(parents=True)
|
||||
(root / "SKILL.md").write_text(CONTENT, encoding="utf-8")
|
||||
(root / "SKILL.md").write_text(CONTENT, encoding="utf-8", newline="")
|
||||
engine = create_engine(f"sqlite:///{tmp_path / 'mutations.db'}")
|
||||
Base.metadata.create_all(engine)
|
||||
sessions = sessionmaker(engine, expire_on_commit=False)
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user