Fix asset file signature verification by puckey · Pull Request #156 · ditojs/dito

puckey · 2026-03-15T21:12:54Z

Summary

Re-enables HMAC signature verification for asset files, which was reverted in d4314d9. The original implementation had two bugs:

createAssets() passed file objects by reference into $parseJson(), which deleted the signature via convertAssetFile(). This mutated the original objects before they were sent back to the client as the upload response — so clients never received signatures.
Files read from the database via $parseDatabaseJson() were not signed on output. When $formatJson() serialized a model for the API, the file objects had no signature, so clients couldn't send them back for updates.

Changes

Storage.js: Add signAssetFile(), verifyAssetFile(), _signAssetKey(), and signature verification in convertAssetFile().

Model.js: Sign asset files in $formatJson() (with cloning to avoid mutating model internals). Pass trusted flag through $parseJson() so DB reads skip verification. Extract _forEachAssetFile() and _signAssetFiles() helpers.

Application.js: Shallow-clone file objects in createAssets() to prevent mutation of the upload response. Sign imported foreign files before createAssets().

AssetMixin.js: Pass trusted flag through $parseJson() override.

Improved type definitions for server, admin, and utils packages. Moved type tests to tests/types/.

Added E2E test infrastructure: PGlite in-memory Postgres, Playwright fixtures, and shared test utilities. Includes asset E2E tests for upload endpoints, asset lifecycle, foreign imports (programmatic and via API), path traversal rejection, and signature forgery.

Test plan

pnpm run test (vitest — includes Storage.test.js signature unit tests and type tests)
pnpm run test:e2e (Playwright — includes asset E2E suite)

puckey · 2026-03-15T21:18:45Z

(I realize these are basically three prs in one, let me know if you would prefer me to split them up)

lehni · 2026-03-16T09:33:39Z

packages/server/src/models/Model.js

+        )
+      }
+    }
+  }


This is great! Pretty sure it could use _forEachAssetFile() to avoid code duplication? Then you could maybe inline signAssetFiles as it would only need to handle single instances?

See 1b51d4b. Couldn't use _forEachAssetFile directly though, since signing needs to clone file objects before mutating them (to avoid leaking signatures onto the model's internal state). Added _mapAssetFiles as a functional counterpart -> _forEachAssetFile visits and _mapAssetFiles transforms and replaces

tests/tsconfig.json

lehni · 2026-03-16T10:08:58Z

@puckey this MR does so much more than what the title says :-)
Thank you so much for the work on this!

- Make ModelRelations and ModelProperties generic for type safety - Add typed ApplicationConfig, ApplicationServices, ApplicationStorages interfaces for module augmentation - Add typed KoaContextState, QueryFilterTypes, Service<Config> - Extend DitoComponentInstanceBase with ComponentPublicInstance - Remove id and foreignKeyId from Model type - Add type tests in tests/types/{admin,server,utils}/

- Add tests workspace with Playwright, PGlite, and shared utilities - Inline PGlite Knex dialect for in-memory Postgres - Add test app factory, database helpers, network helpers, admin helpers - Add tasks-crud server integration test - Add Playwright config and tsconfig for E2E tests

- Admin UI tests: upload, delete, validation, drag reorder, single mode - Server tests: upload endpoint, asset lifecycle, foreign imports - Programmatic tests: AssetFile creation, model hooks

- Re-enable HMAC signature verification (reverts d4314d9) - Clone file objects in createAssets() to prevent signature stripping before the upload response reaches the client - Sign asset files in $formatJson() so DB-sourced models include signatures in API responses - Clone before signing to avoid mutating model internals - Extract _forEachAssetFile and _signAssetFiles helpers

- Test file:// and http:// foreign imports via API POST/PATCH - Test rejection of disallowed import sources via API - Test rejection of disallowed file:// paths (e.g. /etc/passwd) - Test rejection of path traversal out of allowed directory

Use test.describe() for grouping instead of A/B/C number prefixes. Test output now reads e.g. "foreign imports › via API › reject path traversal" instead of "B24: reject path traversal".

Add _mapAssetFiles for functional transformation of asset files, keeping _forEachAssetFile pure for side-effect-only iteration. _signAssetFiles uses _mapAssetFiles to clone and sign files.

- Update action name JSDoc to space-separated convention - Add examples to ControllerAction JSDoc - Clarify `allow` semantics in JSDoc - Fix Authorize to use Partial<Record> for per-method auth - Type ctx.state.user as UserModel instance with index fallback - Remove session.state.user in favor of ctx.state.user - Add type tests for inline action handlers, authorize, and ctx.state

