Sign outbound ActivityPub follow actor retrieval #25

Merged
erik merged 2 commits from fix/task-b5f8d008-signed-actor-fetch into main 2026-10-13 22:32:47 -05:00
Owner

Summary

Fix task-b5f8d008: sign outbound Follow target actor GETs with Fedify using the website's configured primary actor, persisted keys, and existing compatible acct: identity. Hachyderm's signed-fetch requirement previously caused unsigned resolution to fail with HTTP 401, exposed through the API as HTTP 422.

  • Inject a lazy site-owned signer into the existing constrained resolver; WebFinger stays unsigned.
  • Preserve HTTPS/private-host checks, five-second abort through body reading, response-size limit, and redirect rejection; explicitly omit credentials, reject URL userinfo, and normalize bracketed private IPv6 hosts.
  • Fail closed without actor/key material, never generate/rotate keys during resolution, and never retry unsigned.
  • Preserve kind/host/status diagnostics without exposing arbitrary exception messages, keys, headers, credentials, or remote bodies.
  • Add cryptographically verified API fixtures and resolver failure/safety tests with intercepted fetch only.

SlugKit boundary

The SlugKit template resolver is identical to the website's pre-fix resolver. @evcraddock/slug-federation 0.1.0 does not provide actor retrieval, and its sender helper's #multikey-1 identity is not interchangeable with the site-local acct: compatibility wrapper. No package release is needed here because Fedify already supplies signing. Consider a reusable constrained-fetch helper accepting explicit signer/fetcher/policy, leaving actor/key/origin/persistence ownership site-local.

Created required linked SlugKit follow-up task-6e0ed12f — Port signed follow resolution to the SlugKit template, including root cause, solution, affected code, library assessment, and verification evidence. Linked from the website task and docs/activitypub-signed-follow-resolution.md.

Validation

  • Regression tests failed before the implementation (unsigned actor retrieval and missing safe rejection context).
  • After clean npm ci: npm test — 556 passed across 55 files.
  • npm run test:ci — 480 passed across 48 files.
  • Focused API/resolver suites — 30 passed, including real GET signature verification against the configured actor's advertised key.
  • npm run lint, npm run typecheck, npm run build, and git diff --check passed.
  • Build-generated CSS exactly matched the unrelated pre-existing generated CSS in the original worktree. Excluded it from this task; the original modification is untouched.
  • No dependency changes. Installation reported existing audit findings (10 moderate, 10 high); no unrelated audit remediation attempted.

Acceptance and approval gates

  • Implement constrained signed actor retrieval and safe rejection context.
  • Focused signed-fetch/rejection tests; no routine live-server requests.
  • Separately approved live Hachyderm verification and actual outcome: not performed.
  • Create and link the SlugKit follow-up with root cause/solution/code/local evidence.
  • Release/deployment evidence under the normal approval process: not performed; local validation documented.

Controlled tests observe pending persistence and one queued Follow from the configured website actor. This is not remote acceptance. No live Follow, production inspection/change, release, deployment, key rotation, or restart was performed. Merge, publishing, deployment, live verification, and task closure remain subject to their explicit approval gates. This PR does not close the task.

