fix: scope the id-addressed storage APIs by namespace - #606
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
🚧 Files skipped from review as they are similar to previous changes (7)
📝 WalkthroughWalkthroughOptional namespace context now flows through storage contracts, Diesel and Redis backends, schedulers, workers, SDK bindings, dashboard routes, and tests. Reads, mutations, recovery, logs, errors, archival listings, and concurrency counts now support namespace filtering. ChangesNamespace scoping
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Queue
participant Scheduler
participant Storage
participant JobRecord
Queue->>Scheduler: start with namespace
Scheduler->>Storage: request operation with namespace
Storage->>JobRecord: filter or mutate matching record
JobRecord-->>Storage: scoped result
Storage-->>Queue: result or not-found
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
Comment |
pratyush618
force-pushed
the
fix/namespace-scoping-id-apis
branch
from
August 2, 2026 09:12
d14eeaa to
197b84b
Compare
A caller scoped to one namespace could read or mutate another's job through a bare id. Every id-addressed method now takes the namespace and answers not-found on a mismatch.
pratyush618
force-pushed
the
fix/namespace-scoping-id-apis
branch
from
August 2, 2026 09:52
197b84b to
7a44fd9
Compare
![coderabbitai[bot]](https://avatars.github.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/taskito-core/src/storage/diesel_common/jobs.rs (1)
1085-1152: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftReject cross-namespace dependencies at enqueue time.
validate_dependencyand Redisvalidate_dep_idsdo not compare namespaces. A pending job in namespace B can therefore depend on a job in namespace A. When A is cancelled or dead,cascade_cancelskips B, whiledeps_satisfiedkeeps B blocked by the non-Complete dependency. Apply the namespace check to every enqueue path, including batch and unique enqueue. Documentation alone does not resolve this stuck-job state.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/taskito-core/src/storage/diesel_common/jobs.rs` around lines 1085 - 1152, Update dependency validation in validate_dependency and Redis validate_dep_ids to reject dependencies whose namespace differs from the enqueued job’s namespace. Apply the same validation consistently across standard, batch, and unique enqueue paths so cross-namespace pending dependencies cannot be created; do not rely on cascade_cancel filtering or documentation as a workaround.Source: Learnings
🧹 Nitpick comments (1)
sdks/java/src/test/java/org/byteveda/taskito/core/NamespaceScopingTest.java (1)
29-53: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDequeue the job before asserting
requestCancel/cancel_running_jobisolation.In all three SDKs, the test job stays Pending because it is enqueued but never dequeued.
requestCancel/cancel_running_jobtargets an executing job; on a Pending job it returnsfalsein every namespace, including the owning one. The assertion passes whether or not namespace filtering is implemented for this API, so it gives no real signal. The Rust reference testa_running_job_from_another_namespace_cannot_be_cancel_requestedincrates/taskito-core/tests/namespace_scoping.rsavoids this by dequeuing the job toRunningfirst.
sdks/java/src/test/java/org/byteveda/taskito/core/NamespaceScopingTest.java#L29-L53: dequeue/claim the job (or otherwise move it toRunning) in namespacens-abefore callingb.requestCancel(id)at line 45, so the assertion exercises cross-namespace protection on a live job.sdks/node/test/core/namespaceScoping.test.ts#L15-L36: start a worker or otherwise dequeue the job in namespacens-abefore callingb.requestCancel(id)at line 29, so the assertion exercises cross-namespace protection on a live job.sdks/python/tests/core/test_namespace.py#L121-L150: dequeue the job in namespacens-abefore callingq_b.cancel_running_job(job.id)at line 144, so the assertion exercises cross-namespace protection on a live job.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sdks/java/src/test/java/org/byteveda/taskito/core/NamespaceScopingTest.java` around lines 29 - 53, Namespace cancellation tests currently use Pending jobs, so they do not verify cross-namespace protection for running jobs. In sdks/java/src/test/java/org/byteveda/taskito/core/NamespaceScopingTest.java lines 29-53, dequeue or claim the job in namespace ns-a before b.requestCancel(id); make the equivalent change in sdks/node/test/core/namespaceScoping.test.ts lines 15-36 before b.requestCancel(id and sdks/python/tests/core/test_namespace.py lines 121-150 before q_b.cancel_running_job(job.id). Preserve the existing ownership and cancellation assertions after the job is Running.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/taskito-core/src/scheduler/result_handler.rs`:
- Line 71: Update every result-path mutation in the result handler to pass
self.namespace into complete_execution, record_error, retry, complete, and
complete_batch, including paths that currently mutate before or without get_job.
Enforce the namespace constraint atomically within each corresponding storage
mutation so scoped callers cannot modify foreign jobs.
In `@crates/taskito-core/src/storage/traits.rs`:
- Around line 118-121: Define and document one cross-namespace policy for
dependency edges, then apply it consistently to single, unique, and batch
enqueue paths across Diesel and Redis. Update the Storage trait methods
get_dependencies and get_dependents, their implementations, and all
dashboard/Java/Node/Python callers to enforce namespace filtering or rejection
according to that policy. Add isolation tests covering enqueue and both
edge-read directions, including Redis batch enqueue and Diesel enqueue_batch.
In `@crates/taskito-java/src/queue/inspect.rs`:
- Around line 324-327: Filter DAG edges by namespace before exposing
relationships: in crates/taskito-java/src/queue/inspect.rs:324-327,
crates/taskito-node/src/queue/inspect.rs:382-398, and
crates/taskito-server/src/dashboard/routes/jobs.rs:208-222, scope each
dependency and dependent lookup with the current namespace and only add the edge
or queue the adjacent job after it is visible; in
crates/taskito-server/src/dashboard/routes/jobs.rs:123-127, retain and pass the
namespace into build_dag.
In `@crates/taskito-java/src/workflows/mod.rs`:
- Line 427: Scope all workflow job operations by passing
queue.namespace.as_deref() instead of None: update get_job at
crates/taskito-java/src/workflows/mod.rs:427-427 and :681-681, and
request_cancel at :539-539. Ensure namespace-scoped queues cannot access or
mutate jobs in another namespace.
In `@crates/taskito-node/src/queue/workflows.rs`:
- Line 205: Scope all workflow job bridge lookups to the queue namespace by
passing self.namespace.as_deref() to get_job at
crates/taskito-node/src/queue/workflows.rs lines 205, 365, and 741, so
cross-namespace jobs return not-found and cannot be read or mutated.
In `@crates/taskito-python/src/py_queue/workflow_ops/nodes.rs`:
- Line 54: Update the get_job call in mark_workflow_node_result to pass the
queue namespace via self.namespace.as_deref() instead of None, ensuring workflow
result updates and cascade mutations only access jobs within the caller’s
namespace.
---
Outside diff comments:
In `@crates/taskito-core/src/storage/diesel_common/jobs.rs`:
- Around line 1085-1152: Update dependency validation in validate_dependency and
Redis validate_dep_ids to reject dependencies whose namespace differs from the
enqueued job’s namespace. Apply the same validation consistently across
standard, batch, and unique enqueue paths so cross-namespace pending
dependencies cannot be created; do not rely on cascade_cancel filtering or
documentation as a workaround.
---
Nitpick comments:
In `@sdks/java/src/test/java/org/byteveda/taskito/core/NamespaceScopingTest.java`:
- Around line 29-53: Namespace cancellation tests currently use Pending jobs, so
they do not verify cross-namespace protection for running jobs. In
sdks/java/src/test/java/org/byteveda/taskito/core/NamespaceScopingTest.java
lines 29-53, dequeue or claim the job in namespace ns-a before
b.requestCancel(id); make the equivalent change in
sdks/node/test/core/namespaceScoping.test.ts lines 15-36 before
b.requestCancel(id and sdks/python/tests/core/test_namespace.py lines 121-150
before q_b.cancel_running_job(job.id). Preserve the existing ownership and
cancellation assertions after the job is Running.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 74d6be1c-4eb7-4c6a-a32d-f0ba3d6f615b
📒 Files selected for processing (54)
crates/taskito-core/examples/hello.rscrates/taskito-core/src/scheduler/maintenance.rscrates/taskito-core/src/scheduler/mod.rscrates/taskito-core/src/scheduler/poller.rscrates/taskito-core/src/scheduler/result_handler.rscrates/taskito-core/src/storage/diesel_common/archival.rscrates/taskito-core/src/storage/diesel_common/dead_letter.rscrates/taskito-core/src/storage/diesel_common/jobs.rscrates/taskito-core/src/storage/diesel_common/logs.rscrates/taskito-core/src/storage/mod.rscrates/taskito-core/src/storage/redis_backend/archival.rscrates/taskito-core/src/storage/redis_backend/dead_letter.rscrates/taskito-core/src/storage/redis_backend/jobs/errors.rscrates/taskito-core/src/storage/redis_backend/jobs/maintenance.rscrates/taskito-core/src/storage/redis_backend/jobs/query.rscrates/taskito-core/src/storage/redis_backend/jobs/state.rscrates/taskito-core/src/storage/redis_backend/logs.rscrates/taskito-core/src/storage/sqlite/tests.rscrates/taskito-core/src/storage/traits.rscrates/taskito-core/src/worker/cancel.rscrates/taskito-core/src/worker/remote.rscrates/taskito-core/src/worker/side_channel.rscrates/taskito-core/tests/namespace_scoping.rscrates/taskito-core/tests/rust/remote_tests.rscrates/taskito-core/tests/rust/storage_tests.rscrates/taskito-core/tests/rust/worker_tests.rscrates/taskito-java/src/dispatcher.rscrates/taskito-java/src/ffi_c.rscrates/taskito-java/src/queue/admin.rscrates/taskito-java/src/queue/inspect.rscrates/taskito-java/src/queue/logs.rscrates/taskito-java/src/queue/mod.rscrates/taskito-java/src/worker.rscrates/taskito-java/src/workflows/mod.rscrates/taskito-node/src/dispatcher.rscrates/taskito-node/src/queue/admin.rscrates/taskito-node/src/queue/inspect.rscrates/taskito-node/src/queue/logs.rscrates/taskito-node/src/queue/mod.rscrates/taskito-node/src/queue/workflows.rscrates/taskito-node/src/worker.rscrates/taskito-python/src/py_queue/inspection.rscrates/taskito-python/src/py_queue/mod.rscrates/taskito-python/src/py_queue/workflow_ops/fan_out.rscrates/taskito-python/src/py_queue/workflow_ops/lifecycle.rscrates/taskito-python/src/py_queue/workflow_ops/mod.rscrates/taskito-python/src/py_queue/workflow_ops/nodes.rscrates/taskito-server/src/dashboard/routes/jobs.rscrates/taskito-server/tests/attach_e2e.rscrates/taskito-tui/src/source/db.rsdocs/content/docs/shared/guides/operations/deployment.mdxsdks/java/src/test/java/org/byteveda/taskito/core/NamespaceScopingTest.javasdks/node/test/core/namespaceScoping.test.tssdks/python/tests/core/test_namespace.py
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
ByteVeda/taskito(manual)
These sites had the queue namespace in hand and passed None, so a scoped queue could resolve and act on a job in another namespace. Distinct from workflow-run scoping, which needs a schema migration.
The walks pushed an edge per dependency id before looking the adjacent job up, so scoping get_job skipped the foreign node but still shipped its id. An edge is now kept only once both endpoints resolved.
11 tasks
kartikeya-27
approved these changes
Aug 2, 2026
This was referenced Aug 2, 2026
Merged
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
2 participants
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.
Summary
get_job, the cancel paths, progress, logs, errors and the archive listing all took a bare id.reap_stale_jobs,reap_orphaned_jobsandcount_running_by_taskswept the whole cluster, so a scheduler could time out or retry another namespace's job and then record the outcome under its own.On #587
It needed no code. #596 had already scoped
stats,list_dead*,get_metricsandquery_task_logs, andtests/namespace_scoping.rsalready covered them. The one listing left unscoped waslist_archived, which #597 enumerates — it lands here, together with the docs note #587 asked for.Changes
Storage(traits.rs+diesel_common+ Redis + thedelegate!macro) — fourteen methods takenamespace: Option<&str>:get_job·request_cancel·is_cancel_requested·mark_cancelled·update_progress·cascade_cancel·get_job_errors·get_task_logs·get_task_logs_after·list_archived·list_archived_after·count_running_by_task·reap_stale_jobs·reap_orphaned_jobsA mismatch gets the same answer an unknown id gets — missing on a read, no effect on a mutation — so a caller scoped to one tenant learns nothing about ids outside it.
Nonestill addresses every namespace, so a single-tenant deployment is unaffected.Callers — the scheduler forwards its own namespace from
maintenance.rsandpoller.rs; every binding (Python, Node, Java),taskito-serverand the TUI forward the queue's.Two paths needed the namespace threaded in, not just forwarded.
CancelSignalsholds storage rather than a queue, so it now carries the worker's namespace. The executor side-channel's progress queue carried only(job_id, progress)— the dispatch already knew the namespace, so it rides along now, exactly as the log queue already did.Cross-namespace dependencies. Dependency validation does not require a dependency to share a namespace with its dependent, so
cascade_cancelfilters rather than assuming. Not rejecting them at enqueue: that is a behaviour change past this issue's done-when, and the filter is correct either way.Two methods have no namespace of their own —
job_errors(no column) and the Redis error list — so their scope comes from the job the rows belong to. Resolved before the connection is taken; a single-connection pool would deadlock on the second.Docs —
shared/guides/operations/deployment.mdxgains a "Namespaces as a tenancy boundary" section stating what the boundary now covers, that an unset namespace addresses every namespace, and that a namespace confines a correctly-configured process rather than authenticating one.Deferred, by decision
Workflow-run scoping. #597 lists paths that "load a run or node by an unscoped id". The
cancel_jobcalls in those paths already pass the queue namespace, so the job side is closed here. The run side is not:workflow_runshas no namespace column at all, so scoping it needs a schema migration plus a signature change across the workflow storage trait and its three backends. That is its own change, not a rider on this one. Recorded as a known limit in the deployment guide.Test plan
crates/taskito-core/tests/namespace_scoping.rs— 18 tests: every read reports not-found across the boundary, every mutation has no effect, and a scheduler in A never reaps or times out a job in B while an unscoped sweep still covers the clustertests/core/test_namespace.py,test/core/namespaceScoping.test.ts,NamespaceScopingTest.java)cargo test --workspacegreen; clippy--all-targets --all-featuresand fmt clean; default / postgres / redis all check./gradlew buildgreen including strict javadoccascade_cancelnamespace filter failsa_cascade_cancel_stops_at_the_namespace_boundaryNote for reviewers
cargo check --workspace --all-targetsdoes not compile#[cfg(feature = "redis")]test modules. The pre-commit clippy hook runs--all-featuresand found 23 further call sites after the workspace check was already clean.Summary by CodeRabbit
New Features
Documentation
Tests