Repository navigation
fix(tenant): update route manifest for two new tenant-scoped routes - #3529
Closed
PetrefiedThunder wants to merge 2 commits into
Closed
PetrefiedThunder wants to merge 2 commits into
PetrefiedThunder wants to merge 2 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: 294e3451-9541-4202-9c2d-66b7852fb233) |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 1 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Title check The title identifies the update for two new tenant-scoped routes. The wording is slightly broad because the change updates test expectations rather than the manifest itself, but it remains clear and r… Description check The description provides a detailed summary, identifies the affected routes, explains tenant-scoping evidence, and documents test limitations and verification. It does not follow the repository templa… Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Title check
-
Explanation
The title identifies the update for two new tenant-scoped routes. The wording is slightly broad because the change updates test expectations rather than the manifest itself, but it remains clear and related to the main change.
Full details: Description check
-
Explanation
The description provides a detailed summary, identifies the affected routes, explains tenant-scoping evidence, and documents test limitations and verification. It does not follow the repository template headings or include completed type and checklist items, but the core required information is present.
-
✨ Finishing Touches: 💡 2
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
🛠️ Fix failing CI checks: 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
Contributor
PR Blocker ChecklistHead SHA: Status: Ready
|
PetrefiedThunder
left a comment
PetrefiedThunder
left a comment
Collaborator
Author
There was a problem hiding this comment.
REVIEW: PASS grok 9466f30
Headless review on a fresh SSD worktree at 9466f30. The only file changed is tests/security/test_route_security_manifest.py.
- Counts: I counted docs/security/route-security-manifest.json (as regenerated in #3492, c9cbfbd) myself.
- enabled_operations = 254.
- By classification: tenant-scoped 201, public 30, global-sysadmin 16, workload-only 7.
- tenant_dual_session_csrf = 51, of which 23 are POST/PUT/PATCH/DELETE.
- These match the new 254/201/51/23. The two new ops are admin:GET:/v1/supplier/facilities/{facility_id}/removal-check and admin:DELETE:/v1/supplier/facilities/{facility_id}. Both are tenant-scoped and use tenant_dual_session_csrf.
- supplier_facilities_routes.py, GET removal-check at :137 and DELETE at :212. Both routes:
- depend on require_owner;
- take the tenant from TenantContext.get_tenant_context (a UUID) and return 400 if it's missing;
- return 400 for an invalid UUID;
- load with db.get() and return 404 if the facility is missing or
facility.tenant_id != tenant_id, so another tenant's facility gives 404 before any count or write; - filter every count query (lots, CTE events, funnel events, FTL scoping) on tenant_id and facility_id.
- DELETE specifically:
- It returns 409 if any lots, CTE events or funnel records are linked.
- Otherwise it deletes only the FTL scoping rows filtered by tenant_id and facility_id, then the facility row it has already verified.
- It can't touch another tenant's rows, and a missing record returns 404 with no write.
- Tests: the existing venv has llama_cloud, so nothing was installed. Both runs used
env -u DATABASE_URL -u SUPABASE_URL -u SUPABASE_SERVICE_ROLE_KEY, so no database was touched.- tests/security/test_route_security_manifest.py: 24 passed.
- services/admin/tests/test_supplier_facilities_routes_1342.py: 31 passed.
- Non-blocking, and from #3492 rather than this PR: the route tests cover no tenant (400), a bad UUID (400) and a non-owner (403). They have no cross-tenant 404 case and no 409 linked-data or successful-delete case for the DELETE. Worth adding as a follow-up.
CI at 9466f30: most checks are still pending. Vercel fails, which is main's broken build that #3524 fixes. Engine Review Pass and PR Blocker Checklist fail on every PR.
No merge (merge freeze).
Posted by Grok Bot for C.Lee. Review comments in C.Lee's repos are pre-approved under his standing rule, which was relayed by VS Code Lead and recorded in shared memory.
PetrefiedThunder
left a comment
PetrefiedThunder
left a comment
Collaborator
Author
There was a problem hiding this comment.
REVIEW: PASS muse 9466f30
- Counted docs/security/route-security-manifest.json at 9466f30: 254 enabled ops; tenant-scoped 201 / public 30 / global-sysadmin 16 / workload-only 7; tenant_dual_session_csrf 51, of which 23 mutations. All match the new asserts.
- The 2 added routes exist in services/admin/app/supplier_facilities_routes.py: GET /facilities/{facility_id}/removal-check (L137) and DELETE /facilities/{facility_id} (L212), both Depends(require_owner) + TenantContext tenant check, so the comment (SEL-421, #3492, owner-only, tenant-filtered) is accurate. +1 mutation = the DELETE.
- Diff touches only test counts and a comment; no data added.
- CI: Tenant Isolation & Auth Tests green. The repo-root security suite has 1 failure, test_check_supply_chain (baseline), outside this diff. Ingestion/Semgrep/pinning/Vercel failures are also outside this diff.
- Not run locally (repo deps not installed on this machine; nothing was installed).
PetrefiedThunder
added a commit
that referenced
this pull request
Oct 23, 2026
…3546 #3535) (#3552) * fix(epcis-export): return honest empty state instead of sample data Remove SAMPLE_EPCIS_EVENTS and the hardcoded FDA sample CSV. Tenants with no rows now get an empty eventList / header-only CSV, count 0, dataSource 'empty' and a 'No data yet' message. Tests are data-free. Drop the now-stale detect-secrets baseline entries for the sample hashes. Signed-off-by: Christopher Sellers <1@christophersellers.com> * fix(frontend): restore main build after SEL-410/SEL-414 ErrorState collision #3475 and #3479 merged back to back and both defined ErrorState. The surviving component lacked requestId/retrying used by #3479 callers, a test mock had a duplicate useQAHealth key, and suppliers/page.tsx used NO_DATA_YET without importing it. Vercel production builds for 03903ee and e7ee4a7 failed type check; prod is still on c7874cb. - ErrorState: optional requestId line, retrying disables Retry and shows Retrying..., support link, className kept on root - drop duplicate useQAHealth mock key - import NO_DATA_YET in suppliers page * test(epcis): drop SAMPLE_EPCIS_EVENTS patches; data-free empty-state and header-only FDA CSV assertions * ci: fix required gates on #3524 (pin copy-lint actions, inventory new workflows, allowlist false-positive secret, declare visual/capabilities knip entries) * chore(secrets): refresh .secrets.baseline (detect-secrets 1.5.0) * test(ingestion): make #2868 error-logging tests data-free to fix collection test_ingest_error_logging_2868.py imported _FakePersistence, _FakeSession, _client and _make_shipping_event from test_webhook_router_v2_1342.py, which the real-data-only cleanup removed, so collection failed on main. Drop the ingest-batch route cases that needed a fabricated event, test _failure_diagnostics directly with no payload, and keep the chain-verify case using MagicMock and a local app builder. Remove the stale detect-secrets baseline entry. Signed-off-by: Christopher <131207452+PetrefiedThunder@users.noreply.github.com> * fix(ci): scope suppliers no-data test; match uses: as YAML key in supply-chain checker * test(ingestion): restore data-free ingest_batch_failed route coverage for #2868 Mint TENANT_ID and KEY_ID with uuid4 at runtime. Re-add a POST /api/v1/webhooks/ingest failure test that uses caplog to check the traceback, tenant_id, event_count, source, error_type and correlation_id, and that the uuid4 canary lot code, product text and location never reach the log. A MagicMock persistence store raises to reach the handler's except block. Signed-off-by: Christopher <131207452+PetrefiedThunder@users.noreply.github.com> * fix(frontend): show QA-signal loading row on dashboard; align qa-health test with ErrorState copy * fix(tenant): update route manifest expectations for two new tenant-scoped routes * ci: list 9 missing workflows in inventory; stop action-pin matching 'statuses:' permissions * ci(frontend): allowlist visual regression files in Knip unused file guard * test(ingestion): fix-forward email_ingest principal stub and sel409 admin DB env Signed-off-by: Christopher Sellers <1@christophersellers.com> * fix(deps): bump pypdf to 6.21.2 for CVE-2026-115572/115585/115580 * test(ingestion): fix suite A collection (2868 persistence import) and drop SAMPLE_EPCIS_EVENTS expectations Signed-off-by: Christopher <131207452+PetrefiedThunder@users.noreply.github.com> * fix(ci): stop Head SHA Check Guard failing main on check_suite/workflow_run events Those events run in the default-branch context, so failed PR verdicts were attributed to main. Keep the failure commit status on the PR head SHA; only fail the job for pull_request events. * ci: clear detect-secrets false positive in vercel-preview-guard * test(ingestion): make 944/945 envelope test data-free after #3500 fixture removal * ci: refresh .secrets.baseline line numbers (detect-secrets 1.5.0, no new findings) * test(ingestion): capture ingest_batch_failed via dedicated logger; bump factory-local pypdf floor * ci: refresh .secrets.baseline after test change (drop stale entry) * fix: address CodeRabbit review on #3552 (checkout creds, retry a11y name, EDI envelope literals, duplicate test name) --------- Signed-off-by: Christopher Sellers <1@christophersellers.com> Signed-off-by: Christopher <131207452+PetrefiedThunder@users.noreply.github.com>
Contributor
Branch updated with mainMain was merged into Because merge commits created with GITHUB_TOKEN do not trigger workflows, a push to this branch or a manual CI re-run is needed for checks to run on it. Last synced: 2026-10-23T22:47:11.385Z |
Collaborator
Author
This branch was successfully deployed
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.
FROZEN: do not merge until Lead lifts the freeze, and then only with Grok's PASS and green CI.
Fixes Tenant Isolation & Auth Tests on main (03903ee). #3492 (SEL-421) regenerated
docs/security/route-security-manifest.json(254 enabled / 201 tenant-scoped) but did not update the hard-coded counts intests/security/test_route_security_manifest.py. The manifest itself is already correct, so this PR only updates the test expectations: 252→254, tenant-scoped 199→201, dual-session 49→51, dual-session mutations 22→23 (the new DELETE).The two new routes (services/admin/app/supplier_facilities_routes.py)
GET /v1/supplier/facilities/{facility_id}/removal-check(line 137)DELETE /v1/supplier/facilities/{facility_id}(line 212)Tenant-scoping evidence (both routes)
Depends(get_current_user)plusDepends(require_owner)(PermissionChecker("owner"), dependencies.py:627), which raisesE_NO_TENANT_CONTEXTwithout a tenant and resolves the role via the membership for the request's tenant, never a global role.TenantContext.get_tenant_context(db); 400 if missing.facility.tenant_id != tenant_id→ 404 (no cross-tenant existence leak).tenant_id == tenant_id(lots, CTE events, funnel events, FTL category rows).tenant-scoped,admin-postgres:jwt-tenant-or-session-membership-tenant, adaptertenant_dual_session_csrf.Tests
Local run of the two tests (
test_checked_in_manifest_matches_live_route_and_dependency_graph,test_dual_session_routes_record_cookie_tenancy_and_csrf_adapter) could not complete: the local venv is missingllama_cloud(import error at collection, unrelated to this change). Counts were verified directly against the checked-in manifest JSON (254 / 201 / 51 / 23). Relying on CI's Tenant Isolation & Auth Tests for the run.Note
Low Risk
Test-only expectation updates; no runtime, auth, or manifest file changes in this diff.
Overview
Aligns route security manifest release-gate tests with the manifest already regenerated in #3492 (SEL-421). No manifest or route code changes—only hard-coded expectations in
test_route_security_manifest.py.Counts: enabled operations 252→254, tenant-scoped 199→201, dual-session (
tenant_dual_session_csrf) routes 49→51, and dual-session mutations 22→23 (includes the new facility DELETE). Comments document the two added tenant-scoped admin operations:GET …/removal-checkandDELETE …/facilities/{facility_id}.Reviewed by Cursor Bugbot for commit 9466f30. Bugbot is set up for automated code reviews on this repo. Configure here.