Skip to content

fix: isolate temp scope by user within an account#1398

Merged
qin-ctx merged 5 commits intovolcengine:mainfrom
Hinotoi-agent:fix/temp-scope-user-isolation
Apr 14, 2026
Merged

fix: isolate temp scope by user within an account#1398
qin-ctx merged 5 commits intovolcengine:mainfrom
Hinotoi-agent:fix/temp-scope-user-isolation

Conversation

@Hinotoi-agent
Copy link
Copy Markdown
Contributor

Summary

This PR hardens OpenViking temp-scope isolation so one authenticated user can no longer read, enumerate, or overwrite another same-account user's temporary files.

  • temp URIs are now user-scoped instead of account-global
  • temp access checks now require the caller to match the temp owner's user space
  • content-write temp trees are created with the active request context
  • regression tests cover same-account read/write isolation and temp-root listing visibility

Security issues covered

Issue Impact Severity
Same-account temp scope was shared across users Confidentiality + integrity break for temporary data High
temp root listings exposed other users' temp artifacts Cross-user enumeration of in-progress work Medium-High

Before this PR

  • viking://temp/* was treated as accessible to any non-root authenticated user
  • temp paths resolved under /local/{account_id}/temp/... without a user ownership segment
  • content-write temporary trees inherited that shared temp namespace
  • one same-account user could read or overwrite another user's temp files if they knew or discovered the URI

After this PR

  • temp URIs are created as viking://temp/<user-space>/<temp-id>
  • non-root access to temp is limited to the owning user space
  • content-write temp trees are created with the request context so they land in the correct user-scoped temp namespace
  • regression tests lock in same-account isolation and temp listing behavior

Why this matters

Temporary files can contain uploaded inputs, parser intermediates, or working copies created during write flows. Sharing the temp namespace across all users in the same account breaks user isolation and lets one user interfere with another user's in-progress work.

Attack flow

same-account authenticated user
    -> accesses viking://temp/*
        -> temp ACL treated all temp URIs as globally accessible
            -> reads, enumerates, or overwrites another user's temporary files

Affected code

Area Files
Temp URI generation openviking_cli/utils/uri.py
Temp access control openviking/storage/viking_fs.py
Content write temp staging openviking/storage/content_write.py
Regression coverage tests/server/test_temp_scope_acl.py

Root cause

  • Temp paths were scoped only by account, not by user ownership.
  • _is_accessible() treated the entire temp scope as universally readable/writable for non-root users.

CVSS assessment

Issue CVSS v3.1 Vector
Same-account cross-user temp access 7.7 High AV:N/AC:L/PR:L/UI:N/S:C/C:H/I:H/A:L

Rationale:

  • low-privileged authenticated users could cross a user-isolation boundary
  • the flaw affected confidentiality and integrity of temporary processing data
  • exploitation did not require user interaction once the attacker had same-account access

Safe reproduction steps

  1. Create a temp artifact as user A under viking://temp/....
  2. As user B in the same account, try to read or write that temp file.
  3. As user B, list viking://temp.
  4. Observe whether user B can see or modify user A's temp content.

Expected vulnerable behavior

  • same-account user B can read user A's temp file
  • same-account user B can overwrite user A's temp file
  • same-account user B can discover user A's temp directory through temp listing

Changes in this PR

  • add user-space ownership to generated temp URIs
  • restrict temp-scope access checks to the owning user space
  • pass request context into content-write temp URI creation
  • add regression tests for same-account temp isolation and temp-root listing visibility

Files changed

Category Files What changed
URI generation openviking_cli/utils/uri.py Added user-scoped temp URI generation with compatibility fallback
Access control openviking/storage/viking_fs.py Temp scope now checks caller user space before allowing access
Write pipeline openviking/storage/content_write.py Temp staging now creates user-scoped temp roots from request context
Tests tests/server/test_temp_scope_acl.py Added regression coverage for temp isolation

Maintainer impact

  • The patch is narrow and limited to temp URI generation and temp-scope authorization.
  • Resource, user, agent, and session scopes keep their existing behavior.
  • The regression tests are self-contained and do not require external services.

Fix rationale

Temp data should be isolated at least as strictly as other user-scoped working data. User-scoped temp URIs preserve the existing temp workflow while restoring the expected user boundary inside a shared account.

