Repository navigation
Security audit fixes - #13330
Merged
diegosouzapw merged 8 commits intoOct 9, 2026
Merged
Security audit fixes#13330diegosouzapw merged 8 commits into
diegosouzapw merged 8 commits into
Conversation
… in workspace audit - executors: pin outbound DNS resolution (createPinnedFetch, shared with remoteImageFetch.ts) to close a DNS-rebinding bypass of the SSRF guard on the provider-dispatch path, including the operator-override baseUrl case - db: never persist plaintext API keys after creation/regeneration (redacted:<id> placeholder, key_hash-only auth lookups); encrypt auto-generated JWT_SECRET/API_KEY_SECRET at rest; fail startup in production when STORAGE_ENCRYPTION_KEY is unset instead of silently falling back to plaintext credential storage - db: add a dedicated, encrypted-at-rest internal-service key so the plaintext-redaction fix above doesn't silently break combo health-check, cloud-sync verify, and the MITM agent-bridge auth (diegosouzapw#6403); reveal endpoint now reports 410 for a redacted key instead of returning a bogus value; key list view falls back to the stored key_prefix for a masked label - rate limiter: fail toward the existing in-memory limiter on a Redis error instead of failing open - mcp: force per-tool scope enforcement for callers resolved from a narrow mcp:connect-only key, regardless of the OMNIROUTE_MCP_ENFORCE_SCOPES default; give agentSkillTools their documented scopes so they don't break once enforcement applies All touched areas covered by updated/passing unit tests; typecheck clean.
JJAbrams-eng
force-pushed
the
fix/security-audit-2026-09-10
branch
from
September 11, 2026 08:37
80df10c to
45b54e8
Compare
Owner
|
Dense but well-reasoned batch — five independent, real security fixes (DNS-rebinding pinning |
…P forceEnforceScopes gap Adds direct coverage for the two new code paths in diegosouzapw#13330 that had no test yet — BaseExecutor.resolvePinnedFetch() (the DNS-rebinding pin now applied to provider dispatch, GHSA-cmhj-wh2f-9cgx) and forceEnforceScopes in withScopeEnforcement() (forces MCP scope checks for HTTP authInfo callers without manage/admin scope even when OMNIROUTE_MCP_ENFORCE_SCOPES is off) — plus a changelog fragment for the batch's five security fixes. Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
diegosouzapw
added
the
deferred-v3.8.52
diegosouzapw
changed the base branch from
release/v3.8.51
to
release/v3.8.52
September 29, 2026 11:25
Owner
|
Re-homed to |
… the dispatch DNS pin Merge conflicts (server.ts, db/apiKeys.ts, remoteImageFetch.ts, executors/base.ts) resolved keeping the tip's behavior plus this PR's MCP scope forcing. - Dispatch DNS pin redesigned: the pin is only applied to a plain direct connection (no context or env proxy, no explicit direct sentinel, no TLS impersonation) and is handed to the ambient global fetch as `init.dispatcher`, so outbound proxies, request logging and stubbed fetches keep working (previously a private undici fetch bypassed the patched fetch). - dnsPin.ts now reuses the tip's dnsPinnedFetch.ts primitives instead of duplicating them; remoteImageFetch.ts is taken from the tip unchanged. - API-key redaction (`redacted:<id>` placeholders, hash-only auth, internal service key) is dropped from this PR: keys are recoverable server-side by design today (cli-tools keys rawKey). - forceEnforceScopes moved to scopeEnforcement.ts::shouldForceScopeEnforcement, and the SSRF string guard moved next to the pin (dispatchPin.ts) to keep server.ts/base.ts inside their file-size ceilings. Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
… instead of pre-resolving DNS Replaces the pre-resolve + private-fetch pin: the guarded dispatcher validates the addresses of the lookup the connection is actually made with, rides on the ambient patched fetch (so stubs, request logging and the direct-path policies keep working), is skipped whenever a proxy, TLS impersonation or the direct sentinel applies, and does no DNS work at dispatch time. The pre-resolve helpers (dnsPin.ts, createPinnedDispatcher) are dropped as unused. Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
diegosouzapw
removed
the
deferred-v3.8.52
# Conflicts: # open-sse/executors/base.ts # open-sse/utils/proxyDispatcher.ts
…n the Redis-path test with the in-memory fallback
The release tip now sends execute()'s transport through validationFetch() (strict-validation fence); the merge plugs dispatchGuarded in as that fence's transport. The new execute() case proves both the fence (redirect lock, beforeFetch) and the guarded dispatcher reach the global fetch.
rate-limiter-backup-flag-not-test-mode detected the Redis path through the old fail-open { allowed: true }. This PR makes a Redis failure fall back to in-memory limiting, so the test now observes the Redis path through the limiter's 'Redis eval failed' log, and the NODE_ENV=test case asserts Redis is never reached.
Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
An empty STORAGE_ENCRYPTION_KEY stays the documented disabled contract. Exiting in production crash-loops Docker images that start from the example env with NODE_ENV=production. Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
…s were retranslated The English docs/frameworks/MCP-SERVER.md changed in this PR. Record that source hash at the top of .i18n-state.json so the docs-drift gate does not treat the edit as a new violation. Locale source_hash and target_hash stay on the previous English source: the mirrors are stale and non-blocking. A full i18n:run is not part of this fix. Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
Owner
|
Thanks @JJAbrams-eng — the dispatch path now checks operator-supplied base URLs at connect time, the rate limiter falls back to memory when Redis errors, MCP scope checks apply to a real Authorization header even when the env flag is off, and secrets at rest stay encrypted. A missing STORAGE_ENCRYPTION_KEY logs a loud boot warning and the process keeps serving, because an empty key is the documented way to leave encryption disabled. That commit is on your branch, co-authored. This is in the upcoming release. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Related Issues
Validation
Choose the change type and focused loop from the
Contribution Golden Path. The full unit suite,
Vitest, the 60% coverage gate, and the production build all run in CI on this PR (#8329):
npm run lintTests Added Or Updated
Coverage Notes
src/,open-sse/,electron/, orbin/, explain which tests cover the change.Reviewer Notes