fix: protect global watch-task control files from non-root access#1396
Open
Hinotoi-agent wants to merge 1 commit intovolcengine:mainfrom
Open
fix: protect global watch-task control files from non-root access#1396Hinotoi-agent wants to merge 1 commit intovolcengine:mainfrom
Hinotoi-agent wants to merge 1 commit intovolcengine:mainfrom
Conversation
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
PR Code Suggestions ✨No code suggestions found for the PR. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
OpenViking global watch-task control file ACL hardening
Title
fix: protect global watch-task control files from non-root access
Summary
This PR hardens OpenViking's watch scheduler state boundary by preventing non-root authenticated users from reading, enumerating, or overwriting the global watch-task control files stored under the shared
resourcesscope.resourcesaccess model or explicitly blocks access to the watch control filesSecurity issues covered
viking://resources/.watch_tasks.jsonand variantsBefore this PR
WatchManagerpersists scheduler state inviking://resources/.watch_tasks.json,.bak, and.tmpVikingFS._is_accessible()allows non-root access to allresources-scope URIsresourcesare omitted from normal listing output but are still enumerable with hidden-file listing enabled.abstract.md,.overview.md, and.relations.json, but do not block watch-task control filesAfter this PR
Why this matters
The watch scheduler stores globally trusted control data in a namespace that the access-control layer currently treats as readable for any non-root authenticated user. That creates a control-plane boundary problem:
If the scheduler later consumes attacker-modified watch state, this becomes a direct cross-user tampering primitive against global scheduler behavior.
Attack flow
Affected code
openviking/resource/watch_manager.pyresourcesURIsopenviking/storage/viking_fs.pyopenviking/storage/content_write.pyRoot cause
Shared scheduler state stored in a broad-access namespace
WatchManagerstores trusted scheduler metadata inviking://resources/.watch_tasks.jsonand backup/temp siblingsresourcesis a shared namespace, but these files function as internal control-plane state rather than user-facing shared contentAccess control is too broad for internal resources
VikingFS._is_accessible()currently returnsTruefor non-root users on allresourcesscope URIsWrite validation protects some internals but misses watch state
ContentWriteCoordinator._validate_target_uri()blocks only derived semantic filesCVSS assessment
CVSS:3.1/AV:N/AC:L/PR:L/UI:N/S:U/C:L/I:N/A:NCVSS:3.1/AV:N/AC:L/PR:L/UI:N/S:U/C:L/I:H/A:LRationale:
Safe reproduction steps
viking://resources/.watch_tasks.jsonviking://resources/.watch_tasks.json.bakviking://resources/.watch_tasks.json.tmpviking://resourceswith hidden-file output enabled.viking://resources/.watch_tasks.jsonthrough the normal content-write path.Expected vulnerable behavior
resources.abstract.mdare blocked, demonstrating that this is a missing-protection gap rather than a universal no-rules write pathValidation performed
WatchManagerpersists task state at:viking://resources/.watch_tasks.jsonviking://resources/.watch_tasks.json.bakviking://resources/.watch_tasks.json.tmpVikingFS._is_accessible()allows non-root access toresourcesscope URIsChanges in this PR
Files changed
openviking/resource/watch_manager.pyopenviking/storage/viking_fs.pyopenviking/storage/content_write.pytests/...Maintainer impact
Fix rationale
The secure default is to keep scheduler control metadata out of the general shared-resource trust model. These files are not ordinary collaborative resources; they are privileged internal state used by the watch subsystem.
A durable fix is to enforce one of these models:
Either approach is better than relying on hidden-file naming alone.
Type of change
Test plan
WatchManagerresourcesaccess rule inVikingFS._is_accessible()VikingFS.ls()ContentWriteCoordinator._validate_target_uri()Executed validation:
openviking/resource/watch_manager.pyopenviking/storage/viking_fs.pyopenviking/storage/content_write.pyDisclosure notes