Skip to content

P0: Close tenant data exposure in scheduling APIs and exports #254

Closed

Description

@tomqwu

Current Priority Decision

Owner direction, 2026-09-13: complete Church/Basketball business workflows and day-to-day operations for every role first; billing and platform readiness later.

Execution lane: phase: business-flow. Current business milestone: #289.
This section overrides older priority, start conditions, broad dependency order and launch estimates below. Keep the detailed technical recommendations where compatible. Execute only the NOW slice; do not expand a mixed issue into its whole production scope. Record completed slice receipts in #289; keep this issue open while retained later work remains.

NOW

Close actual tenant/access/export gaps along every BO journey in #289. Cover anonymous, member, same-tenant admin and foreign-tenant actors with real auth; two simultaneous fictional organizations must stay isolated. Preserve working lifecycle controls.

LATER

Broader release certification may wait; no reachable cross-tenant exposure on an active flow may wait. Reject unsupported export scope rather than leak more data.

Validation stays local. No CI checks, hosted reviewers or Ollama code review. Preserve tenant isolation, real member responses and atomic roster changes. A deferred feature is not permission to expose an unfixed vulnerability. No deployment, paid-provider activation or production sign-off is authorized by this reprioritization.

Before implementation, read #252 and #289; finish one role/work package with tests, local review and affected docs/assets. The earlier completion receipt below covers whole-issue closure, not a requirement to finish every deferred package before the business milestone.


Current Implementation Handoff

Prepared 2026-09-13 for a lower-cost builder at source 21a4a804aa57580451edded04736b1f51aef7e48.
No CI checks. All implementation validation and code review run locally.
Use the shared builder contract and this issue's work packages; no xhigh model or automatic model upgrade is required. This is a detailed recommendation, not a claim that a smaller model cannot make mistakes or that tests have passed.

Risk/review focus: High: cross-tenant exposure and export scope.
Start condition: Start only the NOW work package defined above. Use its local business prerequisites, not the entire older platform-release dependency list.

Source of Truth and Current State

Do not infer authorization from router registration, the warning listener or mocked unit auth. Solutions export constructs solution_obj before person filtering, and team scope has an empty implementation. PR #251 remains separate open export work.

This handoff supersedes stale implementation statements in the background below. Preserve existing successful behavior and tests. Recheck the current branch before editing; the baseline is a source pointer, not permission to discard newer changes.

Dependencies and Ownership

Recommended Decisions

  1. Classify every registered method/path with public-token exception, self-service, member-read or admin-write policy. Treat login/bootstrap/token redemption as explicit exceptions rather than blindly requiring JWT everywhere.
  2. Retain HTTPBearer's current missing-token behavior; invalid JWT is 401, insufficient role/explicit foreign org is 403, and a resource looked up inside an authorized tenant is 404 when absent. Choose one exact expectation per route in tests, not broad status ranges.
  3. Derive tenant from the authenticated actor; filter child rows through a tenant-scoped parent before loading/serializing them. For person scope, filter assignee identities too, not just whole events.
  4. Filter a single authorized export dataset before JSON/CSV/ICS/PDF serialization. Until implemented, reject unsupported team/format combinations explicitly; never silently fall back to organization-wide output.

Small Work Packages

Each item is one reviewable slice, not permission for one giant PR. Add the failing regression first; finish code, tests and affected docs for that slice together. Leave this issue open until all packages and original acceptance criteria are satisfied or explicitly revised by the owner.

  • 254.1: Create a tracked route/auth/test matrix from app registration and current tests; add two-tenant negative tests for one route family at a time.
  • 254.2: Patch events/conflicts, then availability ownership, then solution read/write/delete/export. Keep UI/dependency call signatures synchronized.
  • 254.3: Repair export filtering before serialization and verify content in every supported format; coordinate Sprint 4 follow-up — implement ICS format for /solutions/{id}/export #251 rather than duplicate its work.
  • 254.4: Refresh the local OpenAPI snapshot, callers/examples and schema compatibility notes; add a local matrix-coverage check for future routes.

Required Regression Cases

These are specifications for tests to add/retain, not claimed execution results. Each new negative case must assert unchanged unauthorized state and zero forbidden side effects.

  • T254-01: Anonymous/invalid JWT/volunteer/own admin/foreign admin against each operation -> exact matrix response, no unauthorized rows or writes.
  • T254-02: Foreign child under an own-tenant parent identifier and own child under a foreign parent -> controlled denial, no leaked metadata.
  • T254-03: Person export from a multi-assignee event -> only allowed identities; team export -> exact team membership or explicit unsupported error.
  • T254-04: Pagination/search/stream/export errors cannot expose foreign counts, names, timestamps or provider effects.

