Skip to content

Passthrough token support - #134

Merged
thehabes merged 9 commits into
mainfrom
133-passthrough-token-support
Sep 30, 2026
Merged

thehabes merged 9 commits into
mainfrom
133-passthrough-token-support

Conversation

@cubap

@cubap cubap commented Sep 18, 2026 •

Copy link
Copy Markdown
Member

Closes #133

What this does

A registered application can make machine-to-machine calls through a running TinyNode without cloning the whole thing. Send your own access token in the Authorization header of any /create, /update, /overwrite, or /delete (both forms) request, and TinyNode forwards it to RERUM verbatim. The write is attributed to your registered agent, not the instance's. That makes the minimum machinery for a small annotation app an auth-wrapping backend call instead of a full driver clone of TinyThings.

Behavior

  • Verbatim forwarding. The caller's header value reaches RERUM unchanged. No scheme rewriting, no local validation. RERUM is the authority.
  • Passthrough error visibility. For modification routes (/create, /update, /overwrite, /delete), when the request carried a caller-supplied Authorization header, an upstream 401 or 403 is passed back to the caller with its real status code so the caller can debug their own token. For headerless (instance-token) requests, RERUM's 401/403 is reported as a TinyNode 502 whose body starts with 401: or 403: and includes RERUM's message, preserving TinyNode's existing error contract.
  • No header, no change. Requests without Authorization behave exactly as before, using the instance's ACCESS_TOKEN.
  • Operator kill switch. ALLOW_PASSTHROUGH_TOKENS=false in .env rejects any modification request carrying the header with 403 ("Token passthrough is not allowed on this TinyNode instance"). Rejection, not silence: falling back to the instance identity would misattribute the write, which is the exact failure this issue exists to prevent. The check is case-insensitive. Default is true so tiny.rerum.io can relay out of the box.
  • Refresh cycle. checkAccessToken skips the instance token refresh when the request is a passthrough one, so a failed refresh cannot fail a request that never uses the instance token.
  • Query is read-only and unchanged. /query does not forward caller Authorization headers and has no passthrough guard; it always uses the instance token and follows TinyNode's normal 502 error contract.
  • Overwrite 409 exception. /overwrite still returns 409 for version conflicts regardless of passthrough mode.

Implementation

  • passthrough.js (new, at repo root): resolveAuthorization, requirePassthroughAllowed, isPassthroughAllowed, isPassthroughRequest
  • tokens.js: passthrough short-circuit in checkAccessToken
  • Write routes (create, update, overwrite, delete both forms): guard middleware in the chain, resolveAuthorization(req) for the upstream header, conditional 401/403 pass-through when isPassthroughRequest(req) is true
  • /query: uses the instance token only, no guard middleware, no special 401/403 handling
  • openapi/components/tinynode-shared-components.openapi.yaml: bearerAuth HTTP security scheme documenting the passthrough contract and the kill switch; version bumped 0.1.0-alpha.1 -> 0.2.0-alpha.1 (additive, so the receiver repo can tell something arrived). The sync workflow will pick this up on merge.

Tests

All mocked per test/TESTING.md conventions; no live RERUM calls.

  • test/routes/passthrough.test.js (new): helper semantics (default-on, case-insensitive false kill switch, verbatim forwarding including non-Bearer env fallback), guard 403/next() behavior, and checkAccessToken interaction (skips refresh for passthrough, refreshes otherwise). Refresh tests run in isolated temp directories so they cannot overwrite the real .env.
  • Per-route __mock_functions __core suites for create, update, overwrite, delete (both forms), and query: verbatim upstream contract, 403 when disabled with proof no upstream fetch fires, passthrough 401/403 pass-through with real status codes, instance-token 401/403 mapped to 502, 502 preserved for other failures
  • openapi_sync_artifacts.test.js: guard asserting the bearerAuth scheme exists with its kill-switch documentation

Local results: npm test (116 pass).

Docs

  • README: "Passthrough Token Mode" section with a curl example, alongside the existing Client App and Centralized Client API modes
  • sample.env: ALLOW_PASSTHROUGH_TOKENS with comment
  • test/TESTING.md: what the passthrough tests validate and why (403 over silent misattribution, conditional 401/403 handling)

A registered application can now send its own Authorization header to any
TinyNode CRUD route and have it forwarded to RERUM verbatim, so the write is
attributed to the caller's agent instead of the instance's.  This gives small
annotation apps an auth-wrapping backend call without cloning TinyThings.

