Skip to content
This repository was archived by the owner on May 8, 2026. It is now read-only.

Remove HMAC auth from runner server#16

Merged
rowan-stein merged 2 commits intomainfrom
noa/issue-15
Mar 19, 2026
Merged

Remove HMAC auth from runner server#16
rowan-stein merged 2 commits intomainfrom
noa/issue-15

Conversation

@casey-brooks
Copy link
Copy Markdown
Contributor

Summary

  • remove HMAC auth contracts and exports
  • drop grpc auth verification/config fields in server/main
  • simplify grpc test helpers and integration/e2e tests

Testing

  • pnpm lint
  • pnpm test

Closes #15

@casey-brooks
Copy link
Copy Markdown
Contributor Author

Summary

  • removed HMAC auth contracts and config fields from the runner server
  • dropped grpc auth verification/nonce cache usage
  • simplified grpc test helpers and integration/e2e tests

Testing

  • pnpm lint (no errors)
  • pnpm test (12 passed, 0 failed, 0 skipped)

noa-lucent
noa-lucent previously approved these changes Mar 19, 2026
Copy link
Copy Markdown

@noa-lucent noa-lucent left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clean, well-scoped removal of HMAC auth. All server-side HMAC verification, config fields, helper functions, and test plumbing have been correctly stripped. No stale references remain in production code.

One minor cleanup noted in the test helper — the path parameter chain (metadataForunary → call sites) is now dead weight and can be removed along with the unused path constant imports. Not blocking.

Approving — this fully resolves #15.

Comment thread __tests__/helpers/grpc-test-client.ts Outdated
const { client, secret } = options;
const { client } = options;

const metadataFor = (path: string): Metadata => {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[minor] metadataFor now ignores the path parameter — it's a dead leftover from HMAC signing. This also makes the path parameter in unary dead, and the RUNNER_SERVICE_*_PATH imports on lines 6–10 are only used as dead arguments flowing into these two functions.

Consider cleaning this up:

  • Remove path from metadataFor signature (it can just be a no-arg function returning new Metadata()).
  • Remove path from unary and call sites.
  • Remove the now-unused RUNNER_SERVICE_*_PATH imports.

This avoids leaving stale plumbing that will confuse future readers about whether path still matters.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed — removed dead path parameter from metadataFor and unary, and cleaned up the unused RUNNER_SERVICE_*_PATH imports.

@casey-brooks
Copy link
Copy Markdown
Contributor Author

Summary

  • simplified grpc test helper metadata utilities by removing unused path args
  • dropped unused runner service path imports in grpc test client

Testing

  • pnpm lint (no errors)
  • pnpm test (12 passed, 0 failed, 0 skipped)

@rowan-stein rowan-stein merged commit 2f3fedb into main Mar 19, 2026
1 check passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Remove HMAC shared-secret authentication from docker-runner

3 participants