## Summary Fix `task-b5f8d008`: sign outbound Follow target actor GETs with Fedify using the website's configured primary actor, persisted keys, and existing compatible `acct:` identity. Hachyderm's signed-fetch requirement previously caused unsigned resolution to fail with HTTP 401, exposed through the API as HTTP 422. - Inject a lazy site-owned signer into the existing constrained resolver; WebFinger stays unsigned. - Preserve HTTPS/private-host checks, five-second abort through body reading, response-size limit, and redirect rejection; explicitly omit credentials, reject URL userinfo, and normalize bracketed private IPv6 hosts. - Fail closed without actor/key material, never generate/rotate keys during resolution, and never retry unsigned. - Preserve kind/host/status diagnostics without exposing arbitrary exception messages, keys, headers, credentials, or remote bodies. - Add cryptographically verified API fixtures and resolver failure/safety tests with intercepted fetch only. ## SlugKit boundary The SlugKit template resolver is identical to the website's pre-fix resolver. `@evcraddock/slug-federation` 0.1.0 does not provide actor retrieval, and its sender helper's `#multikey-1` identity is not interchangeable with the site-local `acct:` compatibility wrapper. No package release is needed here because Fedify already supplies signing. Consider a reusable constrained-fetch helper accepting explicit signer/fetcher/policy, leaving actor/key/origin/persistence ownership site-local. Created required linked SlugKit follow-up **task-6e0ed12f — Port signed follow resolution to the SlugKit template**, including root cause, solution, affected code, library assessment, and verification evidence. Linked from the website task and `docs/activitypub-signed-follow-resolution.md`. ## Validation - Regression tests failed before the implementation (unsigned actor retrieval and missing safe rejection context). - After clean `npm ci`: `npm test` — 556 passed across 55 files. - `npm run test:ci` — 480 passed across 48 files. - Focused API/resolver suites — 30 passed, including real GET signature verification against the configured actor's advertised key. - `npm run lint`, `npm run typecheck`, `npm run build`, and `git diff --check` passed. - Build-generated CSS exactly matched the unrelated pre-existing generated CSS in the original worktree. Excluded it from this task; the original modification is untouched. - No dependency changes. Installation reported existing audit findings (10 moderate, 10 high); no unrelated audit remediation attempted. ## Acceptance and approval gates - [x] Implement constrained signed actor retrieval and safe rejection context. - [x] Focused signed-fetch/rejection tests; no routine live-server requests. - [ ] Separately approved live Hachyderm verification and actual outcome: **not performed**. - [x] Create and link the SlugKit follow-up with root cause/solution/code/local evidence. - [ ] Release/deployment evidence under the normal approval process: **not performed**; local validation documented. Controlled tests observe pending persistence and one queued Follow from the configured website actor. This is not remote acceptance. No live Follow, production inspection/change, release, deployment, key rotation, or restart was performed. Merge, publishing, deployment, live verification, and task closure remain subject to their explicit approval gates. This PR does not close the task.
Task: #task-b5f8d008
docs: link SlugKit follow-up and clarify signed-fetch test fixtures
All checks were successful
CI / build-lint-test (push) Successful in 42s
cda377866b
Task: #task-b5f8d008
Author
Owner

Review update

  • PR: erik/erikvancraddock.com#25 — open; reviewed head cda377866b21673e5af6ca241ac48c565ee5182c for task-b5f8d008.
  • Result: changes requested under the bounded review contract because required live verification is not evident. This is an operational acceptance gap, not an identified implementation defect.
  • Summary: The diff wires the primary actor's persisted keys and existing acct: signature identity into actor GETs through Fedify, leaves WebFinger unsigned, preserves the constrained fetch wrapper, sanitizes retrieval errors, and adds meaningful signature-verification and failure tests. No code correctness blocker was identified in the reviewed diff and directly relevant files.
  • Evidence: Implementation reports 556 full tests, 480 CI-subset tests, 30 focused tests, lint, build/typecheck and diff check passing; CI job #87 succeeded for cda377866b (provided evidence, not independently rerun). Review was read-only; no broad checks, live requests, Follow submission, merge, publishing or deployment occurred.

Acceptance criteria

  1. met — Signed actor retrieval and existing protections: src/api/routes/following.ts injects the lazy signer; src/federation/keys.ts reads existing primary-actor keys without generating them; src/following/following.ts signs inside the existing HTTPS/private-host, timeout, size and redirect restrictions. URL credentials are rejected; transport/signing errors are sanitized; no unsigned retry is introduced.
  2. met — Focused signed-fetch and rejection tests: API fixtures cryptographically verify the configured actor's signature for direct URLs and handles; resolver tests cover unsafe/credential-bearing URLs, malformed/oversized responses, rejection, missing signer material, transport/signing failures and timeout through response reading. Fetch is intercepted rather than sent to live social servers.
  3. not-evident (pending) — Separately approved live Hachyderm verification and recorded actual outcome have not happened. Local HTTP 201, pending persistence and one queued Follow demonstrate controlled local submission only; they do not prove remote delivery or acceptance.
  4. met — SlugKit follow-up task-6e0ed12f is created and linked from the bug and documentation. The task's existing update records root cause, solution, affected template code, local verification evidence and the library-boundary assessment. No additional SlugKit implementation is part of this PR.
  5. partial (pending) — Local validation is recorded, and documentation preserves the normal release/deployment approval gates. Publishing/release and deployment evidence are absent because those operations have not happened.

Blocking issues: 2

  • B1 — Missing live acceptance evidence (criterion 3): Before task completion, obtain separate authorization through the normal workflow, verify the identified Hachyderm target, and record actor resolution, Follow delivery/queueing, correlated remote Accept/Reject if observed, and persisted status. Report pending or failed outcomes honestly. This review requests/grants no authorization and submits no Follow.
  • B2 — Incomplete release/deployment evidence (criterion 5): Before task closure, complete the separately approved normal release/deployment process and attach deployed version/digest and validation evidence. The implementation PR does not close the task; this finding is not an instruction to release or deploy during review.