Type of change

  • Bug fix (non-breaking change which fixes a vulnerability)
  • Security hardening
  • Test coverage for regression prevention
  • New feature
  • Breaking change
  • Documentation-only change

Test plan

  • Added regression tests for same-account temp read/write isolation
  • Added regression tests for temp root listing visibility
  • Verified generated temp URIs include the caller user space

Executed:

python -m pytest -o addopts='' tests/server/test_temp_scope_acl.py -q

Disclosure notes

  • Claims in this PR are limited to the reviewed temp-scope code paths and reproduced VikingFS behavior.
  • This PR does not change unrelated storage scopes.
  • The issue appears to affect same-account cross-user isolation rather than cross-account isolation.

@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Apr 12, 2026

CLA assistant check
All committers have signed the CLA.

@github-actions
Copy link
Copy Markdown

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🏅 Score: 95
🧪 PR contains tests
🔒 No security concerns identified
✅ No TODO sections
🔀 No multiple PR themes
⚡ No major issues detected

@github-actions
Copy link
Copy Markdown

PR Code Suggestions ✨

No code suggestions found for the PR.

@MaojiaSheng MaojiaSheng requested a review from qin-ctx April 13, 2026 06:45
@qin-ctx qin-ctx self-assigned this Apr 13, 2026
Copy link
Copy Markdown
Collaborator

@qin-ctx qin-ctx left a comment

Choose a reason for hiding this comment

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

I found two blocking issues before this temp-scope hardening is safe to merge: the temp scope root is still globally mutable for same-account users via destructive operations, and the implementation no longer preserves the documented legacy temp URI compatibility. I also left one non-blocking test coverage suggestion.

@Hinotoi-agent
Copy link
Copy Markdown
Contributor Author

Resolved the blocking temp-scope follow-ups in commit 33e0892.

Validated on latest PR head 33e0892:

  • PYTHONPATH=. python -m pytest -o addopts= tests/server/test_temp_scope_acl.py -q ✅ (5 passed)
  • uv run pytest -o addopts= tests/server -q ⚠️ blocked here because the editable build expects the missing openviking/bin/ov artifact
  • uv run ruff check openviking tests / uvx ruff check openviking tests ⚠️ repo currently has many unrelated pre-existing lint violations outside this change

What changed:

  • blocked non-root destructive/mutating operations against viking://temp itself
  • preserved access to legacy viking://temp/<temp-id> trees while keeping new temp URIs user-scoped
  • added regression coverage for both edges in tests/server/test_temp_scope_acl.py

@Hinotoi-agent
Copy link
Copy Markdown
Contributor Author

Addressed on the PR branch in bc1304a.

What changed:

  • preserved legacy temp URI behavior when no explicit or bound request context is available, so callers without request context still get the historical viking://temp/<temp-id> shape instead of viking://temp/<user>/<temp-id>
  • taught temp-space parsing to recognize the legacy shape as compatibility mode rather than treating the second segment as an owner-space segment
  • added regression coverage for both legacy temp compatibility and the destructive viking://temp root edge case the review called out

Validation run:

  • PYTHONPATH=/Users/lennon/.hermes/repos/volcengine__OpenViking /tmp/ovtest-venv/bin/python -m pytest -o addopts= tests/server/test_temp_scope_acl.py -q
  • /tmp/ovtest-venv/bin/python -m ruff check openviking/storage/viking_fs.py tests/server/test_temp_scope_acl.py

Thanks — this should cover the two blocking issues plus the follow-up regression suggestion.

Copy link
Copy Markdown
Collaborator

@qin-ctx qin-ctx left a comment

Choose a reason for hiding this comment

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

I re-checked the latest patch after the previous feedback. The earlier temp-root mutation and legacy no-context compatibility issues are fixed, but the current legacy-temp compatibility heuristic introduces a new blocking ACL regression: a valid user space that matches the legacy temp-id pattern is now treated as globally accessible temp data.

@qin-ctx qin-ctx merged commit ce89b46 into volcengine:main Apr 14, 2026
6 checks passed
@github-project-automation github-project-automation bot moved this from Backlog to Done in OpenViking project Apr 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants