# Authorize Activity File Reads Through ActivityHub

## Background

Clinic exposes `GET /clinic/api/v1/activityhub/file/view` and
`GET /clinic/api/v1/activityhub/file/download`. The original contract accepts
an arbitrary S3 object key in the `key` query parameter and passes it to an AWS
SDK v1 `GetObject` call configured with path-style addressing. SDK URI cleaning
allows non-canonical keys to escape the configured bucket path, and canonical
keys can select unrelated objects in the same bucket because the request is not
bound to an activity or caller-visible resource.

Issue #1695 tracks both the path traversal and object-authorization problems.
PR #1696 initially added canonical key validation, but review correctly found
that syntax validation alone does not close the same-bucket authorization gap.

## Goal

Remove caller control of the S3 key. Keep the two file routes, but require only
`activity_id`. Clinic will fetch that exact activity from ActivityHub, verify
the returned activity ID, extract `payload.details.key` on the server, validate
that key as defense in depth, and only then read S3.

The authorization boundary is explicit: these legacy OOM/ActivityHub APIs are
available only to internal Clinic users, and internal users are authorized for
all ActivityHub activities. `application_id` and `target_id` are therefore not
accepted as redundant caller-supplied consistency fields; they would not create
an independent org or cluster permission check.

The OOM report UI will pass the activity identity it already receives instead
of forwarding `payload.details.key` to Clinic.

## Current Behavior

- Both file routes accept a caller-provided `key` query parameter.
- The key is not associated with an ActivityHub activity, user, organization,
  or cluster.
- Canonical keys can read any object allowed by the Clinic S3 credentials.
- Non-canonical keys can be rewritten by AWS SDK URI cleaning before transport.
- The current Clinic UI exposes the ActivityHub-provided S3 key back to Clinic.

## Target Contract

- Both existing file paths require `activity_id`; they no longer accept `key`
  as object identity.
- The caller must be an authenticated internal Clinic user, matching the
  authorization contract of the ActivityHub activity and activity-group read
  APIs used by the OOM report.
- Clinic fetches the activity using the request context and an escaped activity
  ID, then verifies the returned `ActivityID` matches exactly.
- Clinic obtains the S3 key only from the matched activity's
  `payload.details.key`.
- Empty or non-canonical server-derived keys are rejected before S3.
- A legacy key-only request is rejected and cannot reach ActivityHub or S3.
- An upstream response for a different activity is rejected and cannot reach
  S3, even if its key names a valid object in the configured bucket.
- An empty or non-canonical key in the matched activity returns HTTP 400 and
  cannot reach S3.
- Validation, authorization, upstream failure, storage failure, and success
  emit trace-aware structured logs without logging the S3 key or file content.

## Test Seams

The backend seam is the public Gin HTTP route registered by
`activity_file.RegisterRouter`. Tests install the normal Clinic error
middleware and authenticated user context, use an ActivityHub
`httptest.Server`, and use a real AWS SDK v1 S3 client against a separate local
`httptest.Server`. These are the two external-system boundaries.

The retained backend cases cover:

- a matched activity reads its server-derived object through view and download;
- a missing internal user is forbidden before either upstream is called;
- a legacy key-only request is rejected before either upstream is called;
- an activity identity mismatch is rejected before S3;
- missing, empty, and traversal-bearing activity payload keys are rejected
  before S3.

Clinic UI has no test runner. Its seam is the typed `getOOMDetail` API wrapper;
the call contract will be checked by web type-check and lint rather than adding
new test infrastructure for this focused change.

## Affected Modules and Routes

Backend repository:

- Go module: root module `github.com/pingcap-inc/clinic`
- Packages: `pkg/api/activity_file` and `pkg/activityhub_client`
- Runtime entrypoint: `cmd/server/main.go`
- Routes:
  - `GET /clinic/api/v1/activityhub/file/view`
  - `GET /clinic/api/v1/activityhub/file/download`
- Docker build target: `docker_build_server`
- Produced image: `clinic`

Frontend repository:

- Non-Go package: `packages/clinic-web`
- Files: `src/api/activity.ts` and the OOM report page
- Produced frontend artifact: `packages/clinic-web/dist`, packaged in the
  Clinic UI image by `docker/Dockerfile`
