LibreKAT/Ocideck

Watch

Fork 0

feat: bewaak dat een pad uit FilePicker achter een platformpoort staat #527

Merged

brenno merged 1 commit from feat/filepicker-poort into main

2026-07-22 10:32:12 +00:00

brenno commented

2026-07-22 10:32:10 +00:00

Owner

Summary

#150 came back as #506 because nothing guarded the fix. This adds the guard, in tool/check_conventions.dart rather than tool/check_web_hardening.dart — the latter reads build/web, the output of a web build, and cannot reason about source at all.

Closes #505

The rule

A file in lib/ that takes a filesystem path from the file picker must name supportsLocalProjectFolders (or isWebPlatform) itself. Two shapes count:

FilePicker.getDirectoryPath( — no web implementation; returns null without a sound. This was #150.
FilePicker.pickFiles( whose .path is read without withData: true — works on web, but hands back a blob: URL that points nowhere after a reload. This was #506.

saveFile(bytes:) is deliberately excluded: in a browser that is a download and does exactly what it promises.

Why static and not a test

kIsWeb is always false under flutter test, the flag arrives through a conditional import with no injection point, and there is no --platform chrome target. A widget test sits green around the broken branch — which is worse than no test, because it buys confidence that is not there.

The baseline is 7, not 0

I told the issue it would land on zero. It does not, and the reason matters: the class is wider than #150 and #506 suggested. Seven files take a path from the picker without naming the flag:

four dialogs (open_presentation, slide_finder, import_slides, save_destination) that are correctly gated at their caller — the file itself cannot prove it;
lib/services/file_service.dart (2 sites) and lib/services/image_service.dart (3 sites), the latter being precisely the service that handles mem: keys on web;
image_carousel_picker_actions.dart, which is an open bug (#526).

Baselining them is not approval — it is writing them down. Each needs its own decision (hide on web, or take the bytes route), and none of those decisions is safe to make unattended. A stale baseline entry is reported too: a baseline that no longer matches reality reads as "someone thought about this" when nobody did.

Follow-up filed to work the list down.

Verification, in four directions

fires on a planted getDirectoryPath in an ungated file;
fires on a planted pickFiles that reads .path;
stays silent on withData: true — the rule must not punish the correct pattern;
reports a baseline entry that no longer matches anything.

Also in this PR

docs/CHECKS.md said serviceUiImportBaseline is "currently 8"; the code says 4. Corrected while documenting the new rule — one word, in the section being edited.

Gates

make check green. make check-secrets / make sast still red for the pre-existing reasons in #522 and #521.

Principles

No format, storage, dependency or outgoing traffic. Adds a gate, which is the cheap direction: it constrains us, not the user's files.

## Summary #150 came back as #506 because nothing guarded the fix. This adds the guard, in `tool/check_conventions.dart` rather than `tool/check_web_hardening.dart` — the latter reads `build/web`, the *output* of a web build, and cannot reason about source at all. Closes #505 ## The rule A file in `lib/` that takes a filesystem path from the file picker must name `supportsLocalProjectFolders` (or `isWebPlatform`) itself. Two shapes count: - `FilePicker.getDirectoryPath(` — no web implementation; returns `null` without a sound. This was #150. - `FilePicker.pickFiles(` whose `.path` is read without `withData: true` — works on web, but hands back a `blob:` URL that points nowhere after a reload. This was #506. `saveFile(bytes:)` is deliberately excluded: in a browser that is a download and does exactly what it promises. ## Why static and not a test `kIsWeb` is always `false` under `flutter test`, the flag arrives through a conditional import with no injection point, and there is no `--platform chrome` target. A widget test sits *green* around the broken branch — which is worse than no test, because it buys confidence that is not there. ## The baseline is 7, not 0 I told the issue it would land on zero. It does not, and the reason matters: **the class is wider than #150 and #506 suggested.** Seven files take a path from the picker without naming the flag: - four dialogs (`open_presentation`, `slide_finder`, `import_slides`, `save_destination`) that are correctly gated **at their caller** — the file itself cannot prove it; - `lib/services/file_service.dart` (2 sites) and `lib/services/image_service.dart` (3 sites), the latter being precisely the service that handles `mem:` keys on web; - `image_carousel_picker_actions.dart`, which is an open bug (#526). Baselining them is not approval — it is writing them down. Each needs its own decision (hide on web, or take the bytes route), and none of those decisions is safe to make unattended. A **stale** baseline entry is reported too: a baseline that no longer matches reality reads as "someone thought about this" when nobody did. Follow-up filed to work the list down. ## Verification, in four directions - fires on a planted `getDirectoryPath` in an ungated file; - fires on a planted `pickFiles` that reads `.path`; - **stays silent** on `withData: true` — the rule must not punish the correct pattern; - reports a baseline entry that no longer matches anything. ## Also in this PR `docs/CHECKS.md` said `serviceUiImportBaseline` is "currently 8"; the code says 4. Corrected while documenting the new rule — one word, in the section being edited. ## Gates `make check` green. `make check-secrets` / `make sast` still red for the pre-existing reasons in #522 and #521. ## Principles No format, storage, dependency or outgoing traffic. Adds a gate, which is the cheap direction: it constrains us, not the user's files.

brenno added 1 commit

2026-07-22 10:32:10 +00:00

feat(poort): een pad uit FilePicker moet achter een platformpoort staan

CI / Gate (Linux) · Format · Analyze · Coverage (pull_request) Has been cancelled

Details

CI / Test (macos-latest) (pull_request) Has been cancelled

Details

CI / Test (windows-latest) (pull_request) Has been cancelled

Details

CI / Web hardening (pull_request) Has been cancelled

Details

CI / Docs links (pull_request) Has been cancelled

Details

CI / Supply-chain (Trivy · advisory) (pull_request) Has been cancelled

Details

CI / Gate (Linux) · Format · Analyze · Coverage (push) Has been cancelled

Details

CI / Test (macos-latest) (push) Has been cancelled

Details

CI / Test (windows-latest) (push) Has been cancelled

Details

CI / Web hardening (push) Has been cancelled

Details

CI / Docs links (push) Has been cancelled

Details

CI / Supply-chain (Trivy · advisory) (push) Has been cancelled

Details

304b876430

#150 kwam terug als #506 omdat niets de reparatie bewaakte. Een test kan
dat hier niet: kIsWeb is onder flutter test altijd false, de vlag komt via
een conditionele import zonder injectiepunt, en er is geen
--platform chrome-doel. Een widgettest staat dus groen om de kapotte tak
heen. Vandaar een statische regel.

Twee gedaanten tellen mee: getDirectoryPath (bestaat niet op web, geeft
stil null) en pickFiles waarvan .path gelezen wordt zonder withData
(levert een blob:-URL). saveFile(bytes:) telt bewust niet mee — dat is op
web een download en doet wat het belooft.

De basislijn staat op zeven en niet op nul. Dat is geen goedkeuring maar
een opschrijving: vier dialogen poorten bij hun aanroeper, en drie
bestanden (file_service, image_service, de carousel) vragen elk een eigen
afweging. Een verouderde regel in die lijst wordt óók gemeld, want een
basislijn die niet meer klopt leest als "hier is over nagedacht".

Getoetst in vier richtingen: alarm op getDirectoryPath, alarm op
pickFiles→.path, stil bij withData: true, en melding bij een basislijnregel
die nergens meer op slaat.

Closes #505

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>