- Fix response object syntax typo - Clarify that default actions require explicit allow listing - Update allow examples to show inline actions are auto-allowed - Fix handler parameter name (msg → message) - Document authorize configuration - Rename UserController to UsersController and document actions - Remove stale TODO and not-yet-implemented notes

- Derive NonOptionFieldComponent from NonSectionComponent via Exclude - Extract OptionComponent type for select/multiselect with typed options

- Fix "requried" → "required", "has hidden" → "as hidden", garbled unsigned description - Add 'text' to SchemaType union - Add message and silent properties to Keyword and Format types

- Add controller action name routing/rejection tests - Add validator tests for range, custom keywords, and custom formats - Add Keyword, Format, and Schema type tests - Support validator option in createTestApp

- Rewrite `_mapAssetFiles` to use `getEntriesAtDataPath` for resolving wildcard data paths (e.g. `sections[*].items[*].items[*].image`) into concrete path/value pairs - Remove `_forEachAssetFile` and consolidate into `_mapAssetFiles`; `$parseJson` now uses `_mapAssetFiles` with `return file` - Remove unused helpers: `forEachAssetFile`, `mapAssetFiles`, `getValueAtAssetDataPath` - Preserve scalar vs array shape when mapping asset files - Add `NestedAssetWidget` model, controller, and admin view for testing - Add `trackApiErrors` test utility - Add E2E tests for deeply nested upload and multi-position wildcards

- Add `test:e2e:prod` script with DITO_E2E_BUILD=production - Export `isProduction` flag from test utils - Build admin to temp dist dir in production mode, serve via dev server otherwise - Add build-mode test asserting correct script tags per mode - Skip drag-reorder test in production (relies on unminified Vue internals)

@ts-expect-error

- Remove `extends ModelController` constraints from action type generics, default to `any` to break self-referencing circularity - Use conditional type for ModelFromModelController - Add @ts-expect-error for intentionally invalid action test - Handle nullable viteConfig in test build helper

lehni reviewed Mar 16, 2026

View reviewed changes

tests/tsconfig.json Show resolved Hide resolved

puckey force-pushed the feature/asset-signatures branch 2 times, most recently from b4aa0fd to 1b51d4b Compare March 16, 2026 10:54

puckey added 19 commits March 19, 2026 15:31

Add asset upload/storage E2E tests

f582591

- Admin UI tests: upload, delete, validation, drag reorder, single mode - Server tests: upload endpoint, asset lifecycle, foreign imports - Programmatic tests: AssetFile creation, model hooks

Add signature forgery E2E tests (B15-B19)

2a16792

Replace test ID prefixes with describe blocks

aa248f6

Use test.describe() for grouping instead of A/B/C number prefixes. Test output now reads e.g. "foreign imports › via API › reject path traversal" instead of "B24: reject path traversal".

Reuse _forEachAssetFile in _signAssetFiles

26888b7

Add _mapAssetFiles for functional transformation of asset files, keeping _forEachAssetFile pure for side-effect-only iteration. _signAssetFiles uses _mapAssetFiles to clone and sign files.

Fix Components type for object-valued properties

097c9ee

- Derive NonOptionFieldComponent from NonSectionComponent via Exclude - Extract OptionComponent type for select/multiselect with typed options

Fix doc typos and add message/silent to Keyword and Format types

3c43685

- Fix "requried" → "required", "has hidden" → "as hidden", garbled unsigned description - Add 'text' to SchemaType union - Add message and silent properties to Keyword and Format types

Add controller and validator tests

cffe7cd

- Add controller action name routing/rejection tests - Add validator tests for range, custom keywords, and custom formats - Add Keyword, Format, and Schema type tests - Support validator option in createTestApp

Avoid duplicate vitest dep

69ea899

Update .nvmrc from 20 to 24

eca3b2c

Add @ditojs/utils dependency to tests package

253e33d

puckey force-pushed the feature/asset-signatures branch from 7101a67 to 7869998 Compare March 19, 2026 15:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix asset file signature verification#156

Fix asset file signature verification#156
puckey wants to merge 19 commits intomainfrom
feature/asset-signatures

puckey commented Mar 15, 2026 •

edited

Loading

Uh oh!

puckey commented Mar 15, 2026

Uh oh!

lehni Mar 16, 2026

Uh oh!

puckey Mar 16, 2026

Uh oh!

Uh oh!

lehni commented Mar 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

puckey commented Mar 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Test plan

Uh oh!

puckey commented Mar 15, 2026

Uh oh!

lehni Mar 16, 2026

Choose a reason for hiding this comment

Uh oh!

puckey Mar 16, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

lehni commented Mar 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

puckey commented Mar 15, 2026 •

edited

Loading