- Image workflow: Clinic UI `Build docker and trigger deploy`
- app-delivery application and workload: `clinic-ui` / `clinic-ui` Deployment

The Clinic module/deployment ownership map does not change, so
`docs/clinic-module-overview.md` does not require an ownership update.

## Implementation Steps

1. Replace the file request key with an ActivityHub activity ID and reject an
   incomplete request.
2. Add a context-aware ActivityHub single-activity read that safely handles a
   missing result and escapes the activity ID in the upstream path.
3. Inject the ActivityHub reader into `DiagnosisActivityAPI` at its external
   boundary.
4. At each file handler, authorize an internal user, resolve and bind the
   activity, derive and validate the key, then call the existing S3 read path.
5. Keep canonical key validation as defense in depth and never log the key.
6. Rewrite the HTTP regression tests around authorized activity identity and
   both local upstream boundaries.
7. Update Clinic UI `getOOMDetail` and the OOM report page to pass the activity
   ID.
8. Run focused backend tests, the full changed backend packages, race checks,
   and Clinic UI type-check/lint.
9. After explicit authorization, deploy both feature-branch images to staging
   through their normal source workflows and app-delivery image bumps. Wait for
   both GitOps rollouts before testing the changed request contract.
10. After successful staging verification and explicit release authorization,
    merge both source PRs, record the resulting backend `master` and frontend
    `main` SHAs, and verify the merged runtime files match the staged feature
    revisions.
11. Release those exact source revisions to `prod-cloud` through the normal
    backend and frontend workflows. Inspect the generated app-delivery PRs for
    image-only changes, wait for their required checks and approvals, merge
    them under the requester's authorization, and wait for both GitOps
    rollouts.
12. Verify the production API contract with low-risk negative probes and an
    existing OOM activity as the positive fixture. Inspect production workload
    image/readiness/restarts and bounded control-plane logs when the approved
    credentials permit it.

## Exclusions

- No new database table, cache, signed URL, token format, key prefix scheme, or
  generalized authorization framework.
- No support expansion for external users; the existing OOM ActivityHub reads
  are internal-only.
- No changes to IAM, bucket policy, S3 client URI-cleaning configuration,
  CSRF, app-delivery desired state, or Kubernetes resources.
- No direct Kubernetes mutation or bypass of the GitHub Actions/app-delivery
  GitOps path.
- No `prod-op` release.
- No creation or mutation of production ActivityHub or S3 fixtures; production
  positive verification uses an existing authorized OOM activity and discards
  the returned file body.

## Compatibility Concerns

This intentionally removes the insecure key-only API contract. Existing direct
callers must move to activity identity parameters. Clinic UI is the known
caller and is updated in a separate coordinated PR.

Because backend and frontend images deploy independently, their runtime rollout
must be coordinated: an old UI cannot call the new backend contract, and a new
UI cannot call an old backend. The feature-branch images will be deployed
together to staging for validation. After that validation succeeds, the source
PRs and production image bumps will be merged and deployed as one coordinated
release under the explicit authorization recorded below.

Normal S3 object names remain supported when they are present in a matched
ActivityHub activity. Keys containing leading, trailing, or repeated slashes
or explicit `.`/`..` segments remain invalid because the configured SDK cannot
preserve their path identity safely.

## Test Plan

Run from the Clinic backend module root with GVM Go 1.24.2:

```bash
env GOROOT=/home/xhy/.gvm/gos/go1.24.2 \
  /home/xhy/.gvm/gos/go1.24.2/bin/go test ./pkg/api/activity_file \
  -run '^TestActivityFileHandlers' -count=1

env GOROOT=/home/xhy/.gvm/gos/go1.24.2 \
  /home/xhy/.gvm/gos/go1.24.2/bin/go test \
  ./pkg/api/activity_file ./pkg/activityhub_client -count=1

env GOROOT=/home/xhy/.gvm/gos/go1.24.2 \
  /home/xhy/.gvm/gos/go1.24.2/bin/go test -race \
  ./pkg/api/activity_file ./pkg/activityhub_client -count=1
```

Run from the Clinic UI repository root:

```bash
pnpm --filter @clinic/web type-check
pnpm --filter @clinic/web lint
```

