Commit Graph

9 Commits

Author SHA1 Message Date
chihlasm
758cd61621 fix: propagate account_id through all write paths missing NOT NULL coverage
Service layer (production code):
- branch_manager: set account_id on SessionBranch (root + fork) and ForkPoint
  from session.account_id; load session in create_fork for this purpose
- handoff_manager: set account_id on SessionHandoff from session.account_id
- ai_suggestions endpoint: set account_id on AISuggestion from current_user
- steps endpoint (/feedback): set account_id on StepRating from current_user
- ratings endpoint: set account_id on StepRating from current_user

Test infrastructure:
- conftest.py: seed PLATFORM_ACCOUNT_ID (00000000-...-0001) account after
  Base.metadata.create_all so global categories and gallery items have a valid FK
- test_rls_isolation: add _ensure_rls_schema fixture that runs
  'alembic upgrade head' before module tests — previous function-scoped
  test_db fixtures drop the schema, leaving the RLS tests with no tables
- test_branding: create Account before User in helper functions
- test_admin_gallery: set account_id=PLATFORM_ACCOUNT_ID on Tree/ScriptTemplate
- test_public_templates: set account_id=PLATFORM_ACCOUNT_ID on Tree,
  ScriptTemplate, TreeCategory
- test_resolution_outputs: set account_id=session.account_id on
  SessionResolutionOutput
- test_analytics_phase5: set account_id on PsaPostLog
- test_draft_trees: replace account_id=None with PLATFORM_ACCOUNT_ID in
  migration default test (NOT NULL now enforced)