Local Commands and Evidence

Existing targeted commands (paths checked against the audit source; run only after the stated safe preflight):

poetry run pytest tests/api/test_multi_tenant.py tests/api/test_calendar_auth.py -q
poetry run pytest tests/integration/test_tenancy_guard.py tests/integration/test_calendar.py tests/integration/test_solutions.py -q
poetry run pytest tests/contract -q

Also run the shared formatting/lint/touched-type/unit/full-suite and local review protocol from #252 for the final pushed revision. Add new targeted tests to these commands when implemented. Run API and browser tiers in separate processes. Native, PostgreSQL, image, provider and operator drills require their explicit environment; an unavailable tool/target is blocked/not run, never a pass.

Schema and Compatibility

Auth/filter fixes normally need no schema migration. Any new route/request policy must update the contract snapshot and trigger #191 compatibility verification.

Stop Conditions

Stop a slice if changing visibility requires a new product access policy, or if #251 modifies the same export path. Reconcile the contract before editing both versions.

After two failed focused repair attempts without new diagnostic evidence, stop the affected package and post the exact failure, commands, suspected boundary and needed decision. Do not silently broaden scope, weaken tests or upgrade models. A fresh local reviewer checks: Inspect every serializer input, child-parent join and public exception; require real-JWT zero-side-effect evidence rather than auth-mocked unit tests.

Completion Receipt

  • Work-package and regression IDs above map to changed files and actual results.
  • Commands, versions, dates, pass/fail/skip/not-run counts, logs/screenshots and tested head/base SHAs are linked.
  • A separate local review records findings and resolution; self-review is labeled if used and is not misrepresented as independent review.
  • Affected docs/README/playbooks/screenshots and dependency/roadmap status are reconciled, not left as unnamed follow-ups.
  • If implementation is authorized through PR/merge, GitHub reports mergeable and the shared local-evidence requirements are met; reviewer agents never merge.
  • No hosted CI check, status attestation, Ollama reviewer, live provider action, deployment, real-data purge or store submission was introduced by implication.

Copyable Builder Prompt

First read this issue's Current Priority Decision and #289. Run only its NOW slice.
If this issue is deferred, report that state instead of starting the older package list.
Implement the next ready work package in tomqwu/SignUpFlow issue #254.
Read its Current Implementation Handoff and #252 Builder Handoff Contract first.
Inspect current source and preserve newer/unrelated changes. Start with the
package's failing regression, then complete code, local tests, local review and
affected docs/assets together. Do not skip acceptance or invent passing evidence.
No CI checks or Ollama code review. Do not deploy, activate providers, purge real
data or submit to stores. Stop and report unmet prerequisites or policy decisions.
Record the package/test IDs and exact reviewed/tested source SHAs before claiming done.

Earlier Audit and Acceptance Context

Current policy (2026-09-13)

No CI checks. Everything is validated locally. This includes code review,
formatting, lint, type checks, migrations, all test tiers, security scans,
artifact checks and mobile validation. Do not add hosted jobs, required CI
statuses, synthetic success checks or an Ollama reviewer. GitHub is for source,
PRs, issues and publication, not validation.

Record commands, environment, results, limitations and reviewed head/base SHAs.
Builders merge only with completed local evidence and GitHub mergeability;
reviewer agents never merge. Real staging/provider/device acceptance remains
required where applicable, driven by authorized local operator tools.
Historical evidence and older comments do not override this policy.

Parent roadmap: #252

Priority: P0, blocks core production pilot. Phase: A. Suggested owner: Backend/security. Original estimate (superseded; re-estimate remaining work): 5-8 engineering days across small PRs.

Historical audit evidence (recheck against current source)

api/routers/events.py:124 and api/routers/solutions.py:45 allow unauthenticated list/read with optional org filters. api/routers/availability.py:25 exposes availability/time-off/rrule writes; api/routers/conflicts.py:33 exposes person/event conflict data. Solution export/manual-create/delete also lack auth. Disposable checks returned an event without auth (200) and created another synthetic person's time-off (201). The tenancy listener api/utils/tenancy_guard.py only warns by default and accepts broad ID/email heuristics; it is not authorization.

Baseline: GitHub main 214e3f3f17a582d5f9b2063be6872ea2b1d25714, audited 2026-09-09. Findings concern synthetic reproduction or source inspection, not a claim of live exploitation.

