fix: misc token fixes

[MasterPtato]

Description

Please include a summary of the changes and the related issue. Please also include relevant motivation and context.

Type of change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes.

Checklist:

• My code follows the style guidelines of this project
• I have performed a self-review of my code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes

[MasterPtato]

• chore: remove non-engine drivers #4574 : 2 dependent PRs (#4581 , #4582 )
• feat: dynamic actors #4397
• fix: driver test suite & sqlite reliability #4559
• fix: remove serverless token #4572
• fix: misc token fixes #4569 👈 (View in Graphite)
• fix(pb): clean up actor stop decision handling #4566
• fix(envoy): use global instance, add signal handlers #4565
• chore: misc fixes, add pb snapshot test #4561
• fix(cache): make in memory cache global #4560
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[claude]

PR Review: fix: misc token fixes

Good set of focused fixes. A few notes below.

Security: Constant-Time Token Comparison

The use of subtle::ConstantTimeEq (ct_ne) is correct and a genuine improvement. ct_ne returns Choice(1) when inputs differ, so .into::<bool>() yields true on mismatch, triggering the Forbidden error. The early-return on None token in ctx.rs also cleans up the old Option<String> == Some(read()) comparison. Note: guard already had subtle in its Cargo.toml; only api-public needed it added.

Outbound Token Extraction

Secret<T>::read() returns &T directly with no locking, so the original code was not holding a lock across an await. The actual issue was likely a borrow lifetime constraint - serverless_outbound_req reading ctx.config().auth for the token while ctx was also needed elsewhere in the async body. Passing token: Option<&str> as a parameter is a cleaner interface regardless.

KV Channel: Removed Rationale Comment

In engine/packages/pegboard-kv-channel/src/lib.rs, the resolve_actor doc comment lost its two-line rationale explaining why namespace verification exists even though admin_token grants broad access (defense-in-depth against future less-privileged auth). Worth restoring unless the reasoning no longer applies.

KV Channel: Two Iterator Passes

The refactor uses two separate url.query_pairs().find_map() calls over the same query string. Negligible for short URLs, but a single pass collecting both keys would be slightly more explicit. Not blocking.

Envoy Token Refresh (TypeScript)

The GLOBAL_SHARED_CTX guard is fine since both globals are always set/cleared together. One pre-existing edge: if the envoy is mid-shutdown (globals not yet cleared), a concurrent startEnvoySync call hits the early-return path and writes a token update into a dying envoy. Not introduced here.

Serverful Protocol Version + LB Key Deletion

Both changes in pegboard-envoy/src/conn.rs look correct: tx.delete(&old_lb_key) during init mirrors the behavior already in update_ping.rs, and writing ProtocolVersionKey during serverful init is the right fix since metadata_poller only runs for serverless pools.

[claude]

PR Review: fix: misc token fixes

Overall this is a solid set of bug fixes. A few things worth flagging.

---

Security: Constant-Time Token Comparison

Files: engine/packages/api-public/src/ctx.rs, engine/packages/guard/src/routing/envoy.rs

Good use of the subtle crate to prevent timing-based token oracle attacks. Minor readability note: ct_ne(...).into() is correct, but !ct_eq(...).into() reads more naturally as "if not equal, return forbidden." Both are correct, just a nitpick.

---

Potential Lifetime Issue in pegboard-outbound

File: engine/packages/pegboard-outbound/src/lib.rs

The call ctx.config().auth.as_ref().map(|a| a.admin_token.read().as_str()) looks suspicious. If admin_token.read() returns a guard type (e.g. RwLockReadGuard<String>), the temporary guard drops at the end of the closure and the returned &str would be dangling. The compiler should catch this. If read() returns &str directly with lifetime tied to self, this is fine. Worth confirming it compiles cleanly in CI. If the return type is a guard, bind it to a named variable before calling .as_str().

---

Query Param Parsing Behavior Change

File: engine/packages/pegboard-kv-channel/src/lib.rs

The switch from HashMap to find_map reduces an allocation, but changes behavior for duplicate query keys: the old HashMap kept the last occurrence; find_map returns the first. Unlikely to matter in practice but worth noting.

---

Removed Security Comment in resolve_actor

File: engine/packages/pegboard-kv-channel/src/lib.rs

The removed comment explained why namespace-membership validation is performed even though admin_token is a global credential (defense-in-depth for future less-privileged auth). This is useful rationale for future auditors. Consider keeping a shorter version.

---

Missing tx.delete in handle_init

File: engine/packages/pegboard-envoy/src/conn.rs

The added tx.delete(&old_lb_key) is a clear bug fix. The old code added a conflict key but never removed the stale entry. Good catch.

---

Token Refresh for Serverless Envoys (TypeScript)

File: engine/sdks/typescript/envoy-client/src/tasks/envoy/index.ts

GLOBAL_SHARED_CTX and the token update on re-entry is a clean fix for the token expiry problem. The check GLOBAL_ENVOY && GLOBAL_SHARED_CTX is slightly redundant (they are set/cleared together) but is harmless defensive coding. Suggestion: add a short comment near the token assignment explaining why both globals are kept in sync, so it is clear neither becomes a stale reference if one is cleared independently.

---

Protocol Version Write for Serverful Pools

File: engine/packages/pegboard-envoy/src/conn.rs

The comment explains the rationale (serverful pools skip metadata_poller). No test coverage for this path is visible in the diff, but that is likely acceptable given the difficulty of integration-testing this code path.

---

[MasterPtato]

Merge activity

• Apr 8, 8:52 PM UTC: A user started a stack merge that includes this pull request via Graphite.
• Apr 8, 9:02 PM UTC: Graphite rebased this pull request as part of a merge.
• Apr 8, 9:03 PM UTC: @MasterPtato merged this pull request with Graphite.