feat: add tenant-safe learning kernel - #4
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
4eddbeb to
e7aa3eb
Compare
|
@opencode-agent Please review the current exact head |
|
@opencode-agent Re-review current exact head |
7c7a846 to
b832ed6
Compare
|
@opencode-agent Please review the current exact head |
seonghobae
left a comment
seonghobae
left a comment
There was a problem hiding this comment.
Current-head review request for 8490f15. Re-verify the exact-head diff, required Checks, tenant/security boundaries, and any valid review findings after the shared Rust component-install fix. COMMENT only; no approval or protected-merge bypass is requested.
|
@opencode-agent Review only exact current head b5b4f6f against base 7ba5864; do not update, approve, merge, or bypass protections. Focus on the migration-once smoke fix and learner registration regression risk. |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
@opencode-agent Review only exact current head 9de3bcf against base 7ba5864; do not update, approve, merge, or bypass protections. Re-check the non-superuser/NOBYPASSRLS CI role, disposable rollback/reapply evidence, and added customer/self-sponsored/multi-tenant tests. Reassess the repeated-enrollment point against PR4's learner-only scope. |
|
@opencode-agent Review only exact current head fe1b890 against base 7ba5864; do not update, approve, merge, or bypass protections. Re-check the final docs-only head after the non-superuser/NOBYPASSRLS and rollback-evidence fixes; prior 9de3bcf review request is stale. |
…ent' into feat/external-learner-kernel # Conflicts: # docs/product-technical-gap-baseline.md
|
@OpenCode review only the exact current head 4ce89f9 against base 61f9d8c. Re-check the least-privilege PostgreSQL role, rollback/reapply rehearsal, tenant-affiliation coverage, merge-base reconciliation, and current workflow. Leave an independent review or approval for this exact head only. Do not update code, merge, self-approve, bypass protections, or treat queued Checks as proof. |
| sqlx::query("SELECT set_config('app.tenant_id', $1, true)") | ||
| .bind(tenant_id.to_string()) | ||
| .execute(&mut *transaction) | ||
| .await?; |
There was a problem hiding this comment.
🟨 Learner registration endpoint has no authentication or tenant authorization
The POST /v1/tenants/{tenant_id}/learners handler (crates/lms_kernel/src/bin/lms_api.rs:95-163) accepts the target tenant_id directly from the URL path and then sets the row-level-security context from that same untrusted value (set_config('app.tenant_id', $1, true) at crates/lms_kernel/src/bin/lms_api.rs:116-119). There is no authentication middleware and no check that the caller is entitled to act on the supplied tenant. Because the caller both chooses the tenant and supplies the RLS context, the PostgreSQL RLS boundary provides no protection against a malicious caller: anyone who can reach the service can create/enumerate learner memberships in any tenant (and, via the global login_identity_reference/learner_profile lookups, learn the stable learner_id for a guessed external identity).
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
What changed
NOSUPERUSER NOBYPASSRLSapplication role rather than the PostgreSQL provisioning superuserWhy
This is the smallest executable slice for issue #3. It makes the documented learner/identity/authority boundary enforceable before adding HTTP and external-provider adapters.
Validation
4ce89f96310d41eaa3b12cf4afa12c36a72462ddcargo +1.97.1 fmt --all -- --checkcargo +1.97.1 test --workspace --all-targets --locked(5 tests)cargo +1.97.1 clippy --workspace --all-targets --locked -- -D warningsRUSTDOCFLAGS='-D warnings' cargo +1.97.1 doc --workspace --no-deps --lockedNOSUPERUSER NOBYPASSRLSrole: migration, health, learner201, duplicate409, rollback, and reapplyCloses #3 partially; the end-to-end enrollment journey and provider adapters remain follow-up work. Repeated-enrollment journey coverage remains in scope for the stacked enrollment slice because this PR has no enrollment relation or endpoint.