Repository navigation
feat: security & limits settings, verified updates, start-up fixes, accessible donut, quality fixes - #583
Conversation
…ccessible donut, quality fixes One change set in eleven parts. Each part has its own tests; nothing changes for existing settings files (the new sections read as their defaults). Security and limits (new settings page "Security and limits", nav.security) - Documents with active content: before a document goes through LibreOffice, ActiveContentScanner looks for VBA macros (module names from vbaProject.bin, auto-run entry points such as AutoOpen / Document_Open / Workbook_Open), ActiveX controls, external relationships (attached templates, OLE links), DDE fields, ODF Basic / script libraries and event listeners, linked sections and objects, and legacy DOC / XLS / PPT macro storages including Excel 4.0 macro sheets (OpenMcdf, already shipped through MsgReader, now referenced directly). Policy: Ask (default), Convert, Refuse. "Ask" shows ActiveContentWindow once per job with every finding: trust and convert, leave these files out, or cancel; "Don't ask again" stores the choice. Folder watching and the CLI never ask (filee-cli --active-content ask|convert|refuse; --json lists the findings). - LibreOffice hardening: every worker profile gets MacroSecurityLevel=3, DisableMacrosExecution, no link updates in Writer and Calc, BlockUntrustedRefererLinks and VBA Executable=false in registrymodifications.xcu (other entries are kept). Headless --convert-to doesn't run document macros today (checked with LibreOffice 26.8 on a dom:load macro and a Basic UDF); this keeps it so if that ever changes. - Picture limits: largest picture in megapixels (default 500, checked from the header before decoding, so a small "image bomb" is refused with error.image_too_large), longest side, ImageMagick memory and disk; 0 = automatic. - Parallel files (replaces Environment.ProcessorCount in the queue) and a time limit per step for built-in engines (default 15 min, 0 = none; error.conversion_timeout). Engines that run another program keep their own limits (IHasOwnTimeout). ProcessRunner no longer waits forever for the output of a killed process; the msiexec log is deleted after a successful engine install. Updates - FileeExpectedPublisher (Directory.Build.props, empty by default): when set, an update installer must be signed by exactly this Authenticode publisher, even if the installed copy is unsigned. - FileeUpdatePublicKey (ECDSA P-256, empty by default): when set, the release's SHA256SUMS file must carry a valid signature (<file>.sig) on every platform, or the update is refused (fail closed). build/sign-checksums.ps1 creates keys and signatures; release.yml and sign-release.ps1 sign when the key is available. Without the properties nothing changes. Code signing only when possible - sign-windows.ps1 / sign-release.ps1 look for a valid code-signing certificate themselves (build/signing-certificate.ps1), warn and continue unsigned when there is none; -RequireSignature makes a missing certificate an error. Start-up problems - CET crash (the README's workaround from KnifeLemon#486): Filee.Setup is built with CETCompat=false so Setup always starts. On Windows 10 (< 22000) Setup runs "filee-cli.exe version" once after installing; when it dies with 0xC0000409 (shadow stack fail-fast) a machine-wide install applies the README's Set-ProcessMitigation exception for Filee.exe and filee-cli.exe only and notes it in the uninstall entry; the uninstaller removes it again. Windows 11 isn't probed; the app keeps CET everywhere else. - KnifeLemon#450: the General page shows when Windows' Task Manager disabled the startup entry (StartupApproved\Run) and offers to enable it again. Release chain (docs/RELEASE-SECURITY.md explains each step and proposes the next ones) - All actions pinned to commit SHAs (version in a comment), least-privilege permissions and timeouts per job. - New: CodeQL (C#), dependency review, Dependabot (actions, NuGet), weekly engine tests (LibreOffice and others), build provenance attestations and an SBOM for release files. Splitting large classes (moves only, no behaviour change) - JobQueue: .Fetch, .Combine, .Dates, .Outputs, .Sweep, .Allocators; UserDataStore: SettingsMigrations; UpdateService: .Verification, .Linux, .Workspaces; DonutMenu: .Input, .Animation, .Rendering; PresetEditorViewModel: .Apply, .Visibility. Accessible donut (see KnifeLemon#524) - DonutMenuAutomationPeer: the donut is a list, each slice a list item with its label (and caption), the reason a slice is disabled as help text, selection and invoke patterns; hover and keyboard changes are announced. - Focusable with a focus ring; in the toolbar editor arrows choose a slice, Enter / Space / F2 / context-menu key edit it, Ctrl + arrow moves it (screen readers read the keys out as the donut's help text). High contrast: the ring follows the system's contrast theme. Quality issues - KnifeLemon#416 / KnifeLemon#506 (Word → PDF): a first-page header of its own is drawn on the first page, and a tall header gets room instead of overlapping the body. - KnifeLemon#433 (PDF → Word): a picture covering most of a page with a text layer goes behind the text instead of being dropped (OCR layers keep the old behaviour). - KnifeLemon#377 (merged PDF with blank pages): after merging, every page is drawn small; a page blank in the result but not in its source triggers a second merge with PdfPig, keeping PDFsharp's file if that fails too. - KnifeLemon#468 (PSD loses transparency): when the composite image has no alpha but the layers do, the layers are flattened. - KnifeLemon#406 (Markdown → PDF): code blocks in a monospace box that stays together, quotes with a bar and background, list items no longer cut off. - KnifeLemon#384 (HWP stamps): rhwp already draws floating pictures in the cases tested; no change without a sample file. Tests that failed outside a Korean or English system (German Windows 11 here) - OfdTests wrote the OFD XML with the system's number format (Size="4,2333" on a German system): invariant now. - SpreadsheetFormatTests: an ODS by LibreOffice keeps the text Calc showed in its locale ("1.350.000원"), compared without separators now; LibreOffice measures Korean text with its locale's default East Asian font, so the XLS width gets the same tolerance as the ODS one (the column ratio stays checked). - The two preset editor tests set the regional format their expected labels use (labels follow it since 2.1.0). - Engines page: a LibreOffice Filee didn't download now shows its version. LibreOffice 26 dropped MsiProductVersion from version.ini, so CopyVersions reads soffice.exe's product version too; this also brings back "update recommended" for such copies. App tests: 96 minutes → 11 minutes (a run looked hung) - Every language switch wrote the ~1,200 texts into the application's resources one by one, and every write made each DynamicResource in every open window look its key up again: 1 to 4 seconds per switch, and the tests switch languages over a thousand times. LocalizationService now builds the new texts in a dictionary of their own and puts it in the old one's place at once, and does nothing when the language stays. A language switch in the app does one update instead of a thousand as well. Translations: new strings in all 16 languages, pt-PT included (machine-translated; native speakers please check). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
de833d7 to
1fb0242
Compare
KnifeLemon
left a comment
There was a problem hiding this comment.
Thanks @jojofox0815, this is a lot of careful work, and several parts fix real problems: the 96 → 11 minute App test run, the tests that fail on a German system, and the missing LibreOffice 26 version. I'd like to take those. I can't review or merge this as one PR, though. It touches the update and release chain, Setup, signing, the settings UI and five large classes all at once. In Filee each PR does one thing, and new features start with a plan agreed in an issue. You offered to split it, so yes please. Here's how I'd like it.
Welcome now, one small PR each (no issue needed):
LocalizationService: replace the dictionary at once, and skip the work when the language stays the same, plus its test.- Locale-independent tests:
OfdTestsinvariant formatting, theSpreadsheetFormatTestscomparison, the pinned culture in the two preset-editor tests. - Engine small fixes:
CopyVersionsreadingsoffice.exe's product version, theProcessRunnerdrain timeout, deleting the msiexec log after success. - Workflow hardening, existing workflows only: SHA pinning, least-privilege
permissions,timeout-minutes. Please include only thegithub-actionspart of Dependabot so the pins don't go stale. The Windows CI job takes about 63 minutes today, so please giveci.ymlsome headroom over 90, or land this after (1). LibreOfficeSecurity(profile lockdown) on its own. CI skips the LibreOffice tests, so please say how you ran them, including HWP through H2Orestart.
Welcome in principle, but please open or continue an issue with a short plan first, then one PR each:
6. #416/#506, #433 and #406: one PR per issue, with before/after renders.
- #377: let's wait for the reporter's files before adding a render check to every merge.
- #468: we asked whether the alpha is a separate channel, so let's wait for that answer. Also, "composite has no alpha and any layer has alpha" matches almost every normal PSD (an opaque Background plus a text layer); it would replace Photoshop's composite with a re-compose that drops adjustment layers, layer styles and masks. The new
<summary>also landed on top ofFlattenXcf's.
- #450: the reporter says Filee isn't listed in Startup apps at all, so let's confirm there that
StartupApprovedis their problem. - Picture and time limits. Defaults matter here: 500 MP and 15 minutes (also in the CLI) would refuse or stop conversions that work today. I'd start with 0 = off. An abandoned step also keeps running after its slot is freed.
- Active content. Your own test shows headless LibreOffice runs no macros, so the issue should first agree on what we protect against. Under the default "Ask",
/embeddings/(every DOCX/PPTX chart has one) andattachedTemplaterelationships would prompt on very ordinary documents. - Donut accessibility: please coordinate in #524 with @cre8ive-marketing, who has an agreed plan there. Keyboard reordering was deliberately left out for now. (Small correction: this branch's base predates #537/#538; it merges cleanly anyway.)
- CodeQL, dependency review, NuGet Dependabot, weekly engine tests, SBOM and attestations: happy to discuss in one issue, as each adds CI time or PR noise.
Not something we'll take, at least for now:
-
Verified updates (
FileeUpdatePublicKey,FileeExpectedPublisher,sign-checksums.ps1, the signing step inrelease.yml). Anything in the update verification chain needs an agreed threat model and key plan first. As written:- the key would live both in a GitHub secret and on the signing PC, and as job-level env it is visible to every step of the release job, including build, tests and a third-party action;
sign-release.ps1doesn't require a signature when the build has a key, so one missed env var would lock those copies out of updates;- there is no key rotation, and a lost key means a permanent lock-out;
- the signature doesn't bind the version (
FindFilespicks the installer by suffix), so an older signed release could be replayed under a new tag.
If you'd like to pursue it, please open an issue with the threat model, and we can talk there.
-
"Sign only if possible" in
sign-windows.ps1/sign-release.ps1. A release without a signature should fail: every signed installed copy refuses an unsigned update, and certificate auto-discovery could pick the wrong certificate. -
The CET probe and
Set-ProcessMitigationin Setup. We decided to keep CET on and document the workaround (README/FAQ). Setup shouldn't change a Windows security mitigation without asking;-Nameapplies machine-wide by file name,-Removewould also delete settings an admin made, and0xC0000409isn't specific to CET. If Setup itself not starting is the problem, that could be its own issue. -
The class splits. They aren't moves only (the JobQueue and UpdateService splits carry the new behaviour), and the churn would conflict with other open PRs. Let's leave the structure as it is for now.
-
The parts of
docs/RELEASE-SECURITY.md/SECURITY.md/ README that describe the declined pieces. Windows signing stays on the maintainer's PC with SimplySign.
Translations: German (de.json) landed after your base. Don't worry about it; we'll add the German strings as each piece lands.
Thanks again. Pieces 1–3 can go in quickly.
Note
This is one large PR on purpose, so the whole proposal can be reviewed and tried together. I know CONTRIBUTING prefers small PRs. If you'd rather take it in pieces, tell me which parts you want and I'll split them into separate PRs (each part below is self-contained and has its own tests).
What does this change?
A set of improvements from a code review of Filee: a new Settings → Security and limits page (documents with macros, picture limits, time limits, parallel files), verifiable updates, code signing only when a certificate is there, fixes for the two start-up problems, a hardened release workflow, smaller classes, a donut that works with screen readers and the keyboard, and fixes for several conversion-quality issues.
Fixes #450, fixes #416, fixes #506, fixes #433, fixes #468, fixes #406. Related: #486 (README workaround for the CET crash), #377, #384, #524.
1. Documents with macros and other active content (LibreOffice)
AutoOpen,Document_Open,Workbook_Open), ActiveX controls, external relationships, DDE fields, ODF Basic/script libraries and event listeners, linked sections/objects, macro storages in DOC/XLS/PPT including Excel 4.0 macro sheets.filee-cli --active-content ask|convert|refuse;--jsonlists the findings).BlockUntrustedRefererLinksand VBAExecutable=false(existing entries inregistrymodifications.xcuare kept).--convert-todoes not run document macros today (LibreOffice 26.8, adom:loadmacro in an ODT and a Basic UDF in an ODS). That's why there's no "run macros" switch: the profile settings keep it that way if LibreOffice's behaviour ever changes.2. Limits and time limits
error.image_too_large). Also: longest side, ImageMagick memory and disk.Environment.ProcessorCount).error.conversion_timeout). Engines that run another program keep their own limits (IHasOwnTimeout).ProcessRunnerno longer waits forever for a killed process's output; the msiexec log is deleted after a successful engine install.3. Updates
FileeExpectedPublisher(empty by default): an update installer must be signed by exactly this publisher, even if the installed copy is unsigned.FileeUpdatePublicKey(ECDSA P-256, empty by default): the release'sSHA256SUMSmust carry a valid.sig, on every platform, or the update is refused. Keys and signatures come frombuild/sign-checksums.ps1;release.ymlandsign-release.ps1sign when the key is there.SECURITY.mdanddocs/RELEASE-SECURITY.mddescribe both.4. Code signing only when possible
sign-windows.ps1/sign-release.ps1find a valid code-signing certificate themselves, and warn and go on unsigned when there is none.-RequireSignatureturns that into an error.5. Start-up problems
Filee.Setupis built withCETCompat=false, so Setup always starts. On Windows 10 (build < 22000), Setup runsfilee-cli.exe versiononce after installing. If that dies with0xC0000409, a machine-wide install applies the README'sSet-ProcessMitigationexception, forFilee.exeandfilee-cli.exeonly. It's noted in the uninstall entry and removed on uninstall. Windows 11 is not probed. The app keeps CET everywhere else.StartupApproved\Run) and offers to enable it again.6. Release chain
permissionsandtimeout-minutesare set per job.docs/RELEASE-SECURITY.mdexplains each step and suggests next ones: signing in CI instead of on a PC, reproducible builds, signing the Explorer-menu MSIX, and macOS notarization.7. Splitting large classes (moves only)
JobQueue.Fetch,.Combine,.Dates,.Outputs,.Sweep,.AllocatorsUserDataStoreSettingsMigrationsUpdateService.Verification,.Linux,.WorkspacesDonutMenu.Input,.Animation,.RenderingPresetEditorViewModel.Apply,.Visibility8. Accessible donut (please see #524, happy to align with #537 / #538)
Screen readers: the donut is a list. Each slice is a list item with:
Keyboard: the donut has a focus ring. In the toolbar editor:
Screen readers read these keys out as the donut's help text.
High contrast: the ring follows the system's contrast theme.
9. Conversion quality
10. Tests on non-Korean systems, and App tests in 11 minutes instead of 96
DynamicResourcein every open window look its key up again. That's 1 to 4 s per switch, and the tests switch languages more than 1,000 times.LocalizationServicenow swaps in a new dictionary at once and does nothing when the language stays the same.main.OfdTestsbuilt their XML with the system's number format.MsiProductVersionfromversion.ini.CopyVersionsnow readssoffice.exe's product version, which also brings back "update recommended" for such copies.Type of change
How was it tested?
dotnet format --verify-no-changesdotnet build: no new warnings (the same 13 as onmain)dotnet teston Windows 11 (de-DE), rebased onmainafter 2.1.0 (e830916):On
main, 12 of these tests failed on this German system. Section 10 above fixes them:Size="4,2333".App test time: 96 → 11 minutes. Without this PR, a full App run took 96 minutes on a 16-core PC and looked hung. Most of that went into language switches; see section 10 above.
Tried it with
filee-clion Windows 11 (de-DE) with LibreOffice 26.8:dom:loadBasic macro:--active-content refuserefuses it, and--jsonlistsStandard.Module1anddom:load;convertconverts it with the notice "nothing in it was run".New tests:
ActiveContentScannerTests,HardeningAndQualityTests(Engines);JobQueueSecurityTests,SecuritySettingsTests(Core);UpdateSigningTests,DonutAccessibilityTests, plus render tests for the new page and window (App);CetCheckTests(Setup).Not covered by tests:
Screenshots (UI changes)
Settings → Security and limits (new page):
Before a document with macros goes through LibreOffice (policy "Ask"):
The donut with the system's high contrast:
Checklist
src/Filee.App/Assets/i18n/*.jsonfiles (machine-translated; native speakers please check)THIRD-PARTY-NOTICES.md(OpenMcdf is now also used directly)🤖 Generated with Claude Code