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

Remove `extends ModelController` constraints from generic type parameters on action/hook types and use polymorphic `this` for collection, member, and hooks properties. This resolves contravariance issues under strictFunctionTypes that prevented subclasses from using `ModelControllerActions<MyController>`.

- ModelRelation.scope: support array of scopes (602cfbf) - ModelRelation: add modelClass property (17ebe9a) - Storage: add signAssetFile and verifyAssetFile methods (af5d280) - creatable: add query callback option (be1cc3b) - notify: add duration and error options, document all props (ff9922d) - options.equals: fix signature to use DitoContext (07b4821) - CheckboxesSchema: add multiple property (6eb6577)

Type definition for 2f9d3a6 (Add nestedFolders option to DiskStorage).

@example

Replace complex generic hook key derivation with a single ModelControllerHookKey template literal type, breaking a circular type reference that caused TS 6.0 to hang when using polymorphic this on hooks. Use intersection of mapped types so each hook category gets a precise handler signature: - before: (ctx, params?) => void - after member CRUD: (ctx, item: Model) => any - after collection get: (ctx, result: Model[] | Page) => any - after collection mutate: (ctx, item: Model) => any - after delete: (ctx, result: { count }) => any - after custom: (ctx, result: any) => any Export ModelControllerHooks for use in consumer code. Add type tests for hook key patterns, this typing, signatures, and arity. Add JSDoc with @example showing guard, enrich, side effect, and custom action patterns.

searchTerm is always a non-empty string when the search filter callback is invoked, since onSearchChange guards with `if (searchTerm)`.

lehni reviewed Mar 16, 2026

View reviewed changes

tests/tsconfig.json Show resolved Hide resolved

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

puckey added 13 commits April 2, 2026 15:30

Add asset upload/storage E2E tests

8d8859d

- 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)

32d4a0f

Replace test ID prefixes with describe blocks

d9b1335

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

d7d7970

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

10abf61

- 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

343827d

- 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

3521f20

- 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

lehni force-pushed the feature/asset-signatures branch from e395a22 to 3521f20 Compare April 2, 2026 13:31

puckey added 7 commits April 2, 2026 15:57

Add typescript dependency to tests package

cae627f

Add @ditojs/utils dependency to tests package

233147b

Server: Add nestedFolders to DiskStorageConfig type

45103ab

Type definition for 2f9d3a6 (Add nestedFolders option to DiskStorage).

Types: narrow DitoContext.searchTerm to string

c4fe0e5

searchTerm is always a non-empty string when the search filter callback is invoked, since onSearchChange guards with `if (searchTerm)`.

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 20 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