Warnings: 1

  • W1 — Inherited resolver safety limits: Hostname/literal checks are not complete DNS-rebinding protection, and the size check remains a post-read character limit rather than a streaming byte cap. The PR documents these existing limits and does not claim to fix them; they are not a new regression or blocker for this scoped change.

Final verdict

changes-requested: 5 criteria total; 3 met, 1 partial, 1 not-evident; 2 not fully met (criteria_missing=2). The shared contract requires this verdict for any not-evident criterion. No implementation change is prescribed by this review; operational acceptance must remain pending until separately authorized work supplies evidence. No implementation approval or merge authorization is requested or granted.

### Review update - PR: `erik/erikvancraddock.com#25` — open; reviewed head `cda377866b21673e5af6ca241ac48c565ee5182c` for `task-b5f8d008`. - Result: **changes requested** under the bounded review contract because required live verification is not evident. This is an operational acceptance gap, not an identified implementation defect. - Summary: The diff wires the primary actor's persisted keys and existing `acct:` signature identity into actor GETs through Fedify, leaves WebFinger unsigned, preserves the constrained fetch wrapper, sanitizes retrieval errors, and adds meaningful signature-verification and failure tests. No code correctness blocker was identified in the reviewed diff and directly relevant files. - Evidence: Implementation reports 556 full tests, 480 CI-subset tests, 30 focused tests, lint, build/typecheck and diff check passing; CI job #87 succeeded for `cda377866b` (provided evidence, not independently rerun). Review was read-only; no broad checks, live requests, Follow submission, merge, publishing or deployment occurred. #### Acceptance criteria 1. **met** — Signed actor retrieval and existing protections: `src/api/routes/following.ts` injects the lazy signer; `src/federation/keys.ts` reads existing primary-actor keys without generating them; `src/following/following.ts` signs inside the existing HTTPS/private-host, timeout, size and redirect restrictions. URL credentials are rejected; transport/signing errors are sanitized; no unsigned retry is introduced. 2. **met** — Focused signed-fetch and rejection tests: API fixtures cryptographically verify the configured actor's signature for direct URLs and handles; resolver tests cover unsafe/credential-bearing URLs, malformed/oversized responses, rejection, missing signer material, transport/signing failures and timeout through response reading. Fetch is intercepted rather than sent to live social servers. 3. **not-evident (pending)** — Separately approved live Hachyderm verification and recorded actual outcome have not happened. Local HTTP 201, pending persistence and one queued Follow demonstrate controlled local submission only; they do not prove remote delivery or acceptance. 4. **met** — SlugKit follow-up `task-6e0ed12f` is created and linked from the bug and documentation. The task's existing update records root cause, solution, affected template code, local verification evidence and the library-boundary assessment. No additional SlugKit implementation is part of this PR. 5. **partial (pending)** — Local validation is recorded, and documentation preserves the normal release/deployment approval gates. Publishing/release and deployment evidence are absent because those operations have not happened. #### Blocking issues: 2 - **B1 — Missing live acceptance evidence (criterion 3):** Before task completion, obtain separate authorization through the normal workflow, verify the identified Hachyderm target, and record actor resolution, Follow delivery/queueing, correlated remote Accept/Reject if observed, and persisted status. Report pending or failed outcomes honestly. This review requests/grants no authorization and submits no Follow. - **B2 — Incomplete release/deployment evidence (criterion 5):** Before task closure, complete the separately approved normal release/deployment process and attach deployed version/digest and validation evidence. The implementation PR does not close the task; this finding is not an instruction to release or deploy during review. #### Warnings: 1 - **W1 — Inherited resolver safety limits:** Hostname/literal checks are not complete DNS-rebinding protection, and the size check remains a post-read character limit rather than a streaming byte cap. The PR documents these existing limits and does not claim to fix them; they are not a new regression or blocker for this scoped change. #### Final verdict **changes-requested**: 5 criteria total; 3 met, 1 partial, 1 not-evident; 2 not fully met (`criteria_missing=2`). The shared contract requires this verdict for any not-evident criterion. No implementation change is prescribed by this review; operational acceptance must remain pending until separately authorized work supplies evidence. No implementation approval or merge authorization is requested or granted.
erik merged commit 185a498186 into main 2026-10-13 22:32:47 -05:00
erik deleted branch fix/task-b5f8d008-signed-actor-fetch 2026-10-13 22:32:47 -05:00
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
erik/erikvancraddock.com!25
No description provided.