Where
UserUniversalAPIParams.validate_filename and write_file in src/marketdata/input_types/base.py, exercised on every request via _validate_user_universal_params in src/marketdata/resources/base.py (which sets result_data["filename"] = None on each call specifically to force the validator to run).
Problem A — validation has filesystem side effects, defaulting to CWD
@field_validator("filename")
def validate_filename(cls, file_path: str | Path | None) -> Path:
if file_path is None:
Path("output").mkdir(parents=True, exist_ok=True)
file_path = Path(f"output/{datetime.datetime.now().strftime('%Y%m%d_%H%M%S_%f')}.csv")
- A Pydantic validator mutates the filesystem.
mkdir("output") runs during validation — including for requests that never write a file (JSON/DataFrame/internal output formats), because the validator is re-triggered on every request.
- The location is relative to CWD. Wherever the consumer's process runs — a systemd service with CWD
/, a cron job in $HOME, a container with a read-only filesystem — the SDK silently creates an output/ directory there. Consequences:
- Market data gets written to disk in a location the consumer never chose (mild data-spill concern).
- On read-only filesystems the
mkdir fails with a confusing error on requests that had nothing to do with CSV output.
Problem B — TOCTOU between the exists-check and the write
if file_path.exists():
raise ValueError(f"Filename already exists: {file_path}") # check
...
def write_file(self, content: str) -> str:
self.filename.write_text(content) # use — no re-check
The exists() check runs at validation time, before the HTTP request is sent; the write happens after the response arrives — hundreds of milliseconds later. In that window:
- Silent overwrite race: two threads/processes can validate the same path (both pass the exists-check) and both write. The "never overwrite an existing file" guarantee the validator advertises is not actually enforced; last writer wins, silently.
- Symlink attack (low severity): in a shared/world-writable directory, a local attacker who can predict the path can place a symlink at it after validation.
write_text follows symlinks, so the SDK overwrites the symlink's target with API-response content. Requires a local attacker and a predictable path (the timestamped default has microsecond granularity, so mainly relevant for caller-chosen filenames), but it is the classic check-then-write hole.
More checks cannot fix B — the fix is an atomic exclusive write.
Proposed fix (needs maintainer sign-off)
Keep the ./output/ default (deliberate DX choice, consistent with the SDK family) and keep the validator's early checks as a UX courtesy, but:
- Move the
mkdir out of the validator into write_file so the filesystem is only touched when a file is genuinely written.
- Switch
write_file to exclusive create — open(path, "x") (O_CREAT|O_EXCL) — so the file is created atomically and the write fails if the path appeared (or was symlinked) between validation and write.
Why this is an issue instead of a PR
Per SECURITY.md this is Tier 2 — every remedy changes observable behavior:
- Moving the
mkdir changes when errors surface (validation-time ValueError → write-time error) and stops creating output/ on non-CSV requests (someone could conceivably depend on that).
- Exclusive-create changes the race's failure mode from silent overwrite to
FileExistsError — which is arguably what the existing exists-check already promises, but it is still a behavior change.
Found during the security sweep that produced #41; related discussion in #42.
Where
UserUniversalAPIParams.validate_filenameandwrite_fileinsrc/marketdata/input_types/base.py, exercised on every request via_validate_user_universal_paramsinsrc/marketdata/resources/base.py(which setsresult_data["filename"] = Noneon each call specifically to force the validator to run).Problem A — validation has filesystem side effects, defaulting to CWD
mkdir("output")runs during validation — including for requests that never write a file (JSON/DataFrame/internal output formats), because the validator is re-triggered on every request./, a cron job in$HOME, a container with a read-only filesystem — the SDK silently creates anoutput/directory there. Consequences:mkdirfails with a confusing error on requests that had nothing to do with CSV output.Problem B — TOCTOU between the exists-check and the write
The
exists()check runs at validation time, before the HTTP request is sent; the write happens after the response arrives — hundreds of milliseconds later. In that window:write_textfollows symlinks, so the SDK overwrites the symlink's target with API-response content. Requires a local attacker and a predictable path (the timestamped default has microsecond granularity, so mainly relevant for caller-chosen filenames), but it is the classic check-then-write hole.More checks cannot fix B — the fix is an atomic exclusive write.
Proposed fix (needs maintainer sign-off)
Keep the
./output/default (deliberate DX choice, consistent with the SDK family) and keep the validator's early checks as a UX courtesy, but:mkdirout of the validator intowrite_fileso the filesystem is only touched when a file is genuinely written.write_fileto exclusive create —open(path, "x")(O_CREAT|O_EXCL) — so the file is created atomically and the write fails if the path appeared (or was symlinked) between validation and write.Why this is an issue instead of a PR
Per SECURITY.md this is Tier 2 — every remedy changes observable behavior:
mkdirchanges when errors surface (validation-timeValueError→ write-time error) and stops creatingoutput/on non-CSV requests (someone could conceivably depend on that).FileExistsError— which is arguably what the existing exists-check already promises, but it is still a behavior change.Found during the security sweep that produced #41; related discussion in #42.