Implementation plan

  1. Inventory every registered route and classify: public onboarding/token redemption, authenticated self-service, tenant member read, or tenant admin write. Record required organization/ownership predicates, including exports and child models without org_id.
  2. Deliver small PRs for event/conflict reads, availability ownership, and solution read/export/write. Derive tenant from the authenticated person, call verify_org_member, and join child rows through the authorized parent.
  3. Apply person/team export scope before building every serialization format. JSON currently builds solution_obj before scope filtering; team scope currently does nothing. Reject unsupported scope instead of silently exporting more data.
  4. Update web calls, Flutter callers, CLI/API examples and tests that rely on anonymous access. Coordinate with existing ICS export PR Sprint 4 follow-up — implement ICS format for /solutions/{id}/export #251 rather than duplicate it; do not merge it before export authorization and scope tests pass.
  5. Add a route coverage test so newly registered business routes cannot lack an explicit auth policy; retain narrowly documented public token flows.

Acceptance criteria

  • No anonymous tenant data reads/writes; explicit missing/wrong org_id, guessed IDs, and foreign parent/child combinations return controlled denials.
  • Members cannot read another person's private availability unless an explicit authorized admin policy permits it.
  • Exports in all supported formats contain only authorized rows; person/team scopes never include unrelated identities.
  • Unauthorized requests cause zero database writes, queued tasks or information-rich errors.
  • Every exposed route appears in the reviewed auth matrix; the listener is not used to substitute for tenant filtering.

Dependencies

None; start immediately.

Validation

Add parametrized tests/api negative cases with real JWTs and at least two organizations. Run affected web/browser flows and the full suite. Add PostgreSQL foreign-key/tenant relationship regression cases.

Whole-repository audit scope (2026-09-13)

Baseline: 21a4a804aa57580451edded04736b1f51aef7e48. This addendum assigns full-scope follow-through; it is not a new test pass or production sign-off. No CI checks; all review and validation runs locally.

