TenantAtlas/specs/339-provider-connection-scope-hardening/plan.md
ahmido fcb03d2aee feat: harden provider connection authority resolution (339) (#410)
## Summary
- harden Provider Connection authority so workspace scope comes only from explicit workspace context and record ownership
- require explicit `environment_id` for Provider Connection create flows and remove remembered-environment or Filament-tenant fallback authority
- keep legacy query aliases such as `tenant`, `tenant_id`, and `managed_environment_id` inert for Provider Connection access
- add targeted Spec 339 feature coverage for create authority, workspace authority, and wrong-workspace / legacy-query denial behavior
- include Spec 339 artifacts (`spec.md`, `plan.md`, `tasks.md`) for the hardening slice

## Validation
- `cd apps/platform && ./vendor/bin/sail artisan test --compact tests/Feature/ProviderConnections --filter=ScopeHardening`

## Notes
- no new uncommitted workspace changes were present to commit in this turn; the branch already contained the feature commits
- Livewire v4 compliance unchanged
- Filament provider registration remains in `bootstrap/providers.php`
- no migrations, new assets, or route-family restructures

Co-authored-by: Ahmed Darrazi <ahmed.darrazi@live.de>
Reviewed-on: #410
2026-05-31 11:59:41 +00:00

315 lines
23 KiB
Markdown
Raw Permalink Blame History

This file contains ambiguous Unicode characters

This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.

# Implementation Plan: Spec 339 - Provider Connection Scope Hardening
- Branch: `339-provider-connection-scope-hardening`
- Date: 2026-05-31
- Spec: `specs/339-provider-connection-scope-hardening/spec.md`
- Input: Spec 339 + repo inspection of Provider Connection policy/resource authority seams.
## Summary
Harden Provider Connections (credential-adjacent) so that authorization and scope authority come only from:
- explicit workspace context (`WorkspaceContext`, session),
- explicit `environment_id` (filter/create), or
- record ownership (workspace + managed environment) for view/edit/actions.
Remove hidden fallbacks (remembered environment and Filament tenant) as authority sources and guard the contract with targeted tests.
## Technical Context
- Language/Version: PHP 8.4.15, Laravel 12.52.x.
- Primary Dependencies: Filament 5.x, Livewire 4.x, Pest 4.x.
- Storage: PostgreSQL; no schema change expected.
- Testing: Pest Feature tests (ProviderConnections family).
- Validation Lanes: fast-feedback (Feature). Browser smoke only if visible UX changes are introduced.
- Target Platform: Sail locally; Dokploy/container posture unchanged.
- Project Type: Laravel monolith under `apps/platform`.
- Constraints: no new persisted truth, migrations, packages, or broad navigation redesign.
## UI / Surface Guardrail Plan
- **Guardrail scope**: existing operator-facing resource + credential-adjacent action gating (authority hardening).
- **Affected surfaces**:
- Provider Connections list/view/create/edit routes (`/admin/provider-connections*`).
- Create CTA availability and create authorization (must require explicit `environment_id`).
- Policy semantics (404 vs 403) and record-derived authority for actions.
- **Native vs custom**: native Filament + existing capability/policy seams; no custom UI framework.
- **Shared-family relevance**: workspace hub filter contract, authorization semantics, action-surface discipline for credential-adjacent actions.
- **State layers in scope**: URL query (`environment_id` only), session workspace context, record ownership.
- **Handling modes**: review-mandatory (security semantics).
- **Required tests / smoke**:
- Feature tests that prove forbidden fallbacks cannot authorize create/view/actions.
- Optional reuse of existing browser smoke only if UI copy/CTA visibility changes are required.
- **UI/Productization coverage**: no new surface; do not update UI audit coverage files unless navigation routes change.
## Shared Pattern & System Fit
- **Cross-cutting feature marker**: yes.
- **Systems touched (expected)**:
- `apps/platform/app/Policies/ProviderConnectionPolicy.php`
- `apps/platform/app/Filament/Resources/ProviderConnectionResource.php`
- `apps/platform/app/Support/Workspaces/WorkspaceContext.php`
- `apps/platform/app/Support/Navigation/WorkspaceHubEnvironmentFilter.php`
- `apps/platform/tests/Feature/ProviderConnections/*`
- **Shared abstractions reused**: `WorkspaceContext`, deny-as-not-found semantics, capability resolver and gates, existing ProviderConnections test family.
- **New abstraction introduced?**: none.
- **Bounded deviation / spread control**: do not introduce a new “scope resolver framework”; keep changes local to the ProviderConnection policy/resource seams.
## OperationRun UX Impact
No new `OperationRun` types or start UX changes.
Provider operations reachable from Provider Connections must remain scoped to the records owning environment/workspace, but this specs changes should be limited to authority and authorization guardrails.
## Implementation Approach
### Phase 1 — Repo truth + failing tests first
- Inventory the current forbidden fallbacks:
- `ProviderConnectionPolicy::currentWorkspace()` fallback to `Filament::getTenant()`.
- `ProviderConnectionPolicy::resolveCreateTenant()` fallback to `WorkspaceContext::lastEnvironmentId()` and `Filament::getTenant()`.
- Provider Connection resource query scoping fallback paths when workspace context is missing.
- Add failing tests proving:
- create is impossible without explicit `environment_id` (even if remembered environment exists),
- Filament tenant cannot substitute for workspace context,
- legacy query aliases do not widen access or grant authority.
### Phase 2 — Policy authority hardening
- Remove forbidden fallbacks in `ProviderConnectionPolicy`:
- workspace authority must come from `WorkspaceContext` only,
- create tenant must come from explicit `environment_id` only.
- Keep 404 vs 403 semantics consistent with repo rules.
### Phase 3 — Resource scoping hardening
- Ensure `ProviderConnectionResource` list/query scoping:
- requires workspace context for membership scope (no tenant-context fallback),
- uses `environment_id` only as an explicit filter, validated against workspace/access scope.
- Ensure record pages/actions always use record-derived tenant/workspace (no hidden context authority).
### Phase 4 — Validation and regression posture
Run narrow tests first:
- `cd apps/platform && ./vendor/bin/sail artisan test --compact tests/Feature/ProviderConnections --filter=ScopeHardening`
- `cd apps/platform && ./vendor/bin/sail pint --dirty --format agent`
Run optional browser smoke only if implementation changes visible CTA/copy:
- `cd apps/platform && ./vendor/bin/sail artisan test --compact tests/Browser/Spec281ProviderConnectionScopeSmokeTest.php`
## Deployment / Ops Impact
- Migrations: none.
- Env vars: none.
- Queues/scheduler: none (no new job orchestration beyond existing provider operations).
- Filament assets: unchanged.
> **Fill when the feature touches shared provider/platform seams, identity scope, governed-subject taxonomy, compare strategy selection, provider connection descriptors, or operator vocabulary that may leak provider-specific semantics into platform-core truth. Docs-only or template-only work may use concise `N/A`.**
- **Shared provider/platform boundary touched?**: [yes / no / N/A]
- **Provider-owned seams**: [List or `N/A`]
- **Platform-core seams**: [List or `N/A`]
- **Neutral platform terms / contracts preserved**: [List or `N/A`]
- **Retained provider-specific semantics and why**: [none / short explanation]
- **Bounded extraction or follow-up path**: [none / document-in-feature / follow-up-spec / N/A]
## Constitution Check
*GATE: Must pass before Phase 0 research. Re-check after Phase 1 design.*
- Inventory-first: clarify what is “last observed” vs snapshots/backups
- Read/write separation: any writes require preview + confirmation + audit + tests
- Graph contract path: Graph calls only via `GraphClientInterface` + `config/graph_contracts.php`
- Deterministic capabilities: capability derivation is testable (snapshot/golden tests)
- RBAC-UX: two planes (/admin vs /system) remain separated; cross-plane is 404; tenant-context routes (/admin/t/{tenant}/...) are tenant-scoped; canonical workspace-context routes under /admin remain tenant-safe; non-member tenant/workspace access is 404; member-but-missing-capability is 403; authorization checks use Gates/Policies + capability registries (no raw strings, no role-string checks)
- Workspace isolation: non-member workspace access is 404; tenant-plane routes require an established workspace context; workspace context switching is separate from Filament Tenancy
- RBAC-UX: destructive-like actions require `->requiresConfirmation()` and clear warning text
- RBAC-UX: global search is tenant-scoped; non-members get no hints; inaccessible results are treated as not found (404 semantics)
- Tenant isolation: all reads/writes tenant-scoped; cross-tenant views are explicit and access-checked
- Run observability: long-running/remote/queued work creates/reuses `OperationRun`; start surfaces enqueue-only; Monitoring is DB-only; DB-only <2s actions may skip runs but security-relevant ones still audit-log; auth handshake exception OPS-EX-AUTH-001 allows synchronous outbound HTTP on `/auth/*` without `OperationRun`
- OperationRun start UX: any feature that creates, queues, deduplicates, resumes, blocks, completes, or links `OperationRun` reuses the central OperationRun Start UX Contract; no local composition of queued toast/link/event/start-state messaging; `OperationRun UX Impact` is present in the active spec or plan
- Ops-UX 3-surface feedback: if `OperationRun` is used, default feedback is toast intent-only + progress surfaces + exactly-once terminal `OperationRunCompleted` (initiator-only); queued DB notifications remain explicit opt-in through the shared start UX contract; running DB notifications stay disallowed
- Ops-UX lifecycle: `OperationRun.status` / `OperationRun.outcome` transitions are service-owned (only via `OperationRunService`); context-only updates allowed outside
- Ops-UX summary counts: `summary_counts` keys come from `OperationSummaryKeys::all()` and values are flat numeric-only
- Ops-UX guards: CI has regression guards that fail with actionable output (file + snippet) when these patterns regress
- Ops-UX system runs: initiator-null runs emit no terminal DB notification; audit remains via Monitoring; tenant-wide alerting goes through Alerts (not OperationRun notifications)
- Automation: queued/scheduled ops use locks + idempotency; handle 429/503 with backoff+jitter
- Data minimization: Inventory stores metadata + whitelisted meta; logs contain no secrets/tokens
- Test governance (TEST-GOV-001): actual test-purpose classification, affected lanes, fixture/helper/factory/seed/context cost risks, heavy-family visibility, review-stop points, reviewer handoff, and any budget/baseline/trend follow-up are explicit; the narrowest proving lane mix is planned and any structural cost change has an escalation path
- Proportionality (PROP-001): any new structure, layer, persisted truth, or semantic machinery is justified by current release truth, current operator workflow, and why a narrower solution is insufficient
- No premature abstraction (ABSTR-001): no new factories, registries, resolvers, strategy systems, interfaces, type registries, or orchestration pipelines before at least 2 real concrete cases exist, unless security, tenant isolation, auditability, compliance evidence, or queue correctness require it now
- Persisted truth (PERSIST-001): new tables/entities/artifacts represent independent product truth or lifecycle; convenience projections and UI helpers stay derived
- Behavioral state (STATE-001): new states/statuses/reason codes change behavior, routing, permissions, lifecycle, audit, retention, or retry handling; presentation-only distinctions stay derived
- UI semantics (UI-SEM-001): avoid turning badges, explanation text, trust/confidence labels, or detail summaries into mandatory interpretation frameworks; prefer direct domain-to-UI mapping
- Shared pattern first (XCUT-001): cross-cutting interaction classes reuse existing shared contracts/presenters/builders/renderers first; any deviation is explicit, bounded, and justified against current-release truth
- Provider boundary (PROV-001): shared provider/platform seams are classified as provider-owned vs platform-core; provider-specific semantics stay out of platform-core contracts, taxonomy, identifiers, compare semantics, and operator vocabulary unless explicitly justified; bounded extraction beats speculative multi-provider frameworks
- V1 explicitness / few layers (V1-EXP-001, LAYER-001): prefer direct implementation, local mappings, and small helpers; any new layer replaces an old one or proves the old one cannot serve
- Spec discipline / bloat check (SPEC-DISC-001, BLOAT-001): related semantic changes are grouped coherently, and any new enum, DTO/presenter, persisted entity, interface/registry/resolver, or taxonomy includes a proportionality review covering operator problem, insufficiency, narrowness, ownership cost, rejected alternative, and whether it is current-release truth
- Badge semantics (BADGE-001): status-like badges use `BadgeCatalog` / `BadgeRenderer`; no ad-hoc mappings; new values include tests
- Filament-native UI (UI-FIL-001): admin/operator surfaces use native Filament components or shared primitives first; no ad-hoc status UI, local semantic color/border decisions, or hand-built replacements when native/shared semantics exist; any exception is explicitly justified
- Filament-native UI (UI-FIL-001): custom Filament UI follows `docs/ui/tenantpilot-enterprise-ui-standards.md`; no ad-hoc custom styling for cards, buttons, hovers, badges, icons, progress bars, empty states, or interactive rows
- Filament-native UI (UI-FIL-001): if local Blade/Tailwind cards are
still necessary, they preserve dark mode correctness, spacing
consistency, badge semantics, action hierarchy, progressive
disclosure, accessibility, and Filament visual language
- Filament-native UI (UI-FIL-001): custom Blade/Widget/Page surfaces
keep Filament-native interaction semantics, preserve one dominant
primary action per focused area, express state through
BADGE-001-aligned badges/labels/supporting text instead of arbitrary
button colors, and do not create independent button/card/spacing
systems
- Filament-native UI (UI-FIL-001): hover, pointer, focus, shadow, or
similar interactive affordance appears only when a repo-real
route/action and permitted capability exist; non-interactive rows
stay visibly static
- UI/UX surface taxonomy (UI-CONST-001 / UI-SURF-001): every changed operator-facing surface is classified as exactly one allowed surface type; ad-hoc interaction models are forbidden
- Decision-first operating model (DECIDE-001): each changed
operator-facing surface is classified as Primary Decision,
Secondary Context, or Tertiary Evidence / Diagnostics; primary
surfaces justify the human-in-the-loop moment, default-visible info
is limited to first-decision needs, deep proof is progressive
disclosed, one governance case stays decidable in one context where
practical, navigation follows workflows not storage structures, and
automation / alerts reduce attention load instead of adding noise
- Audience-aware disclosure (DECIDE-AUD-001 / OPSURF-001): detail or
status surfaces separate customer-readable decision content,
operator diagnostics, and support/raw evidence; customer-readable
default paths hide raw JSON, copied context, fingerprints, internal
reason ownership, platform reason families, and debug semantics;
one dominant next action is explicit; duplicate visible truth is
removed
- UI/UX inspect model (UI-HARD-001): each list surface has exactly one primary inspect/open model; redundant View beside row click or identifier click is forbidden; edit-as-inspect is limited to Config-lite resources
- UI/UX action hierarchy (UI-HARD-001 / UI-EX-001): standard CRUD and Registry rows expose at most one inline safe shortcut; destructive actions are grouped or in the detail header; queue exceptions are catalogued, justified, and tested
- UI/UX scope, truth, and naming (UI-HARD-001 / UI-NAMING-001 / OPSURF-001): scope signals are truthful, canonical nouns stay stable across shells, critical operational truth is default-visible, and standard lists remain scanable
- UI/UX placeholder ban (UI-HARD-001): empty `ActionGroup` / `BulkActionGroup` placeholders and declaration-only UI conformance are forbidden
- UI naming (UI-NAMING-001): operator-facing labels use `Verb + Object`; scope (`Workspace`, `Tenant`) is never the primary action label; source/domain is secondary unless disambiguation is required; runs/toasts/audit prose use the same domain vocabulary; implementation-first terms do not appear in primary operator UI
- Operator surfaces (OPSURF-001): `/admin` defaults are operator-first; default-visible content avoids raw implementation detail; diagnostics are explicitly revealed secondarily
- Operator surfaces (OPSURF-001): execution outcome, data completeness, governance result, and lifecycle/readiness are modeled as distinct status dimensions when all apply; they are not collapsed into one ambiguous status
- Operator surfaces (OPSURF-001): every mutating action communicates whether it changes TenantPilot only, the Microsoft tenant, or simulation only before execution
- Operator surfaces (OPSURF-001): dangerous actions follow configuration safety checks/simulation preview hard confirmation where required execute, unless a spec documents an explicit exemption and replacement safeguards
- Operator surfaces (OPSURF-001): workspace and tenant context remain explicit in navigation, actions, and page semantics; tenant surfaces do not silently expose workspace-wide actions
- Operator surfaces (OPSURF-001): each new or materially refactored operator-facing page defines a page contract covering persona, surface type, operator question, default-visible info, diagnostics-only info, status dimensions, mutation scope, primary actions, and dangerous actions
- Filament UI Action Surface Contract: for any new/modified Filament Resource/RelationManager/Page, define Header/Row/Bulk/Empty-State actions, ensure every List/Table has a surface-appropriate inspect affordance, remove redundant View when row click or identifier click already opens the same destination, keep standard CRUD/Registry rows to inspect plus at most one inline safe shortcut, group or relocate the rest to More or detail header, forbid empty bulk/overflow groups, require confirmations for destructive actions, write audit logs for mutations, enforce RBAC via central helpers (non-member 404, member missing capability 403), and ensure CI blocks merges if the contract is violated or not explicitly exempted
- Filament UI UX-001 (Layout & IA): Create/Edit uses Main/Aside (3-col grid, Main=columnSpan(2), Aside=columnSpan(1)); all fields inside Sections/Cards (no naked inputs); View uses Infolists (not disabled edit forms); status badges use BADGE-001; empty states have specific title + explanation + 1 CTA; max 1 primary + 1 secondary header action (see HDR-001); tables provide search/sort/filters for core dimensions; shared layout builders preferred for consistency
- Action-surface discipline (ACTSURF-001 / HDR-001): every changed
surface declares one broad action-surface class; the spec names the
one likely next operator action; navigation is separated from
mutation; record/detail/edit pages keep at most one visible primary
header action; monitoring/workbench surfaces separate scope/context,
selection actions, navigation, and object actions; risky or rare
actions are grouped and ordered by meaning/frequency/risk; any special
type or workflow-hub exception is explicit and justified
- UI review workflow: native/custom classification, shared-family
relevance, state-layer ownership, repository-signal treatment,
exception path, and the active feature PR close-out entry stay
explicit without duplicating the same decision across spec, plan,
tasks, checklist, and close-out surfaces
- UI/Productization coverage (UI-COV-001): every spec completes the UI
Surface Impact decision; changed reachable UI surfaces update
`docs/ui-ux-enterprise-audit/` coverage artifacts with page archetype,
design depth, repo-truth level, target pattern/mockup need,
customer-safe review need, and dangerous-action review need; a checked
`No UI surface impact` decision is used only with a short rationale
## Test Governance Check
> **Fill for any runtime-changing or test-affecting feature. Docs-only or template-only work may state concise `N/A` or `none`.**
- **Test purpose / classification by changed surface**: [Unit / Feature / Heavy-Governance / Browser / N/A]
- **Affected validation lanes**: [fast-feedback / confidence / heavy-governance / browser / profiling / junit / N/A]
- **Why this lane mix is the narrowest sufficient proof**: [Why the chosen classification and lanes fit the actual proving purpose]
- **Narrowest proving command(s)**: [Exact commands reviewers should run before merge]
- **Fixture / helper / factory / seed / context cost risks**: [none / describe]
- **Expensive defaults or shared helper growth introduced?**: [no / describe explicit opt-in path]
- **Heavy-family additions, promotions, or visibility changes**: [none / describe]
- **Surface-class relief / special coverage rule**: [standard-native relief / named special profile / N/A]
- **Closing validation and reviewer handoff**: [What must be re-run, what reviewers should verify, and what exact proof command they should rely on]
- **Budget / baseline / trend follow-up**: [none / describe]
- **Review-stop questions**: [lane fit / breadth / hidden cost / heavy-family risk / escalation]
- **Escalation path**: [none / document-in-feature / follow-up-spec / reject-or-split]
- **Active feature PR close-out entry**: [Guardrail / Exception / Smoke Coverage / N/A]
- **Why no dedicated follow-up spec is needed**: [Routine upkeep stays inside this feature unless recurring pain or structural lane changes justify a separate spec]
## Project Structure
### Documentation (this feature)
```text
specs/[###-feature]/
├── plan.md # This file (/speckit.plan command output)
├── research.md # Phase 0 output (/speckit.plan command)
├── data-model.md # Phase 1 output (/speckit.plan command)
├── quickstart.md # Phase 1 output (/speckit.plan command)
├── contracts/ # Phase 1 output (/speckit.plan command)
└── tasks.md # Phase 2 output (/speckit.tasks command - NOT created by /speckit.plan)
```
### Source Code (repository root)
<!--
ACTION REQUIRED: Replace the placeholder tree below with the concrete layout
for this feature. Delete unused options and expand the chosen structure with
real paths (e.g., apps/admin, packages/something). The delivered plan must
not include Option labels.
-->
```text
# [REMOVE IF UNUSED] Option 1: Single project (DEFAULT)
src/
├── models/
├── services/
├── cli/
└── lib/
tests/
├── contract/
├── integration/
└── unit/
# [REMOVE IF UNUSED] Option 2: Web application (when "frontend" + "backend" detected)
backend/
├── src/
│ ├── models/
│ ├── services/
│ └── api/
└── tests/
frontend/
├── src/
│ ├── components/
│ ├── pages/
│ └── services/
└── tests/
# [REMOVE IF UNUSED] Option 3: Mobile + API (when "iOS/Android" detected)
api/
└── [same as backend above]
ios/ or android/
└── [platform-specific structure: feature modules, UI flows, platform tests]
```
**Structure Decision**: [Document the selected structure and reference the real
directories captured above]
## Complexity Tracking
> **Fill when Constitution Check has violations that must be justified OR when BLOAT-001 is triggered by new persistence, abstractions, states, or semantic frameworks.**
| Violation | Why Needed | Simpler Alternative Rejected Because |
|-----------|------------|-------------------------------------|
| [e.g., 4th project] | [current need] | [why 3 projects insufficient] |
| [e.g., Repository pattern] | [specific problem] | [why direct DB access insufficient] |
## Proportionality Review
> **Fill when the feature introduces a new enum/status family, DTO/presenter/envelope, persisted entity/table/artifact, interface/contract/registry/resolver, taxonomy/classification system, or cross-domain UI framework.**
- **Current operator problem**: [What present-day workflow or risk requires this?]
- **Existing structure is insufficient because**: [Why the current code cannot serve safely or clearly]
- **Narrowest correct implementation**: [Why this shape is the smallest viable one]
- **Ownership cost created**: [Maintenance, testing, cognitive load, migration, or review burden]
- **Alternative intentionally rejected**: [Simpler option and why it failed]
- **Release truth**: [Current-release truth or future-release preparation]