- routes/helpers/passthrough.js: resolveAuthorization picks the caller's
  header when present, requirePassthroughAllowed rejects with 403 when an
  operator sets ALLOW_PASSTHROUGH_TOKENS=false (no silent misattribution)
- tokens.js: checkAccessToken skips the instance refresh cycle for
  passthrough requests, so a failed refresh cannot fail a request that
  does not use the instance token
- all routes: pass upstream 401/403 through with their real status codes
  so callers can debug their own tokens; other failures remain 502
- docs: README Passthrough Token Mode section, sample.env flag,
  TESTING.md passthrough coverage notes
- openapi: bearerAuth security scheme, artifact bumped to 0.2.0-alpha.1

Closes #133
@cubap
cubap requested a review from thehabes as a code owner September 18, 2026 16:33
@cubap
cubap deployed to development September 18, 2026 16:34 — with GitHub Actions Active
thehabes

This comment was marked as outdated.

The ALLOW_PASSTHROUGH_TOKENS=false kill switch exists to prevent writes
from being attributed to the wrong agent.  A /query attributes nothing,
so rejecting it served no purpose and broke public reads for clients
that send an Authorization header on every call.

Keep resolveAuthorization so /query still forwards caller tokens for
uniformity, but remove requirePassthroughAllowed from the route chain.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@cubap
cubap deployed to development September 22, 2026 20:08 — with GitHub Actions Active
Upstream RERUM 401/403 responses now surface as TinyNode 502 errors with
RERUM's status and message in the body, matching the existing contract.
The only deliberate exception remains /overwrite returning 409 for version
conflicts.

- Remove 401/403 pass-through branches from create, update, overwrite,
  delete (both handlers), and query route handlers.
- Remove now-unused httpError imports from create.js and delete.js.
- Update per-route tests to assert 502 status while preserving the
  upstream 401:/403: prefix in the response body.
- Update TESTING.md, README.md, and OpenAPI bearerAuth description to
  describe the 502 contract.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@cubap
cubap deployed to development September 23, 2026 15:51 — with GitHub Actions Active
For modification routes (create, update, overwrite, delete), when the caller
supplied an Authorization header that is forwarded to RERUM, upstream 401/403
responses now pass through with their real status codes so the caller can
debug their own token. Instance-token requests (no Authorization header)
continue to receive TinyNode's 502 contract with RERUM's status and message
in the body.

- Add isPassthroughRequest checks before the 401/403 pass-through branch in
  create, update, overwrite, and both delete handlers.
- Re-import httpError in create.js and delete.js for the pass-through branch.
- Split per-route tests to cover both passthrough (401/403 real status) and
  instance-token (502) cases.
- Update TESTING.md, README.md, and OpenAPI bearerAuth description to
  describe the conditional behavior.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@cubap
cubap deployed to development September 23, 2026 16:52 — with GitHub Actions Active
@thehabes
thehabes self-requested a review September 28, 2026 19:44

@thehabes thehabes left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Copilot may have gone a little bonkers. Some of this has to be undone or cleaned up.

Static Review Comments

Review Date: 2026-09-28
Reviewer: Pair Static Review - Claude & @thehabes

Claude and Bryan make mistakes. Verify all issues and suggestions. Avoid unnecessary scope creep.

Category Issues Found
🔴 Critical 1
🟠 Major 1
🟡 Minor 3
🔵 Suggestions 3

Critical Issues 🔴

🔴 Issue 1: The new refresh-cycle tests overwrite the real .env, including production's

File: test/routes/passthrough.test.js:122 and test/routes/passthrough.test.js:146
Category: Breaking Changes / Unhandled side effects (tests)

Problem:
"Still refreshes the instance token…" and "Does NOT skip the refresh cycle…" both let generateNewAccessToken() run against the current working directory. That function rewrites ./.env with envfile.stringify. npm test runs from the repo root, so each run:

  • sets the real .env to ACCESS_TOKEN=refreshed-anyway
  • deletes all comments in .env

Production impact: .github/workflows/cd_prod.yaml:21 runs npm run allTests inside /srv/node/tiny-node/ on vlcdhprdp02, which is the live production directory, before it deploys. So the first production deploy after this PR merges will:

  1. Replace production's real ACCESS_TOKEN with refreshed-anyway and remove the comments from .env. .env is gitignored, so the git stash in the deploy job doesn't restore it.
  2. Restart TinyNode with pm2 start using that fake token. isTokenExpired treats anything that isn't a JWT as not expired, so the token is never refreshed.
  3. Make every write sent without a caller Authorization header fail: RERUM answers 401 and TinyNode reports it as 502. This lasts until someone repairs .env by hand, and every later production deploy breaks it again.