Record the red failure before implementation and the green results after each
vertical slice. Also run `gofmt`, Prettier on touched UI files,
`git diff --check`, and inspect the final diffs.

### Local Validation Record

Validated on 2026-08-25. Backend commands used the GVM Go 1.24.2 binary and an
explicit matching `GOROOT`.

The initial backend test build failed as expected because the new public-route
test required an ActivityHub dependency that `DiagnosisActivityAPI` did not yet
have:

```text
unknown field activityAPI in struct literal of type DiagnosisActivityAPI
```

After implementation, the following passed:

- focused activity-file handler suite: `ok`, 0.020 seconds;
- `pkg/api/activity_file`, `pkg/activityhub_client`, and
  `pkg/api/activities`: all `ok`;
- race tests for `pkg/api/activity_file` and `pkg/activityhub_client`: both
  `ok`;
- `cmd/server` compile-only test: passed.

The UI contract check produced the expected `TS2345` error when the OOM page
was changed to pass only `activityId` while `getOOMDetail` still required its
prior request shape. The worktree dependencies and generated clients were then
built. After updating the wrapper and generated API contract, these checks
passed:

- `pnpm --filter @clinic/client build`;
- `pnpm --filter @clinic/web type-check`;
- `pnpm --filter @clinic/web lint`;
- `pnpm build` for the full workspace.

The full UI build emitted existing bundle-size/browser-data warnings. Its
external dashboard asset downloads failed, but those downloads are non-fatal in
the existing build and the workspace command completed successfully. Generated
slow-log-explorer build artifacts were not added to the change.

## Staging Verification

Authorized by the requester on 2026-08-25. Use the following bounded staging
path without merging either source PR:

1. Dispatch Clinic `Clinic CD` from the backend feature branch with
   `env=staging` and `components=clinic`.
2. Dispatch Clinic UI `Build docker and trigger deploy` from the frontend
   feature branch with `env=staging`, `region=global`, a feature-specific
   version, and PR #737 as `pr-info`.
3. Record both workflow run URLs, image tags, downstream app-delivery workflow
   runs and PRs, and wait for the app-delivery PRs to merge and GitOps to apply
   both images.
4. With the staging-only internal API key, confirm the legacy `key` request is
   rejected and cannot select an object; confirm a nonexistent activity is not
   read; and, when a suitable staging OOM activity is available, confirm the
   valid `activity_id` path still returns its detail.
5. Check the user-visible OOM route only if an authenticated staging UI session
   is available. A screenshot is unnecessary for this API contract change.
6. Query Clinic staging control-plane logs for the exact verification window
   when the staging credential is available. Do not print object keys, payloads,
   customer identifiers, authentication values, or unrelated log lines.
7. Inspect EKS Pod image/readiness/restarts only if separately approved AWS
   read access is available; otherwise record that this check was skipped.

The required `/private/tmp/clinic-api-staging.env` credential file was not
present at the initial preflight. Deployment may proceed, but authenticated API
and system-log verification must wait until the file exists, is not a symlink,
is owned by the current user, and has mode `600`.

### Staging Verification Record

Completed on 2026-08-25. Both feature images rolled out through app-delivery.
The backend and UI Deployments were available with the expected images, their
observed Pods were Ready with zero restarts, and the staging API rejected
missing parameters, legacy `key` requests (including traversal forms), and a
nonexistent activity with the expected status codes. Bounded control-plane log
review found the activity-file request lifecycle entries and did not find the
probe values in structured application logs.

All active staging clusters visible to the authorized key were searched over
the maximum supported 90-day ActivityHub window, but none exposed an existing
activity with `payload.details.key`. The production authorization below permits
the positive live-object check to use an existing production OOM activity
instead; local regression tests already exercise the positive view and download
paths against real HTTP ActivityHub and S3 test boundaries.

## Production Release and Verification

Authorized by the requester on 2026-08-25 after successful staging validation:

1. Wait for current required source checks, then merge Clinic PR #1696 and
   Clinic UI PR #737 using repository-supported merge methods.
2. Record the resulting backend `master` and frontend `main` SHAs. Compare the
   touched runtime files with the staged feature revisions; if merge resolution
   changed runtime content, stop and revalidate the exact merged image in
   staging.
