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
Hi, thanks for this library! And for accepting my PR earlier.
I'm trying to use @stoqey/ib with Deno. Its capability-based security model showed that the library probes a number of apparently irrelevant environment variables. Even running a script that only does import("npm:@stoqey/ib"); will request access to env vars like CI, TEAMCITY_VERSION, CIRCLECI and more:
stoqey-env-vars.webm
The workaround of allowing access to all env vars defeats the point of the security model, and allowing each variable individually via --allow-env is cumbersome. Allowing only the relevant variables (IB_*) doesn't work, because the consumer script exits if access is denied to any of the unnecessary variables:
Additionally, there are attempts to read the package's own config/default.json and config/local.json (from the Deno cache).
These requests happen at import time due to top-level code. Some are caused by the colors dep (sabotaged by its maintainer in 2022), others by the unconditional dotenv.config() in src/common/configuration.ts, and others by eager configuration loading (configuration = load() at the bottom of the same file). The result is that every user of the library pays for these side effects on import, even if they:
pass all connection settings explicitly via constructor options
always supply their own logger
never use the sample tools or test config
Proposed solutions
Remove the colors dependency entirely.
The colored logging is a nice-to-have for the bundled tools and default logger, but it can be replaced with a tiny internal ANSI helper (no deps, no env probing) or simply plain output for the default ConsoleLogger. This single change would eliminate a large class of the env probes.
Let config loading be the client's responsibility.
An Interactive Brokers API client library should not be in the business of loading configuration from the filesystem or environment at import time. Its job is to speak the IB protocol. How the application gets host, port, clientId, credentials, rate limits, etc. is an application concern.
This is a clean scope/separation-of-concerns argument. Good precedents exist:
Most modern database clients (pg, mysql2, mongodb, @libsql/client, etc.) expect you to pass connection parameters explicitly.
Cloud SDKs (AWS, GCP, Azure) accept explicit config objects. They may document env var names as a convention, but they don't auto-load them inside the library unless you call a specific loader.
Even dotenv itself is usually treated as an application tool, rather than something a library should depend on and invoke.
Suggestion: Treat configuration as an explicit input to the API client. The IBApi and IBApiNext constructors (and options) should be the single source of truth.
These of course would be breaking changes, probably best saved for 2.0.
Hi, thanks for this library! And for accepting my PR earlier.
I'm trying to use @stoqey/ib with Deno. Its capability-based security model showed that the library probes a number of apparently irrelevant environment variables. Even running a script that only does
import("npm:@stoqey/ib");will request access to env vars likeCI,TEAMCITY_VERSION,CIRCLECIand more:stoqey-env-vars.webm
The workaround of allowing access to all env vars defeats the point of the security model, and allowing each variable individually via --allow-env is cumbersome. Allowing only the relevant variables (
IB_*) doesn't work, because the consumer script exits if access is denied to any of the unnecessary variables:Additionally, there are attempts to read the package's own
config/default.jsonandconfig/local.json(from the Deno cache).These requests happen at import time due to top-level code. Some are caused by the
colorsdep (sabotaged by its maintainer in 2022), others by the unconditionaldotenv.config()insrc/common/configuration.ts, and others by eager configuration loading (configuration = load()at the bottom of the same file). The result is that every user of the library pays for these side effects on import, even if they:loggerProposed solutions
Remove the
colorsdependency entirely.The colored logging is a nice-to-have for the bundled tools and default logger, but it can be replaced with a tiny internal ANSI helper (no deps, no env probing) or simply plain output for the default
ConsoleLogger. This single change would eliminate a large class of the env probes.Let config loading be the client's responsibility.
An Interactive Brokers API client library should not be in the business of loading configuration from the filesystem or environment at import time. Its job is to speak the IB protocol. How the application gets host, port, clientId, credentials, rate limits, etc. is an application concern.
This is a clean scope/separation-of-concerns argument. Good precedents exist:
Suggestion: Treat configuration as an explicit input to the API client. The IBApi and IBApiNext constructors (and options) should be the single source of truth.
These of course would be breaking changes, probably best saved for 2.0.