[WRONG BRANCH] fix(config): validate noProxy values to prevent startup crash - #349
[WRONG BRANCH] fix(config): validate noProxy values to prevent startup crash#349luvs01 wants to merge 1 commit into
Conversation
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
✅ Deterministic PR hygiene checks passed. |
⏳ DRAFT
What to do
Its title has been prefixed with |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 830aa5b651
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| defaultModelAliases: z.boolean().optional(), | ||
| // A malformed hand edit must not make startup fail before the listener binds. | ||
| // Live writes remain strict through noProxyError(). | ||
| noProxy: noProxySchema.optional().catch(undefined), |
There was a problem hiding this comment.
Preserve valid bypasses in mixed persisted arrays
When a hand-edited noProxy array contains both a valid bypass and one malformed element (for example, ["internal.example", 1]), z.array(z.string()) fails and .catch(undefined) discards the entire field. Consequently, loadConfig() removes internal.example before applyProxyEnv() can perform its new element-level filtering, so requests intended to bypass the proxy are routed through it. Salvage string elements individually on the read path while retaining noProxyError() for strict live writes, and cover the mixed persisted-array case.
Useful? React with 👍 / 👎.
|
Superseded: reworked as a self-contained upstream PR against current dev. |
Motivation
noProxyfield without runtime validation, allowing malformed persisted values (non-strings / mixed arrays) to reachapplyProxyEnvand throw when it called string-only methods, preventing startup..passthrough()so undeclared fields could be preserved on disk but must not be allowed to create new crash paths at runtime.Description
noProxySchema(string | string[]) and declarenoProxyin the top-level ZodconfigSchemaso the load-time representation is well-formed or degrades safely.noProxyErrorintegrated intovalidateConfigCandidate()so CLIconfig set/import rejects malformednoProxycandidates instead of persisting them.applyProxyEnv()to defensively ignore non-string array elements and non-string values (filtering non-strings and treating non-string raw values as empty) so malformed persisted values no longer throw during startup.noProxy, and runtime handling of malformed array entries intests/config.test.tsandtests/proxy-env.test.ts.Testing
bun test tests/proxy-env.test.ts tests/config.test.tsand the affected tests passed (focused suite completed successfully).bun run typecheckand it succeeded.bun run testexercised the full repository and surfaced unrelated, preexisting failures in other subsystems; the focused tests for this change passed and the typecheck is green, so the patch is limited to the config/proxy boundary and validated by the focused runs.Codex Task