Skip to content

fix(pi-kilocode): security hardening#13

Closed
iRonin wants to merge 1 commit intosudosubin:mainfrom
iRonin:fix/security-hardening
Closed

fix(pi-kilocode): security hardening#13
iRonin wants to merge 1 commit intosudosubin:mainfrom
iRonin:fix/security-hardening

Conversation

@iRonin
Copy link
Copy Markdown

@iRonin iRonin commented Apr 7, 2026

Summary

Several defensive fixes for scenarios where the Kilo Code API—or a
compromised network path—returns unexpected data.

Changes

src/lib/env.ts

  • Guard PI_CODING_AGENT_DIR against null-byte injection before passing
    it to path.resolve()

src/provider/oauth.ts

  • Cap auth responses at 1 MB (checked via content-length header and
    actual body length) to prevent memory exhaustion
  • Change fetchJson return type from T to T | null; all three
    call-sites (initiateDeviceAuth, pollDeviceAuth, fetchProfile) now
    explicitly guard against a missing or unparseable body instead of
    receiving undefined silently cast as T
  • Reject org IDs containing control characters (CR, LF, NUL, etc.)
    before injecting them into HTTP headers — defence-in-depth alongside
    the protections already in modern fetch implementations

src/provider/models.ts

  • Add isValidCacheFile() type guard; readCache() now validates the
    JSON structure before trusting it, so a tampered local cache file
    cannot inject arbitrary model configuration
  • Cap model-list fetch at 50 MB and assert the data array shape
    before returning

Tests

  • 14 new tests covering each new guard: response size limits, invalid
    JSON, header injection via org ID, cache file tampering

package-lock.json

  • npm audit fix — resolves 4 moderate advisories (brace-expansion,
    fast-xml-parser, yaml, @aws-sdk/xml-builder)

- env: guard PI_CODING_AGENT_DIR against null-byte injection; normalise
  to an absolute path via path.resolve()
- oauth: cap auth responses at 1 MB (content-length header + body length)
  to prevent memory exhaustion from a malicious or compromised server
- oauth: change fetchJson return type from T to T | null; callers now
  explicitly check for null instead of receiving undefined silently cast
  as T, closing a potential null-dereference in all three call-sites
  (initiateDeviceAuth, pollDeviceAuth, fetchProfile)
- oauth: reject org IDs containing control characters (CR, LF, NUL,
  etc.) before injecting them into HTTP headers to prevent header
  injection; defence-in-depth alongside modern fetch implementations
- models: add isValidCacheFile() type guard; readCache() now validates
  the JSON structure before trusting it, so a tampered cache file cannot
  inject arbitrary model configuration
- models: cap model-list fetch at 50 MB and assert the data array shape
  before returning
- tests: 14 new tests covering each guard (size limits, invalid JSON,
  header injection, cache file tampering)
@sudosubin
Copy link
Copy Markdown
Owner

Thanks for the change. After reviewing the threat model, I found that these guards address scenarios that are highly unlikely to accour in this provider, so their utility does not appear to outweigh the complexity of the code. Therefore I'll close this for now.

@sudosubin sudosubin closed this Apr 11, 2026
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.

2 participants