- test_maintenance_schedules: set account_id on other_tree
- test_save_session_as_tree: set account_id on all 5 Session() constructors

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-04-11 04:24:36 +00:00
chihlasm
b3dba57bc5 feat: tenant isolation Phase 0 — app-layer filters, UUID audit, CI gate (#132)
* docs: add tenant data isolation design spec

Complete architecture plan for multi-tenant data isolation across
all layers (PostgreSQL RLS, application-layer filtering, schema
migration, testing strategy, and phased rollout checklist).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* docs: add background job isolation policy to tenant isolation spec

Documents policy for all 5 existing background jobs:
- Knowledge Flywheel and PSA Retry flagged for account_id threading
- Chat Retention already follows correct pattern (model for others)
- Maintenance Schedule Firing needs account_id in queries + Session creation
- AI Conversation Expiry approved as cross-tenant with justification

Adds approved cross-tenant query registry and Phase 2 checklist items.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* docs: add tenant isolation Phase 0 implementation plan

8 tasks covering: CRITICAL copilot hotfix, tenant_filter() helper,
get_tenant_context dependency, analytics/category/AI session gap fixes,
full UUID endpoint audit, TargetList dead code audit, teams orphan
check, and CI grep check for missing tenant filters.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* feat: add tenant_filter() helper and get_tenant_context dependency

tenant_filter(model, account_id) is the canonical app-layer tenant
scoping expression. Every query on a tenant table must use it.
build_tree_access_filter and build_step_visibility_filter updated
to call tenant_filter() internally for the account_id match.

get_tenant_context is a FastAPI dependency that returns account_id
or raises 403 if the user has no account — prevents raw access to
current_user.account_id and centralises the null check.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix: scope analytics/flows/{tree_id} to requesting account

Any authenticated user could read flow analytics (session counts,
completion rates, CSAT) for any tree UUID. Now returns 404 if the
tree doesn't belong to the requesting account.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix: scope category tree_count to requesting account

tree_count on GET /categories/{id} was including trees from all
accounts, leaking cross-tenant row counts.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix: restrict AI session search to current user only

Search endpoint used OR(user_id, account_id), exposing other users'
problem_summary and problem_domain within the same account. Sessions
are user-scoped only — cross-user access requires explicit escalation
or sharing. List and search endpoints now behave consistently.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix: add ownership check and 404 responses to ai-sessions endpoints

Cross-tenant isolation audit found:
- retry-psa-push had NO ownership check (CRITICAL) — any user could retry any session's PSA push
- save_task_lane used db.get() without ownership filter, returned 403 revealing existence
- get_session returned 403 instead of 404 for unauthorized access
- stream_documentation returned 403 instead of 404

All now use query-level user_id filtering and return 404 to avoid revealing existence.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix: return 404 instead of 403 for cross-tenant session access

All session endpoints (get, update, complete, scratchpad, variables, export,
ticket-link) now return 404 instead of 403 when a user tries to access
another user's session. This prevents confirming existence of resources
across tenant boundaries.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix: return 404 instead of 403 for cross-tenant tree access

get_tree and update_tree now return 404 when a user cannot access a tree
(private tree from another account). Prevents confirming resource existence
across tenant boundaries.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix: return 404 instead of 403 for cross-tenant step access

get_step_or_404 now returns 404 when can_view_step or can_edit_step fails,
preventing confirmation of step existence across tenant boundaries.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix: return 404 instead of 403 for cross-tenant upload access

get_upload_url and delete_upload now return 404 when the upload belongs to
a different account/user, preventing resource existence confirmation.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix: return 404 instead of 403 for cross-tenant share access

revoke_share and create_share now return 404 when the caller is not the
owner, preventing resource existence confirmation across users.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix: return 404 instead of 403 for cross-team tree access in maintenance schedules

_get_tree_or_403 now returns 404 when the user's team does not match,
preventing confirmation of tree existence across teams.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix: return 404 instead of 403 for cross-account tag access

get_tag now returns 404 for account-specific tags that belong to another
account, preventing resource existence confirmation.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix: return 404 instead of 403 for cross-account step category access

get_step_category now returns 404 for account-specific categories that
belong to another account, preventing resource existence confirmation.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* test: add cross-tenant isolation tests for Task 6 UUID audit

Tests cover:
- Tree GET/PUT returns 404 for cross-account access
- Session GET returns 404 for cross-user access
- AI session GET returns 404 for cross-user access
- AI session retry-psa-push requires ownership
- Upload URL returns 404 for cross-account access
- Share revoke returns 404 for cross-user access

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix: return 404 (not 403) for get_documentation cross-user access; add missing Task 6 tests

get_documentation was revealing session existence via 403. Added pre-check
query filtering by session_id AND user_id before calling the engine.

Also add cross-tenant isolation tests for steps, tags, step_categories,
and maintenance_schedules endpoints fixed in Task 6 (TDD was skipped).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix: address Task 6 quality review — rename helper, restore 403 for intra-account, add docs test

- Rename _get_tree_or_403 → _get_tree_or_404 in maintenance_schedules.py
  (function now raises 404, old name was misleading)
- Restore HTTP 403 for intra-account permission failures in update_tree:
  same-account users who can see a tree but can't edit it got 404 (wrong);
  only cross-account lookups should return 404 to avoid confirming existence
- Apply same 403/404 distinction to update_tree_visibility
- Add test: get_documentation must return 404 for cross-user session access
- Add comment documenting owner-only design for documentation endpoints

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* chore: Task 7+8 — TargetList audit, CI tenant-filter grep check

Task 7: TargetList dead code audit
- Found active code references in 12+ files across backend and frontend
  (full CRUD API + frontend page + MaintenanceScheduleSection + BatchLaunchModal)
- Decision: migrate to account_id in Phase 1 (cannot drop)
- DB row count not available from code-server — must verify from VPS SSH
  before Phase 1 migration
- Teams orphan check query documented; must run from VPS SSH before Phase 1
- Results documented in spec Section 9

Task 8: CI tenant-filter enforcement check (warn mode)
- Create backend/scripts/check_tenant_filters.py
  Scans endpoint and service files for select() on tenant tables without
  tenant_filter/account_id/user_id in surrounding context. Currently
  reports 109 warnings (Phase 1 backlog). Exits 0 (warn mode).
- Add Check tenant filter enforcement step to backend CI job
  Add --fail flag after Phase 1 backlog clears to make it blocking.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* docs: record Phase 0 audit results — 0 orphaned teams, 0 target_list rows

Both checks confirmed 2026-04-09 from production DB.
Phase 1 migration is safe to proceed.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-04-09 00:42:19 -04:00
chihlasm
e6a0c0549b feat: Step Library sync + service account for default tree ownership
* feat: maintenance flow UX redesign — batch status hub, context strip, detail page upgrades (#85)

- Add BatchStatusPage (/flows/:id/batches/:batchId): per-target Start/Resume/View cards, progress bar, 5s polling while in-progress, completion outcome summary
- Add BatchStatusCard: handles not-started/in-progress/complete states with step progress for in-progress targets
- Add ActiveBatchBanner: amber banner on detail page when a batch is running, links to BatchStatusPage
- Add MaintenanceContextStrip: amber strip in ProceduralNavigationPage for maintenance flows showing target name, batch progress (X/Y complete), and Back to Batch nav
- Update MaintenanceFlowDetailPage: active batch banner, clickable run history rows with mini progress dots and outcome summaries, Run button loading state, post-launch navigates to BatchStatusPage
- Update ProceduralNavigationPage: renders MaintenanceContextStrip between top bar and content when tree_type === 'maintenance'; fetches batch progress once on mount
- Add batch_id filter to GET /sessions backend endpoint and SessionListParams frontend type
- Add /flows/:id/batches/:batchId route to router

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* feat: session detail page — completion action + outcome summary card

- In-progress sessions: amber banner with "Complete Session" button opens
  SessionOutcomeModal to set outcome/notes/next-steps and finalize
- Completed sessions: colored outcome summary card (icon + outcome label +
  duration + notes + next steps) replaces dense header metadata; "Copy for
  Ticket" promoted to primary action inside the card
- Export toolbar de-emphasized to secondary row of smaller controls below
  the summary card

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* feat: add library-page action props to StepCard (edit/delete/save)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* feat: pass library-page action props through StepLibraryBrowser + refreshKey

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* feat: StepFormModal wrapper + submitLabel/isSubmitting props on StepForm

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* feat: Step Library page — create, edit, delete, save-to-library

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* feat: add RuntimeStep union type for procedural custom steps

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* feat: StepChecklist accepts RuntimeStep[], renders amber Custom badge

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* feat: StepDetail accepts RuntimeStep, renders Custom Step badge for custom steps

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* feat: custom step insertion in procedural flow sessions

Engineers can add custom steps inline during execution. Steps are
persisted to session.custom_steps and restored on resume.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix: suppress StepFeedback on custom steps, fix resume stepState seeding, functional updater for step index

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* docs: add tree forking UI design doc

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* docs: add tree fork UI implementation plan

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* feat: add ForkInfo type and fork fields to Tree/TreeListItem

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix: align ForkInfo type with backend schema, remove redundant fork fields

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix: ForkInfo placement, required fork_info field, add JSDoc

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* feat: add ForkModal component with name and reason fields

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix: ForkModal accessibility and UX (escape, click-outside, labels, maxLength)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* feat: open ForkModal on fork action in TreeLibraryPage

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* feat: add ForkModal to MyTreesPage

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* feat: show Fork chip badge on forked tree cards

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* docs: add flow-to-library step sync design doc

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* docs: add flow-to-library sync implementation plan

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* feat: add sync tracking columns to step_library

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* feat: add sync columns and source_tree relationship to StepLibrary model

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* feat: add group_label to StepContent, is_flow_synced/source_tree_name to StepLibraryResponse

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* feat: include is_flow_synced and source_tree_name in step list/detail responses

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix: add is_flow_synced and source_tree_name to step list response

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix: add selectinload and sync fields to search and get_step endpoints

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* feat: add step_sync module with extraction and upsert logic

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix: safe NOT IN placeholders for asyncpg, add deactivate docstring

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* feat: trigger step library sync on tree publish and deactivate on delete

- Call sync_steps_from_tree in update_tree whenever the tree is published
  (status transitions to 'published' or is already published and structure changes)
- Call deactivate_synced_steps_for_tree in delete_tree before db.commit()
  so the FK SET NULL does not nullify source_tree_id before the WHERE clause runs
- Fix ::jsonb cast syntax in step_sync.py (asyncpg rejects :: operator in text()
  queries; replaced with CAST(:content AS jsonb))
- Add UniqueConstraint('source_tree_id','source_node_id') to StepLibrary model
  so Base.metadata.create_all (used by tests) creates the constraint that the
  ON CONFLICT clause in sync_steps_from_tree depends on

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* feat: add is_flow_synced and source_tree_name to Step types

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* feat: show From Flow badge and lock icon on flow-synced StepCard

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* feat: show source flow name in StepDetailModal for synced steps

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* feat: add Library Visibility select to procedural StepEditor

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix: address code review issues in flow-to-library sync

- Fix sync trigger: only fire on publish transition, not every PUT
- Add TestSyncOnPublish integration tests (2 tests, 16 total passing)
- Add group_label to frontend StepContent interface
- Guard Library Visibility select to procedure_step nodes only
- Block API edits to flow-synced steps (400 read-only guard)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix: handle None author_id in step sync to avoid invalid UUID error

When a system/default tree has no author (author_id is None),
str(None) produces the literal string 'None' which asyncpg
rejects as an invalid UUID for the created_by column.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix: add ResolutionFlow service account to own default tree steps in library

Default/system trees had no author_id (NULL), causing a NOT NULL violation
when syncing steps to step_library.created_by on publish.

- Add is_service_account flag to users table (migration 4f4137ce)
- Add service_account.py: idempotent ensure_service_account() creates
  noreply@resolutionflow.com with unusable password on startup
- Cache service account ID on app.state at lifespan startup
- Add get_service_account_id() FastAPI dep (returns None in tests)
- sync_steps_from_tree: resolve author_id or service_account_id as created_by
- create_tree: set author_id=service_account_id for is_default trees
- Migration 1490781700bc: backfill author_id on 31 existing default trees

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-02-25 23:17:29 -05:00
chihlasm
bd12ced5ee feat: analytics dashboards & two-tier feedback system (#78)
* docs: add analytics & user feedback design document

Covers team analytics, personal analytics, flow analytics,
step-level thumbs up/down feedback, and flow CSAT ratings.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* docs: add analytics & feedback implementation plan

12-task TDD plan covering session ratings, step feedback,
team/personal/flow analytics endpoints, and frontend pages.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* feat: add session_ratings table and analytics indexes

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* feat: add SessionRating model and analytics schemas

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* feat: add session CSAT rating endpoint with tests

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* feat: add step thumbs feedback and /ratings alias routes

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* feat: add team, personal, and flow analytics endpoints

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* feat: add recharts, analytics types, and API client

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* feat: add inline step thumbs up/down feedback during sessions

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* feat: add CSAT rating modal after session completion

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* feat: add Team Analytics page with charts and leaderboards

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* feat: add Flow Analytics panel with step dropoff and CSAT data

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* feat: add My Analytics page with personal stats and charts

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
2026-02-16 15:23:14 -05:00
chihlasm
b97596d286 refactor: tech debt reduction - extract hooks, deduplicate helpers, update deps, add CI
- Extract useCustomStepFlow hook from TreeNavigationPage (1040 → 759 lines)
- Create core/filters.py with shared tree/step visibility filters
- Create services/export_service.py from session export logic
- Add GitHub Actions CI/CD pipeline (pytest + lint + build)
- Add GIN index migration for full-text search on trees
- Update FastAPI 0.128.5, Pydantic 2.12.5, SQLAlchemy 2.0.46, +5 more
- Fix regex → pattern deprecation in Query() params

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-02-08 08:14:22 -05:00
chihlasm
e0089a9c5a feat: update all endpoints and schemas for account-based model
Replace team_id with account_id across all API endpoints (trees,
categories, tags, steps, step_categories, admin, auth). Add new
accounts and webhooks endpoints. Registration now atomically creates
Account + Subscription, with account_invite_code bypassing the
platform invite gate.

Schemas updated for account_id/account_role. 82 tests passing
including 18 new tests for accounts, subscriptions, and permissions.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-02-07 02:39:01 -05:00
chihlasm
71ba0b95a5 fix: high-severity security hardening (Phase B permissions audit)
Phase B addresses 7 high-severity gaps from the permissions audit:

- B1: Enforce tree access check on session start via can_access_tree
- B2: Replace all inline permission helpers with centralized permissions.py
- B3: Fix require_engineer_or_admin to check is_team_admin before role
- B4: Add is_active field on User with enforcement in get_current_active_user
- B5: Add admin user management endpoints (list, get, role, team-admin, deactivate, activate)
- B6: Add rate limiting on auth/invite endpoints via slowapi (disabled in DEBUG)
- B7: Implement refresh token rotation with JTI-based revocation and meaningful logout

Also reduces access token TTL from 15 to 5 minutes and updates CLAUDE.md
with SaaS/MSP context for future planning sessions.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-02-05 22:44:05 -05:00
chihlasm
34daa26a67 feat: implement RBAC permissions system
Add role-based access control with hierarchy: super_admin > team_admin >
engineer > viewer. Adds is_super_admin boolean to User model (migration 010),
centralized backend permissions module, frontend usePermissions hook, and
UI enforcement (conditional Create/Edit buttons, editor redirect for viewers,
role badge in header). All endpoint admin checks updated from role=="admin"
to is_super_admin.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
2026-02-05 02:42:44 -05:00
chihlasm
7803dc4522 Add step library foundation and user preferences (#24)
## Summary
Implements Phase 2.5 Step Library Foundation:

### Issues Completed
- #3 User Preferences - export format default setting
- #5 Step Categories - database table and seed data  
- #6 Step Library - database schema and migrations
- #7 Step Library - CRUD API endpoints
- #8 Step Library - rating and review system

### Changes
**Backend:**
- Migration 007: step_categories table with 10 seeded global categories
- Migration 008: step_library, step_ratings, step_usage_log tables
- Full CRUD API for step categories (/api/v1/step-categories)
- Full CRUD API for step library (/api/v1/steps) with search, filters, ratings
- CORS support for Railway PR environments (ALLOW_RAILWAY_ORIGINS)

**Frontend:**
- User preferences store (Zustand + localStorage)
- Settings page at /settings with export format dropdown
- Default export format applied in SessionDetailPage

### Testing
- Tested in Railway PR environment
- Database seeded with 7 MSP troubleshooting trees
- All API endpoints verified working
2026-02-03 02:07:46 -05:00