worker: reject wrong-org, expired and replayed WorkerLeases - #45
Merged
Conversation
📝 WalkthroughWalkthroughAdds a bounded signed WorkerLease flow for persistent no-op jobs, including canonical HMAC tokens, PostgreSQL-backed issuance and atomic completion, dedicated RLS roles, Issue ChangesPersistent no-op WorkerLease
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
tests/integration/test_worker_lease.py (1)
399-403: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse raw strings for the anchored
match=regexes (Ruff RUF043).The
pytest.raises(..., match="^work not available$")patterns are regexes but aren't raw strings; Ruff RUF043 flags every occurrence (lines 401, 466, 532, 557, 574, 599, 632, 663, 928, 1002). Behavior is unchanged, but the raw-string form documents regex intent and clears the lint.♻️ Example (apply to each flagged line)
- match="^work not available$", + match=r"^work not available$",🤖 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 `@tests/integration/test_worker_lease.py` around lines 399 - 403, Update every flagged pytest.raises match argument in the test module to use raw string literals for the anchored regex patterns, including the occurrence in issue_noop_lease and the other listed locations. Preserve the existing regex text and test behavior while satisfying Ruff RUF043.Source: Linters/SAST tools
🤖 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 `@docs/decisions/0029-bound-first-worker-lease-to-persistent-no-op-jobs.md`:
- Around line 44-50: Make ServiceActor authorization a prerequisite for worker
database activation alongside the exact-job signed WorkerLease; otherwise keep
activation deferred and fail-closed. Update
docs/decisions/0029-bound-first-worker-lease-to-persistent-no-op-jobs.md
(44-50), CONTEXT.md (45 and 317-328), README.md (21-25 and 109-123), and
docs/decisions/README.md (30) to require the registered least-privilege
ServiceActor, include it in WorkerLease scope, and remove claims that the
ServicePrincipal-only carrier or active lease is sufficient.
- Line 12: Update the ADR metadata date in the decision record to the actual
acceptance date; if it has not yet been accepted, remove or change the accepted
status instead of retaining a future date.
In `@README.md`:
- Around line 85-88: Update the README status section to remove the stale
“WorkerLease: NOT_ACTIVE” claim. Align the WorkerLease wording with the
activated Issue `#17` persistent no-op seam and PostgreSQL authority, or
explicitly qualify NOT_ACTIVE as applying only to the full production carrier.
---
Nitpick comments:
In `@tests/integration/test_worker_lease.py`:
- Around line 399-403: Update every flagged pytest.raises match argument in the
test module to use raw string literals for the anchored regex patterns,
including the occurrence in issue_noop_lease and the other listed locations.
Preserve the existing regex text and test behavior while satisfying Ruff RUF043.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 352c1c62-c670-48d5-b8a0-402d49e7c4f0
📒 Files selected for processing (30)
CONTEXT.mdREADME.mdapplications/worker.pydocs/decisions/0029-bound-first-worker-lease-to-persistent-no-op-jobs.mddocs/decisions/README.mdengine/persistence/__init__.pyengine/persistence/configuration.pyengine/persistence/database.pyengine/persistence/role_guard.pyengine/persistence/schema_security_manifest.yamlengine/persistence/worker_jobs.pyengine/supply/__init__.pyengine/supply/jobs.pyeval/catalogs/security-catalog.schema.jsoneval/catalogs/security-invariants.yamlinfra/postgres/init/10-security-roles.shmigrations/versions/20260722_0006_worker_lease_noop.pyscripts/provision_database_roles.pyscripts/validate_security_catalog.pytests/catalog/test_validate_security_catalog.pytests/integration/conftest.pytests/integration/test_migrations.pytests/integration/test_postgres_harness.pytests/integration/test_worker_lease.pytests/process/test_processes.pytests/unit/test_database_configuration.pytests/unit/test_database_harness_contract.pytests/unit/test_schema_security_manifest.pytests/unit/test_worker_job_authority.pytests/unit/test_worker_lease.py
| # 0029. Bound the first WorkerLease to persistent no-op jobs | ||
|
|
||
| - Status: accepted | ||
| - Date: 2026-07-22 |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="docs/decisions/0029-bound-first-worker-lease-to-persistent-no-op-jobs.md"
echo "== file metadata =="
wc -l "$file"
echo
echo "== lines 1-25 =="
cat -n "$file" | sed -n '1,25p'
echo
echo "== nearby decision docs dates/status patterns =="
rg -n "^Date:|^Status:" docs/decisions/0028* docs/decisions/0029* docs/decisions/0030* 2>/dev/null || trueRepository: stone16/context-engine
Length of output: 1500
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
p = Path("docs/decisions/0029-bound-first-worker-lease-to-persistent-no-op-jobs.md")
text = p.read_text()
for i, line in enumerate(text.splitlines(), 1):
if i <= 25 or "accepted" in line.lower() or "Date:" in line or "Status:" in line:
pass
print("contains accepted:", "accepted" in text.lower())
print("contains 2026-07-22:", "2026-07-22" in text)
print("contains 2026-07-21:", "2026-07-21" in text)
PYRepository: stone16/context-engine
Length of output: 237
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Capture the exact snippet around the date and status for the ADR.
nl -ba docs/decisions/0029-bound-first-worker-lease-to-persistent-no-op-jobs.md | sed -n '1,30p'Repository: stone16/context-engine
Length of output: 200
Use the actual acceptance date
- Date: 2026-07-22 is in the future relative to this review. Update it to the real acceptance date, or drop accepted until the ADR is actually accepted.
🤖 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 `@docs/decisions/0029-bound-first-worker-lease-to-persistent-no-op-jobs.md` at
line 12, Update the ADR metadata date in the decision record to the actual
acceptance date; if it has not yet been accepted, remove or change the accepted
status instead of retaining a future date.
Comment on lines
+44
to
+50
| The token binds the Issue #17 fields that exist: Organization, durable job, | ||
| registered `ServicePrincipal` binding, workload, worker audience, the exact | ||
| persistent no-op operation, issued-at, expiry, and nonce. The current durable | ||
| job row is the authority for those same values. A valid signature alone never | ||
| authorizes work. This is a bounded `ServicePrincipal` receiver binding, not the | ||
| full canonical `ServiceActor`: Issue #17 has no source/allowed-operation set or | ||
| Policy Epoch, so it neither publishes nor claims that broader ActorContext arm. |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Require the registered ServiceActor before activating this worker database boundary.
The repository rule requires worker database access to use a registered least-privilege ServiceActor together with the exact-job signed WorkerLease. These changes instead activate a ServicePrincipal-only carrier and explicitly state that it is not the canonical ServiceActor. Either add the required trusted actor context and enforcement, or keep this activation deferred/fail-closed.
docs/decisions/0029-bound-first-worker-lease-to-persistent-no-op-jobs.md#L44-L50: make ServiceActor authorization a prerequisite.CONTEXT.md#L45-L45: include the required ServiceActor in WorkerLease scope.CONTEXT.md#L317-L328: do not describe the ServicePrincipal-only carrier as sufficient.README.md#L21-L25: revise the active-lease claim.README.md#L109-L123: align the Issue#17activation description with the required actor boundary.docs/decisions/README.md#L30-L30: include the ServiceActor prerequisite in the accepted baseline.
As per coding guidelines: “Worker database access must use a registered least-privilege ServiceActor and a server-minted signed WorkerLease with exact durable-job binding; never impersonate the triggering user or treat ingestion authority as delivery authority.”
📍 Affects 4 files
docs/decisions/0029-bound-first-worker-lease-to-persistent-no-op-jobs.md#L44-L50(this comment)CONTEXT.md#L45-L45CONTEXT.md#L317-L328README.md#L21-L25README.md#L109-L123docs/decisions/README.md#L30-L30
🤖 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 `@docs/decisions/0029-bound-first-worker-lease-to-persistent-no-op-jobs.md`
around lines 44 - 50, Make ServiceActor authorization a prerequisite for worker
database activation alongside the exact-job signed WorkerLease; otherwise keep
activation deferred and fail-closed. Update
docs/decisions/0029-bound-first-worker-lease-to-persistent-no-op-jobs.md
(44-50), CONTEXT.md (45 and 317-328), README.md (21-25 and 109-123), and
docs/decisions/README.md (30) to require the registered least-privilege
ServiceActor, include it in WorkerLease scope, and remove claims that the
ServicePrincipal-only carrier or active lease is sufficient.
Source: Coding guidelines
Comment on lines
+85
to
+88
| 健康响应中的 `runtime_delivery: NOT_ACTIVE` 表示默认进程没有生产认证入口。worker | ||
| 输出中的 `job_behavior: NOT_ACTIVE` 特指默认 CLI 尚未配置生产签名密钥来源、queue/job | ||
| loop 或真实 ingestion/publication handler；Issue #17 的 persistent no-op 应用 seam 与 | ||
| PostgreSQL authority 已激活并由 integration suite 调用。当前数据库测试证明 `compose.yaml` 固定的 |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove the stale WorkerLease: NOT_ACTIVE claim.
This section activates Issue #17, but README.md line 163 still says WorkerLease is not active. Update the stale status or qualify it as referring only to the full production carrier.
🤖 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 `@README.md` around lines 85 - 88, Update the README status section to remove
the stale “WorkerLease: NOT_ACTIVE” claim. Align the WorkerLease wording with
the activated Issue `#17` persistent no-op seam and PostgreSQL authority, or
explicitly qualify NOT_ACTIVE as applying only to the full production carrier.
This was referenced Jul 22, 2026
9 tasks
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
1 participant
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
Verification
Closes #17
Summary by CodeRabbit