3. Dispatch Clinic `Clinic CD` from the exact backend `master` SHA with
   `env=prod-cloud` and `components=clinic`.
4. Dispatch Clinic UI `Build docker and trigger deploy` from the exact frontend
   `main` SHA with `env=prod`, `region=global`, the next verified production
   version, and PR #737 as `pr-info`.
5. Verify both source workflow head SHAs and image tags. Inspect the generated
   app-delivery PRs and require image-only changes in the `prod-cloud`
   `clinic-api` and `clinic-ui` overlays.
6. Wait for normal app-delivery checks and approvals. The requester explicitly
   authorizes merging both production app-delivery PRs once those conditions
   are satisfied. Do not bypass required checks or branch protection.
7. Wait for GitOps to roll out both images. With approved read-only AWS access,
   verify the exact Deployment images, availability, Pod readiness, and restart
   counts without mutating the cluster.
8. With the production Clinic internal API key, verify both routes reject the
   legacy `key` contract and a nonexistent activity. Find an existing OOM
   ActivityHub record without printing its IDs, object key, tenant data, or
   payload; call view and download using only `activity_id`, discard their
   bodies, and record status, media type, and byte count only.
9. Query production Clinic control-plane logs for the bounded release and
   verification windows. Summarize only relevant activity-file outcomes and
   redact all identifiers and request data.

This authorization covers `prod-cloud` only. It does not cover `prod-op`, a
direct Kubernetes mutation, creation of production test data, or release of an
unexpected source revision or app-delivery diff.

## Release Boundary and Deployment Impact

Affected backend source modules:

- root Go module packages `pkg/api/activity_file` and
  `pkg/activityhub_client`;
- executable `cmd/server/main.go`;
- image `clinic`, built by Make target `docker_build_server` in
  `.github/workflows/clinic_cd.yml`.

Required backend deployment modules:

- Clinic CD input: `components=clinic`;
- app-delivery application: `clinic-api`;
- workload: Clinic Deployment;
- environment overlays: `staging/clinic/clinic-api/envs/staging` for this
  completed validation and `clinic/clinic-api/envs/prod-cloud` for the
  authorized production release;
- desired-state change: image-only bump; no separate app-delivery configuration
  PR is required.

Nearby backend components that do not require deployment: `internal-api`,
`task-manager`, `uploader`, `importer`, `clinic-data-service`, `ngm-api`,
`cloud-server`, and `api-gateway`.

Required frontend deployment modules:

- source package: `packages/clinic-web`, with the updated generated
  `packages/clinic-client` contract;
- build artifact and image: `packages/clinic-web/dist` -> `docker/Dockerfile`
  -> `ghcr.io/pingcap-inc/clinic-ui:global-<version>`;
- source workflow: Clinic UI `Build docker and trigger deploy`;
- app-delivery application: `clinic-ui`;
- workload: `clinic-ui` Deployment;
- environment overlays: `staging/clinic/clinic-ui/envs/staging` for this
  completed validation and `clinic/clinic-ui/envs/prod-cloud` for the
  authorized production release;
- desired-state change: image-only bump through
  `bump-clinic-apps-version.yaml` with `app=clinic-ui`; no separate
  app-delivery configuration PR is required.

The frontend and backend images must be released in coordination because the
HTTP contract changes atomically at the user-visible OOM flow. The backend
ownership chain is:

```text
pkg/api/activity_file + pkg/activityhub_client
  -> cmd/server/main.go
  -> docker_build_server
  -> clinic image
  -> Clinic CD components=clinic
  -> app-delivery clinic-api
  -> Clinic Deployment
```

The frontend ownership chain is:

```text
packages/clinic-client + packages/clinic-web
  -> packages/clinic-web/dist
  -> docker/Dockerfile
  -> ghcr.io/pingcap-inc/clinic-ui image
  -> Clinic UI Build docker and trigger deploy workflow
  -> app-delivery clinic-ui
  -> clinic-ui Deployment
```

Staging dispatch and the coordinated feature-image rollout completed
successfully. The requester explicitly authorized merging both source PRs,
dispatching both `prod-cloud` releases, and merging the resulting production
app-delivery PRs after their normal checks and approvals succeed. `prod-op`
remains out of scope.