The existing test/routes/tokens.test.js avoids this with its inTempCwd() helper. The new tests don't use it.

Current Code:

  it("Still refreshes the instance token when the request has no Authorization header.", async () => {
    process.env.ACCESS_TOKEN = jwtWithExp(Math.floor(Date.now() / 1000) - 60)
    process.env.RERUM_ACCESS_TOKEN_URL = "https://auth.example/token"
    process.env.REFRESH_TOKEN = "refresh-token"
    // ...
    await checkAccessToken({ headers: {} }, {}, err => {

Suggested Fix:
Use the same temp-cwd isolation as tokens.test.js, and restore every env var the tests change:

import fs from "node:fs/promises"
import os from "node:os"
import path from "node:path"

const originalCwd = process.cwd()
const originalAccessTokenUrl = process.env.RERUM_ACCESS_TOKEN_URL
const originalRefreshToken = process.env.REFRESH_TOKEN
const tempDirs = []

async function inTempCwd(run) {
    const tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "tinynode-passthrough-test-"))
    tempDirs.push(tempDir)
    process.chdir(tempDir)
    await run(tempDir)
}

afterEach(async () => {
    // ...existing restores...
    process.env.RERUM_ACCESS_TOKEN_URL = originalAccessTokenUrl
    process.env.REFRESH_TOKEN = originalRefreshToken
    process.chdir(originalCwd)
    while (tempDirs.length > 0) {
        await fs.rm(tempDirs.pop(), { recursive: true, force: true })
    }
})

// then wrap the body of both refresh tests:
it("Still refreshes the instance token when the request has no Authorization header.", async () => {
    await inTempCwd(async () => {
        // ...existing body...
    })
})

How to Verify:

  1. Copy .env somewhere safe, run node --test test/routes/passthrough.test.js, and diff the two.
  2. Before the fix, ACCESS_TOKEN and the comments change. After the fix, .env is untouched.

Major Issues 🟠

🟠 Issue 2: Commit ab19912 deleted seven existing /query tests, including the pagination tests from PR #132

File: test/routes/query.test.js:67
Category: Breaking Changes / Test coverage regression

Problem:
main has 9 it() blocks in this file and this branch has 4. Commit ab19912 ("Restore 502 error contract…"), co-authored with Copilot, replaced four whole describe suites while editing the 401/403 test. The deleted tests aren't about passthrough and weren't moved anywhere else.

Suggested Fix:
Restore the file from main, then add the two new passthrough tests back inside the __mock_functions describe block.


Minor Issues 🟡

🟡 Issue 3: The README and OpenAPI overstate what passthrough does on /query

File: README.md:86, README.md:103, openapi/components/tinynode-shared-components.openapi.yaml:31
Category: Documentation accuracy

Problem:
Commits a899112 and ab19912 deliberately left /query without the kill-switch guard and without the 401/403 pass-through. The docs still say:

  • /query is one of the passthrough routes
  • a disabled instance rejects "a request that carries an Authorization header" with 403 (OpenAPI: "for any request carrying this header")

What actually happens: with passthrough disabled, /query accepts the header and quietly uses the instance token. RERUM also ignores Authorization on /query completely. I sent a bogus Bearer token and a Basic header to the running app and both returned 200 with results. Forwarding the caller's header on /query therefore does nothing except send their credential upstream.

Suggested Fix:
Limit the docs to the modification routes:

Send your own registered access token in the `Authorization` header of any `/create`, `/update`, `/overwrite`, or `/delete` request:
...
When disabled, a modification request that carries an `Authorization` header is rejected with `403` ...

Optionally, revert routes/query.js:35 to `Bearer ${process.env.ACCESS_TOKEN}` and remove its passthrough tests, since RERUM doesn't use the header there. That keeps /query out of the passthrough story altogether.


🟡 Issue 4: Test hygiene: redacted "******" header values and an untagged suite

File: test/routes/create.test.js:256 (plus delete.test.js, overwrite.test.js, update.test.js, query.test.js), and test/routes/create.test.js:313
Category: Code hygiene

Problem:

  • There are twelve occurrences of .set("Authorization", "******"). They were introduced in ab19912 and 0f3dccf, both co-authored with Copilot, and look like a secret-redaction artifact from those commits. The tests still pass because any non-empty value triggers passthrough, but a reader can't tell a Bearer token was intended.
  • In create.test.js, describe('Keeps 502 for other upstream failures…') has no __core or __mock_functions tag, so ci:fast (coreTests and functionalTests) skips it. The same test in update.test.js sits inside a tagged suite.

Suggested Fix:

      .set("Authorization", "Bearer bad-or-expired-token")

Move the create test into the tagged "Check that the create route passes caller tokens through to RERUM. __mock_functions __core" describe block, as update.test.js does.


🟡 Issue 5: The JSDoc for isPassthroughRequest is out of date

File: routes/helpers/passthrough.js:58
Category: Unnecessary/outdated comments

Problem:
The comment says it's "Used by checkAccessToken to skip the instance token refresh cycle". Since 0f3dccf, every modification route also uses it to decide whether an upstream 401/403 is passed through or returned as 502.

Suggested Fix:

/**
 * Whether the incoming request carries a caller-provided Authorization
 * header that will be honored upstream.  checkAccessToken uses it to skip
 * the instance token refresh, and the modification routes use it to pass
 * RERUM's 401/403 back with their real status.
 */

Suggestions 🔵

🔵 Suggestion 1: requirePassthroughAllowed doesn't need try/catch

File: routes/helpers/passthrough.js:41

It throws only to catch its own error and pass it to next. The if/return form used by verifyJsonContentType in rest.js is shorter and clearer:

export function requirePassthroughAllowed(req, res, next) {
    if (req?.headers?.authorization && !isPassthroughAllowed()) {
        next(httpError("Token passthrough is not allowed on this TinyNode instance. Remove the Authorization header to act as this instance's agent.", 403))
        return
    }
    next()
}

🔵 Suggestion 2: Move passthrough.js to the repo root

File: tokens.js:5

The root module tokens.js now imports from routes/helpers/, which is a new directory in this PR. The other cross-cutting helpers (rest.js, rerum.js, tokens.js) all live at the root. Keeping passthrough.js beside them avoids a root → routes dependency.

🔵 Suggestion 3: Accept FALSE, False, and surrounding spaces as "off" for the kill switch

File: routes/helpers/passthrough.js:10

ALLOW_PASSTHROUGH_TOKENS !== "false" is case-sensitive, so an operator who writes ALLOW_PASSTHROUGH_TOKENS = False still has passthrough on. That matches the existing OPEN_API_CORS convention, and a test (passthrough.test.js:38) asserts it. A case-insensitive check would be more forgiving for the feature's only operator control:

export function isPassthroughAllowed() {
    return process.env.ALLOW_PASSTHROUGH_TOKENS?.trim().toLowerCase() !== "false"
}

If you adopt it, change the "FALSE" assertion in passthrough.test.js to expect false.


If there are significant code changes in response to this review please test those changes. Run the application manually and test. Run internal programmatic tests when applicable.

…move passthrough helper

- Wrap passthrough refresh tests in temp cwd so .env rewrites cannot corrupt repo env
- Restore query.test.js from main and revert routes/query.js to instance-token only
- Move passthrough helper from routes/helpers/ to repo root and update imports
- Make ALLOW_PASSTHROUGH_TOKENS check case-insensitive; simplify guard middleware
- Replace redacted token placeholders with caller-token in tests and docs
- Fix untagged describe block in create.test.js
- Update README and TESTING.md to limit passthrough scope to modification routes

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@cubap
cubap deployed to development September 28, 2026 20:51 — with GitHub Actions Active
@thehabes
thehabes deployed to development September 29, 2026 15:15 — with GitHub Actions Active
@thehabes
thehabes deployed to development September 30, 2026 15:14 — with GitHub Actions Active
@thehabes
thehabes self-requested a review September 30, 2026 15:51
@thehabes
thehabes deployed to development September 30, 2026 16:16 — with GitHub Actions Active

@thehabes thehabes left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Automated and manual testing are clean. Did some cleanup. Ready to go.

@thehabes
thehabes merged commit 3547b51 into main Sep 30, 2026
5 checks passed
@cubap

cubap commented Oct 21, 2026

Copy link
Copy Markdown
Member Author

Code review: does PR #134 fully address #133?

Verdict: Yes — the issue is addressed completely. Machine-to-machine writes with a caller-supplied token are implemented for every write route (/create, /update, /overwrite, /delete in both forms), the caller's Authorization value replaces the instance token verbatim, and /query is correctly left read-only. No requirement from #133 is missing or partial.

Local deploy + test (this worktree, against a local mock RERUM upstream):

  • npm test → 115/115 pass (the PR body cites 116; the final cleanup commit removed one test).
  • Live end-to-end checks against a mock upstream:
    • Caller token forwarded verbatim; headerless requests use the instance token (confirmed in the mock's request log).
    • Upstream 401/403 → real 401/403 for passthrough requests; → 502 with 401:/403: body for instance-token requests.
    • /overwrite still returns 409 on version conflict.
    • /query does not forward the caller token.
    • ALLOW_PASSTHROUGH_TOKENS=false → 403 with the documented message, no upstream call; headerless requests still succeed.
  • Note: this machine runs Node v22.20.0 while package.json declares >=24.12.0; npm warned EBADENGINE but tests and the server ran fine.

Standards

  • Duplicated Code (judgement call, medium): the 401/403 passthrough block is copy-pasted five times (create.js, delete.js ×2, overwrite.js, update.js). Only the URL differs. Suggest extracting throwIfPassthroughAuthError(req, resp, url) into passthrough.js and calling it from each route.
  • Shotgun Surgery (judgement call, low): the import trio { isPassthroughRequest, requirePassthroughAllowed, resolveAuthorization } travels to all five route files; any change to the passthrough API surface touches every route. A single facade export or req decoration would reduce the blast radius.
  • Primitive Obsession (judgement call, low): isPassthroughAllowed() inlines string→boolean parsing; if more boolean env flags appear, a shared envFlag(name) helper would avoid repeating the shape. Borderline speculative today.
  • No hard violations of documented standards — README, sample.env, TESTING.md, and the OpenAPI file are all updated consistently.

Spec

  • Missing/partial: none. All write routes are covered; query is correctly excluded ("uses the core API to write to RERUM").
  • Scope beyond the literal issue (all documented in the PR, none harmful): the ALLOW_PASSTHROUGH_TOKENS kill switch, the 401/403 passthrough error contract, the OpenAPI bearerAuth scheme + version bump, and the checkAccessToken refresh short-circuit. These are reasonable enabling decisions, but they are inferred requirements, not stated ones — the PR description is a good record of that.
  • Implemented but worth a second look:
    • Kill switch is fail-open: only the exact string false (case-insensitive, trimmed) disables it; 0, no, off, disabled all leave passthrough on. Combined with default-on, a misconfigured instance silently accepts caller tokens. Consider accepting a small set of falsey values, or at least documenting that only false works.
    • (A sub-agent flagged an OpenAPI bearerFormat: JWT mismatch; that is not present — the scheme is type: http, scheme: bearer with no bearerFormat, and the instance fallback is a standard Bearer header, so the contract is consistent.)

Prioritized concerns

  1. Medium — duplicated 401/403 block (5 copies). Extract a helper; the current shape invites drift (e.g., a future route forgetting the passthrough branch).
  2. Medium — fail-open kill switch + default-on. Security-relevant decision point: any instance that never sets the env var relays caller tokens. The PR chose this deliberately ("tiny.rerum.io can relay out of the box") and documents it; flagging so operators know the default.
  3. Low — /query silently ignores a caller token. A caller who sends Authorization to /query gets instance-token results with no error. Documented as unchanged, but a 400/403 or a note would prevent surprise. Optional.
  4. Low — OpenAPI bearerAuth is defined but not referenced by any operation. Fine for a shared-components file, but the receiver repo must wire security: [{bearerAuth: []}] for it to take effect.

General suggestions

  • Add a test asserting the kill switch rejects 0/no/off (or decide they should disable) so the fail-open behavior is pinned.
  • Consider a single passthrough middleware that decorates req (e.g., req.authorization) so routes don't each import three helpers.
  • The test/helpers/env.js pin of ALLOW_PASSTHROUGH_TOKENS=true is good for determinism; note it means the suite never exercises the "unset" default in route tests (covered in passthrough.test.js).

Decision points (clarified)

  • Default-on passthrough: intentional and documented; the tradeoff is that public instances relay any caller token by default. If the team prefers fail-closed, flip the default and update README/sample.env/tests.
  • 401/403 passthrough asymmetry: deliberate — caller-token requests get real status codes (debuggability), instance-token requests keep the 502 contract. Documented in README and TESTING.md; consistent with the existing error contract.

This branch was successfully deployed

1 active deployment
development — 90238426 Deployed Sep 30, 2026 by thehabes via deploy (24, vlcdhp02) #209
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Passthrough token support

2 participants