diff --git a/.claude/skills/pr-tester/SKILL.md b/.claude/skills/pr-tester/SKILL.md new file mode 100644 index 000000000..5bd506235 --- /dev/null +++ b/.claude/skills/pr-tester/SKILL.md @@ -0,0 +1,182 @@ +--- +name: pr-tester +description: Validate an AzureTRE pull request locally and, when explicitly authorized, against a specified Azure TRE environment. +user-invocable: true +argument-hint: PR link, PR number, branch name, or commit SHA +--- + +# AzureTRE PR Tester + +Use this workflow for requests such as “test this PR”, “validate PR 123”, or +“verify this branch”. Validate the exact PR revision without changing its source +logic and finish with an evidence-based verdict. + +## Authorization and safety + +A request to test a PR does not by itself authorize live Azure changes. Before +any external side effect, confirm the target environment, subscription/resource +scope, authorization, and cleanup expectations. Ask for approval immediately +before the first operation that changes the environment unless the user has +explicitly authorized that specific operation and environment. + +External side effects include: + +- pushing images or publishing/registering bundles; +- deploying or changing Azure/Terraform resources; +- creating, deleting, or modifying workspaces or test data; +- running tests against a shared or production-like environment; +- egress, isolation, or other security probes. + +Use synthetic data, bounded test identities, and known test workspaces for +authorized security checks. Never perform uncontrolled scanning, exfiltration, +or access to unrelated workspaces. + +Do not: + +- modify application logic, tests, API contracts, or security policy to make the + PR pass; +- merge or rebase the PR onto `main` unless explicitly requested; +- discard existing working-tree changes; +- commit, push, amend, force-push, or modify the PR; +- report PASS without concrete test, deployment, or functional evidence. + +Environment fixes must be minimal, documented, independently applicable to the +baseline, and reported separately from PR results. Revert temporary changes when +safe to do so. + +## Workflow + +### 1. Resolve the target + +Resolve a PR link, number, branch, or commit to the exact PR head SHA. Fetch +metadata and inspect the title, description, changed files, and relevant review +context. Record the branch and SHA. Do not assume that the currently checked-out +branch is the target. + +Prefer an isolated worktree. If the current worktree is dirty, preserve it and +stop to resolve the conflict or use a separate worktree; never use a forced +checkout or destructive cleanup. + +### 2. Plan and scope + +Create a short todo list covering: + +1. target branch and SHA; +2. changed components; +3. local checks; +4. external operations, if authorized; +5. functional and security checks; +6. cleanup and reporting. + +Determine affected components from the diff. Typical areas are `api_app`, +`resource_processor`, `airlock_processor`, `ui`, `core/terraform`, and +`templates`. Do not build, push, register, or redeploy unaffected components. + +Write a 3–6 bullet test plan stating the PR goal, checks, expected results, and +any failure-mode or security checks required by the changed surface. + +### 3. Run local checks first + +Run focused unit tests before any deployment. Start with the changed test file +or module and expand to the relevant package suite. Also run applicable lint, +type, formatting, bundle, or Terraform validation checks. + +Useful starting points include: + +```bash +cd api_app && python -m pytest -q +cd resource_processor && python -m pytest -q +cd airlock_processor && python -m pytest -q +terraform fmt -check -recursive +make lint-docs +``` + +Use `make lint` for repository-wide linting when Docker and the required tools +are available. Stop and report a local test failure before attempting a live +deployment unless the user explicitly asks for deployment despite the failure. + +### 4. Build and deploy only with authorization + +If live validation is authorized, build only images affected by the diff. Use +scoped Make targets rather than `make images`. Deploy only the affected API, +UI, processor, core, or bundle components. + +Before applying Terraform or running deployment targets, inspect the plan and +watch for replacement or destruction of shared infrastructure. Pause if the +operation exceeds the authorized scope or would be destructive. + +The repository's E2E Make targets may or may not be usable in the current +environment. Determine their prerequisites rather than assuming they run locally +or assuming they are unavailable. If equivalent manual checks are used, report +that full E2E coverage was not obtained. + +### 5. Verify the deployment is the PR + +Before functional validation, confirm that the running artifacts came from the +exact PR revision: + +- compare image tags or digests with the just-built artifacts; +- check API/UI-reported versions where available; +- compare bundle versions and `porter.yaml` metadata; +- confirm the deployed component is not a stale image or registration. + +If the deployed version cannot be established, do not claim live validation of +the PR. Report `BLOCKED` or `PARTIAL`. + +### 6. Functional and security validation + +Exercise the changed behavior against the authorized environment and assert +responses, state, and cleanup. Test both the stated happy path and relevant +failure modes, including invalid or missing input, duplicate/concurrent +operations, timeouts/retries, partial failure, rollback, and name collisions. + +For API changes, where applicable, test: + +- missing, expired, and malformed credentials; +- wrong-role and wrong-workspace access; +- core-token versus workspace-token boundaries; +- ownership and IDOR access to resource identifiers; +- validation of new or oversized fields. + +For Airlock, storage, networking, firewall, private endpoint/DNS, or workspace +isolation changes, use synthetic fixtures to verify that import/export review +flows, blob access, SAS/role scope, egress, DNS, and workspace boundaries remain +intact. Treat a confirmed boundary bypass as a blocking finding. + +### 7. Report + +Always state the checked-out branch, exact SHA, target environment, and whether +external operations were authorized. Use this format: + +```text +PR: — +Branch: <branch> +Commit: <exact SHA> +Environment: <local only or target TRE/subscription/resource scope> + +| Stage | Result | Detail | +|---|---|---| +| Test plan | PASS / – | checks defined | +| Unit tests | PASS / FAIL / – | counts and command | +| Lint/build/validation | PASS / FAIL / – | command and result | +| Images/artifacts | PASS / FAIL / – | affected artifacts and versions | +| Deploy | PASS / FAIL / – | components and notable changes | +| Deployed = PR | PASS / FAIL / – | versions/digests | +| Functional | PASS / FAIL / PARTIAL / – | checks and evidence | +| Data isolation | PASS / FAIL / PARTIAL / – | only when applicable | +| API security | PASS / FAIL / PARTIAL / – | only when applicable | +| Cleanup | PASS / FAIL / – | actions or pending work | + +Verdict: PASS / FAIL / PARTIAL / BLOCKED — <one-line justification> + +Findings / follow-ups: +- <concrete issue, coverage gap, or skipped operation> + +Needs fixing: +- <issue and proposed fix, if any> +``` + +Use `PARTIAL` when the tested scope passed but requested coverage was unavailable. +Use `BLOCKED` when safe or authorized validation could not proceed. Do not make +code fixes during this workflow; report findings and ask separately before +editing source. diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 000000000..74577059a --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,80 @@ +# AzureTRE agent instructions + +## Scope and repository map + +These instructions apply to the repository. A nearer `AGENTS.md` or `CLAUDE.md` may add subsystem-specific rules; read it before editing that subtree. + +AzureTRE is a Trusted Research Environment for secure research workspaces on Azure. The main areas are: + +- `api_app/` — FastAPI API, persistence, services, and API tests +- `resource_processor/` — VMSS Porter/resource processing +- `airlock_processor/` — Airlock event processors +- `ui/` — frontend application +- `core/terraform/` — core Azure TRE infrastructure +- `devops/terraform/` — management/bootstrap infrastructure +- `templates/` — workspace and service bundles +- `e2e_tests/` — environment-dependent pytest tests +- `docs/` — MkDocs documentation +- `.github/workflows/` — CI/CD definitions + +Prefer the nearest relevant code and tests over broad repository exploration. For uncommon workflows, read the linked procedure only when the task requires it. + +## Working rules + +- Inspect the existing implementation, tests, and relevant CI workflow before editing. +- Keep changes narrowly scoped; do not perform unrelated cleanup. +- Preserve existing user changes. Never discard a dirty working tree with `reset --hard`, `checkout -f`, `clean`, or an equivalent command. +- Do not commit, push, merge, publish, deploy, or destroy resources unless explicitly requested. +- Never place credentials, tokens, private environment files, or generated secrets in the repository or logs. +- Do not edit generated artifacts directly. Find the generator and update its source instead. +- Keep API contracts, schemas, migrations, bundle parameters, and their tests synchronized. + +## Safety boundaries + +Treat these as externally visible or state-changing operations and obtain explicit authorization immediately before running them unless the request clearly authorizes the specific operation and environment: + +- `terraform apply`, `terraform destroy`, `make *deploy`, `make *destroy`, and Azure resource changes +- pushing images, publishing or registering bundles, and changing shared environments +- creating, deleting, or modifying workspaces and test data +- end-to-end tests against shared or production-like environments +- security, egress, or isolation probes + +Prefer read-only inspection, formatting, focused tests, `terraform validate`, and `terraform plan` first. A plan is not approval to apply it. Use synthetic data and bounded test identities for authorized security or isolation checks. + +## Validation + +Run the narrowest relevant checks first, then expand as needed: + +| Area | Focused validation | Broader validation | +| --- | --- | --- | +| API | `cd api_app && python -m pytest <relevant-test> -q` | `cd api_app && python -m pytest tests_ma/test_api -q` | +| Resource processor | `cd resource_processor && python -m pytest <relevant-test> -q` | `cd resource_processor && python -m pytest tests_rp -q` | +| Airlock processor | `cd airlock_processor && python -m pytest <relevant-test> -q` | `cd airlock_processor && python -m pytest tests -q` | +| UI | Read `ui/` package scripts and run the relevant lint/typecheck/test command | UI build and applicable tests | +| Terraform | `terraform fmt -check -recursive` and `terraform validate` in the changed module | `terraform plan` in the approved environment | +| Documentation | `make lint-docs` | Documentation build | +| Repository | — | `make lint` (Docker-based, validates all files) | + +E2E tests require environment configuration and may create cloud resources. Use the selectors and authorization procedure in `.claude/skills/pr-tester/SKILL.md` when validating a PR against Azure. + +The top-level Makefile contains both safe validation and state-changing operations. Do not use `make all`, deployment, publishing, registration, or destruction targets as routine checks. + +## Pull request validation + +For PR validation, use `.claude/skills/pr-tester/SKILL.md` rather than inventing a deployment sequence. Resolve and record the exact PR head SHA, prefer an isolated worktree, do not implicitly merge or rebase onto `main`, scope builds and deployments to affected components, and verify deployed versions or digests match the PR before functional testing. + +Never report a PR as passing without concrete evidence. If only local, unit, or partial functional checks ran, report the coverage boundary and use `PARTIAL` or `BLOCKED` where appropriate. + +## Deeper procedures + +Add or consult focused procedures under `docs/development/` when a workflow is substantial or uncommon, such as dependency upgrades, API-version changes, Terraform changes, releases, and E2E execution. Keep this file as an operating manual, not an architecture encyclopedia. + +## Definition of done + +- The requested behavior is implemented without placeholders. +- Focused tests and applicable lint, type, or validation checks were run. +- Generated outputs were refreshed where required. +- Relevant documentation was updated. +- No unrelated files were changed. +- Failures and skipped checks are reported with their exact reason. +- The final response identifies changed files and validation performed. diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 000000000..9875e3dee --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,5 @@ +# Claude Code instructions + +Read and follow the repository-wide guidance in [`AGENTS.md`](AGENTS.md). + +Use the nearest subsystem instructions when they exist. For PR validation against Azure, load `.claude/skills/pr-tester/SKILL.md`; do not invent deployment or testing sequences. Keep the approval boundaries in `AGENTS.md`: external side effects, shared-environment tests, publishing, deployment, and destruction require explicit authorization.