Own the complete registered API authorization matrix, not just the initially reported endpoints. Inventory api/main.py, all 24 router modules (including explicitly unmounted webhooks), schemas, direct web delegates, child/parent joins, exports and streams. Reconcile the OpenAPI contract, README/examples (#278), mobile client (#191) and auth tests (#281). Current solutions export still builds a minimal solution object before person filtering and leaves team filtering as pass. Preserve PR #251 as unmerged until authorization and per-format content tests pass. Link each route policy to real-JWT two-tenant negative tests; grep or warning-only tenancy checks are not proof.

Keep evidence and disposition synchronized with master roadmap #252 and documentation ledger #277. Close only after the remaining acceptance criteria have linked local results; a planning/audit note is not completion.

Activity

  1. added
    bugSomething isn't working
    phase: business-flowCurrent Church/Basketball business-flow work; execute only the active slice in each issue.
    on Sep 9, 2026
  2. tomqwu commented on Sep 14, 2026

    @tomqwu
    OwnerAuthor

    NOW-slice completion receipt: PR #294

    Merged 2026-09-13 as f6da81a73b1a670c106c33d8fadc72eb593e95d6 after GitHub reported PR #294 MERGEABLE and CLEAN.

    Work packages

    • 254.1: Added api/route_auth_policy.py and a live route-table guard covering every mounted API operation.
    • 254.2: Secured event/conflict, availability ownership, and solution read/write/delete/lifecycle paths; synchronized direct web callers and generated Dart security metadata.
    • 254.3: Applied org/person/team scope before JSON, CSV, and PDF serializer inputs; scoped persisted fairness identities and corrupted cross-tenant child rows; unsupported solution ICS returns explicit 501.
    • 254.4: Refreshed OpenAPI snapshot, generated mobile client, README, authorization/security/playbook docs, documentation ledger, and roadmap.

    Regression IDs

    • T254-01: Real JWT anonymous, invalid, member, same-tenant admin, and foreign-admin behavior covered in tests/api/test_scheduling_tenant_boundaries.py; protected route matrix checked in tests/unit/test_api_route_auth_policy.py.
    • T254-02: Foreign team/resource/person children and intentionally corrupt foreign-person assignments are denied or filtered with no writes or identity leakage.
    • T254-03: Person and team exports contain exactly authorized assignees and fairness counts; org scope excludes foreign identities; CSV and captured PDF inputs are content-checked.
    • T254-04: Search/list counts, guessed IDs, assignment snapshots, stats, SSE existence probes, and export errors are tenant-safe.

    Local evidence

    • Base 73b5a9984163821d60b726aad845724c070384af; tested/pushed head e9395eeb0409c924f879d452504606d44234407f.
    • make test-all: 1,497 passed, 21 intentionally skipped (420/21 unit, 469 API, 16 CLI, 326 integration, 232 web, 1 contract, 33 Playwright).
    • Playwright included Church and Basketball at 360px and 1440px plus the full-loop journey.
    • make test-mobile: 65 passed.
    • Black: 335 files clean. Ruff on api, tests, and changed web routers: passed. git diff --check: passed.
    • Touched-module mypy: 140 legacy SQLAlchemy/untyped-route errors, versus previously verified 227 on main; no new mypy regression, but this is not a clean mypy claim.
    • Secret review found only policy prose and synthetic test credentials.

    Local review

    Builder self-review, not independent approval. It found and fixed: direct web handler argument binding failures; corrupt foreign-person child leakage; unscoped persisted fairness IDs; and an inaccurate Twilio callback classification. Focused post-fix suite: 63 passed.

    Documentation and assets

    README, authorization/security docs, playbook boundaries, documentation status, index, roadmap, OpenAPI snapshot, and generated mobile client are reconciled. No web layout changed, so existing README screenshots remain applicable and were not represented as newly captured evidence.

    Retained work

    Keep #254 open for the explicitly deferred PostgreSQL foreign-key/tenant relationship and broader platform release evidence under #253/#260. No reachable business-flow tenant exposure from this slice is being deferred. No CI/Ollama/provider/deployment action occurred.

  3. tomqwu commented on Sep 14, 2026

    @tomqwu
    OwnerAuthor

    BO-10 calendar acceptance is merged in PR #305. Personal download and token feeds now exclude drafts, declined assignments, deleted events, and foreign-tenant child rows; UTC storage renders in the member timezone across DST and moved entries keep one UID. Local church/basketball browser coverage runs at 360px and 1440px.

    This is a completed slice of #254. Keep #254 open for its remaining end-to-end workflow scope and external calendar-client acceptance.

  4. tomqwu commented on Sep 14, 2026

    @tomqwu
    OwnerAuthor

    BO-12 source receipt: PR #307 merged as 35c4bc4124cdb3f014ad9304b117c3d0550b6ffb.

    Implemented and verified:

    • all production API access tokens and browser sessions carry org_id and reload only an active matching membership
    • signup, login, password change, refresh, and invitation acceptance use tenant-bound access credentials
    • Church and Basketball coexist in one live local server; each administrator sees only its own directory
    • every declared Church and Basketball scheduling qualification runs as a volunteer at 360px and 1440px and cannot enter the admin surface, invite, publish, read a foreign person, or mutate a peer's availability
    • tests/security/ is restored to the supported make test-all API tier
    • README, roadmap, authorization/testing docs, coverage manifest, both domain guides, and instruction files are reconciled

    Local evidence on tested head d6848fd8bb09acb3778ff2f456aacd78d09026b4 against base dbafb60008c69d4ba39580dff23bac4e203e957d:

    • initial TDD: 7 failed / 8 passed before implementation
    • Black and Ruff passed
    • focused dependency mypy passed; broader router mypy still has 85 recorded legacy errors and was not called green
    • focused auth/authorization: 169 passed
    • make test-all: 1,600 passed, 21 skipped (includes 54 Playwright)
    • make test-mobile: 65 passed
    • phone/desktop tenant-directory and member-boundary screenshots inspected without overlap/clipping
    • self-review recorded in PR; no CI or Ollama review used
    • GitHub reported MERGEABLE/CLEAN before builder merge

    This completes the BO-12 slice only. Keep the parent issue open where its retained domain, usability, release, provider, PostgreSQL, or whole-application acceptance remains.

  5. tomqwu commented on Sep 14, 2026

    @tomqwu
    OwnerAuthor

    Final completion receipt

    Closed against current main 2c25f99 after reconciling merged PRs #294, #305, and #307 with the current acceptance.

    • 254.1 / T254-01: the registered-route authorization matrix and real-JWT tenant tests cover anonymous, invalid, member, same-tenant admin, and foreign-admin behavior.
    • 254.2 / T254-02: event, conflict, availability, solution, child-parent, stream, pagination, and mutation paths resolve through tenant-scoped parents and produce controlled denials without foreign writes.
    • 254.3 / T254-03 and T254-04: JSON/CSV/ICS/PDF and personal/team scoping filter authorized data before serialization; calendar feeds exclude drafts, declines, deleted events, and foreign children.
    • 254.4: OpenAPI snapshot and route-matrix coverage tests are current.

    Current exact-head local evidence: make test-all reported 1,710 passed, 21 explicitly skipped, 0 failed, 0 errors across unit/API/security/CLI/integration/web/contract/Playwright. Report: test-artifacts/local-validation/20260914T203506.257431Z-27095-14d9ac21/report.json, tested source d632d13, merged unchanged as PR #316 into current main. Local self-review found no unresolved tenant/export acceptance gap. No CI, Ollama review, provider, deployment, or real-data action was used.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingphase: business-flowCurrent Church/Basketball business-flow work; execute only the active slice in each issue.tests

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions