You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
During the review of #2120 (eliminate inline ESLint disables in packages/settings), a recurring tactical anti-pattern was identified: widening values to unknown (via as unknown casts or : unknown annotations) purely to silence @typescript-eslint/no-unnecessary-condition, sonarjs/different-types-comparison, and related rules — without fixing the underlying trust boundary.
This is a subtler form of the same bypass the ESLint-cleanup effort (#2078) set out to eliminate. Instead of an inline eslint-disable comment, the suppression is moved into the type system: the value is cast to unknown so a defensive check that the compiler would otherwise flag as "unnecessary" becomes "necessary" again. The directive disappears, lint goes green, but the root cause — an internal type that over-promises trust while the data actually crosses untrusted boundaries — is left intact.
Concrete example (from #2120, packages/settings/src/settings/SettingsService.ts)
interfaceEphemeralSettings{providers: Record<string,Record<string,unknown>>;// ...}getProviderSettings(provider: string): Record<string,unknown>{
const entry=this.settings.providers[provider]asunknown;// ← the tellif(isProviderSettingsRecord(entry)){returnentry;}return{};}
The declared type says providers[provider] is always a record, so the compiler considers a null/shape check "unnecessary". Rather than make the type honest, the code casts back to unknown and re-narrows at every read site (~7 times in this one class). The defensive check is necessary only because the type invariant is violable: set('providers.openai', 'not-a-record') routes through setNestedValue and can write an arbitrary value into the map, bypassing any guarantee that values are records.
What counts as "abuse" (in scope)
NOT every use of unknown is a problem. Legitimate uses include:
Boundary parse points: JSON.parse() → unknown, external/plugin input typed unknown, then validated once into a trusted type.
Genuine opaque external values that are never narrowed (e.g., passed through verbatim).
Abuse is when unknown is introduced solely to make a lint rule pass without establishing a real trust boundary. Indicators:
as unknown / as unknown as T / : unknown applied to a value whose declared type was already specific, immediately followed by a runtime narrowing or re-cast.
Defensive checks (if (x !== null && typeof x === 'object')) that exist only because the value was widened to unknown, where the surrounding field type already promised a non-null shape.
Repeated identical narrowing predicates scattered across readers instead of validating once at the boundary (e.g., isProviderSettingsRecord/isPlainObject duplicated across files).
Internal/private state fields widened to unknown/Record<string, unknown> to accommodate a write path that violates the field's own declared type.
Goal
Produce a codebase-wide audit and remediation plan. This is analysis + tracking first; large refactors should be decomposed into follow-up issues.
Acceptance criteria
Enumerate every as unknown, as unknown as, and internal-state : unknown annotation in packages/**/src (excluding .d.ts and generated files). Provide the inventory as an attached comment (path:line | snippet | rule it silences, if determinable).
Categorize each occurrence as LEGITIMATE BOUNDARY or ABUSE, with a one-line justification. Be conservative: only flag as abuse where the widening exists solely to satisfy a lint rule and no real trust boundary is established.
For each ABUSE occurrence, identify the root cause (e.g., violable invariant, missing boundary validation, duplicated predicate) and propose the correct fix (boundary parse into a trusted type, enforce the invariant at the write site, dedupe predicates, etc.).
Group findings by package and severity. Recommend which are quick wins vs. require a boundary redesign.
File follow-up issues for the non-trivial remediations (one per package or per cluster), referencing this issue.
Where a lint rule is genuinely inappropriate (not just inconvenient), call that out explicitly as a separate project-wide policy question — do NOT bundle it into the abuse list.
The project already depends on zod across core, agents, auth, cli, ide-integration, lsp — the preferred "parse, don't validate" boundary pattern is already available and idiomatic here.
Note on the preferred fix pattern
Where abuse is confirmed, the recommended architectural direction is "parse, don't validate": validate untrusted input once at the entry point (e.g., a zod schema) into a trusted, branded internal type, so that downstream code holds an honest type and defensive checks become genuinely removable (not merely made "necessary" via an unknown cast). This collapses N scattered narrowing sites into one boundary and lets the type system carry the trust instead of fighting it.
Problem
During the review of #2120 (eliminate inline ESLint disables in
packages/settings), a recurring tactical anti-pattern was identified: widening values tounknown(viaas unknowncasts or: unknownannotations) purely to silence@typescript-eslint/no-unnecessary-condition,sonarjs/different-types-comparison, and related rules — without fixing the underlying trust boundary.This is a subtler form of the same bypass the ESLint-cleanup effort (#2078) set out to eliminate. Instead of an inline
eslint-disablecomment, the suppression is moved into the type system: the value is cast tounknownso a defensive check that the compiler would otherwise flag as "unnecessary" becomes "necessary" again. The directive disappears, lint goes green, but the root cause — an internal type that over-promises trust while the data actually crosses untrusted boundaries — is left intact.Concrete example (from #2120,
packages/settings/src/settings/SettingsService.ts)The declared type says
providers[provider]is always a record, so the compiler considers a null/shape check "unnecessary". Rather than make the type honest, the code casts back tounknownand re-narrows at every read site (~7 times in this one class). The defensive check is necessary only because the type invariant is violable:set('providers.openai', 'not-a-record')routes throughsetNestedValueand can write an arbitrary value into the map, bypassing any guarantee that values are records.What counts as "abuse" (in scope)
NOT every use of
unknownis a problem. Legitimate uses include:JSON.parse()→unknown, external/plugin input typedunknown, then validated once into a trusted type.Abuse is when
unknownis introduced solely to make a lint rule pass without establishing a real trust boundary. Indicators:as unknown/as unknown as T/: unknownapplied to a value whose declared type was already specific, immediately followed by a runtime narrowing or re-cast.if (x !== null && typeof x === 'object')) that exist only because the value was widened tounknown, where the surrounding field type already promised a non-null shape.isProviderSettingsRecord/isPlainObjectduplicated across files).unknown/Record<string, unknown>to accommodate a write path that violates the field's own declared type.Goal
Produce a codebase-wide audit and remediation plan. This is analysis + tracking first; large refactors should be decomposed into follow-up issues.
Acceptance criteria
as unknown,as unknown as, and internal-state: unknownannotation inpackages/**/src(excluding.d.tsand generated files). Provide the inventory as an attached comment (path:line | snippet | rule it silences, if determinable).Context
core,agents,auth,cli,ide-integration,lsp— the preferred "parse, don't validate" boundary pattern is already available and idiomatic here.Note on the preferred fix pattern
Where abuse is confirmed, the recommended architectural direction is "parse, don't validate": validate untrusted input once at the entry point (e.g., a zod schema) into a trusted, branded internal type, so that downstream code holds an honest type and defensive checks become genuinely removable (not merely made "necessary" via an
unknowncast). This collapses N scattered narrowing sites into one boundary and lets the type system carry the trust instead of fighting it.