fix: accept numeric string node ids when serializing request files

[maia-andre]

Resolves: #8363

📝 Summary

serializeRequestFile() in src/store/files.js only accepted nodeId, fileId and id when they were numbers. The Files sidebar can hand AppFilesTab a node whose id is a numeric string — tab.ts maps node.fileid ?? node.id, and AppFilesTab.update() stores it as nodeId unchanged — so for a file copied in the Files app and opened in the sidebar before the list is refreshed, the serializer returned null, the request went out without file, and the API answered 422 "File or files parameter is required". The store's own types already declare nodeId?: number | string | null; the serializer was the one place not honouring that.

The fix normalizes the three ids through one helper, toPositiveIntegerId(), which accepts a positive integer or a numeric string (/^\d+$/). Non-numeric envelope placeholders such as 'temp-node' still produce no file reference, which the existing envelope test relies on.

🧪 How to test

Unit tests (src/tests/store/files.spec.ts):

• includes file.nodeId when the temporary file carries a numeric string nodeId — fails on main with expected null to deeply equal { nodeId: 12345 }, passes with the fix;
• serializes envelope files whose nodeId is a numeric string — fails on main with [ { nodeId: 22 } ] (the string entry was dropped), passes with the fix;
• does not turn a non-numeric envelope nodeId into a file reference — guards the 'temp-node' case; passes before and after.

vitest run: 253 files, 3178 tests passed. eslint clean on both files.

Manual, as reported in the issue: copy a PDF in Files, open the right sidebar on the copy, use the Request signature tab, add an email signer and save — the POST /apps/libresign/api/v1/request-signature payload now carries file: { nodeId }.

⚙️ API / Back‑end changes

Frontend only. No API change.

• Unit tests added

🚧 Backport

Bug present in 14.1.0 (stable34). The cherry-pick applies cleanly to stable32, stable33, stable34 and stable35 (checked locally on each branch).

✅ Checklist

• I have read and followed the contribution guide.
• Commit signed off (DCO).

🤖 AI (if applicable)

• The content of this PR was partially or fully generated using AI

[codecov-commenter]

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 81.25000% with 3 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
src/store/files.js	81.25%	3 Missing ⚠️
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Files with missing lines	Coverage Δ
src/store/files.js	75.93% <81.25%> (+1.07%)	⬆️

... and 220 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[vitormattos]

I think we need to follow the ID value through the complete flow before normalizing nodeId, fileId, and id with the same helper.

LibreSign already completed the migration to the current meaning where:

• nodeId / signedNodeId identify Nextcloud nodes;
• id / fileId / parentFileId identify records in libresign_file.

If any current code still uses fileId for a Nextcloud node ID, that should be treated as a leftover from the old model and corrected, not as a valid alternative meaning.

We already found at least one example of this kind of leftover: RequestSignatureService still has a local $fileId whose value comes from Node::getId() or file.nodeId, and it is then passed to getByNodeId(). In other places, fileId correctly goes to getById().

Because of this history, I do not think we should infer what an ID means only from its property name. We need to follow the actual value.

Looking more about this, I found an important Nextcloud change to consider. In the current @nextcloud/files API, Node.id is string | undefined. Node.fileid is the legacy numeric property and is deprecated. The string representation is intentional because Nextcloud is moving to 64-bit snowflake IDs, which cannot always be represented safely as JavaScript numbers.

Nextcloud documents Snowflake IDs here:

https://docs.nextcloud.com/server/stable/developer_manual/digging_deeper/snowflake_ids.html

They were added in Nextcloud 33 and are 64-bit identifiers. The Nextcloud 33 developer release notes also mention that APIs migrated to Snowflake IDs use strings instead of integers:

https://docs.nextcloud.com/server/stable/developer_manual/release_notes/previous/upgrade_to_33.html

Could we first trace the exact #8363 flow and document where the value comes from and what it represents at each step?

For example:

Nextcloud Node -> tab.ts -> AppFilesTab -> files store -> serializeRequestFile() -> request-signature API -> backend lookup

For each value used as nodeId, fileId, or id in this flow, please verify:

• where the value originates;
• whether it identifies a Nextcloud node or a libresign_file row;
• whether it is renamed or transformed on the way;
• which backend lookup finally consumes it (getByNodeId(), getById(), Nextcloud getById(), etc.).

If this flow still uses fileId for a Nextcloud node ID anywhere, that should be corrected as part of the leftover cleanup from the completed migration.

The fix should happen at the point where the representation first becomes incorrect.

In particular, converting a Nextcloud Node.id string with Number() is not safe for future snowflake IDs. A value above Number.MAX_SAFE_INTEGER can silently become a different ID.

I would therefore avoid making serializeRequestFile() generally accept and convert numeric strings until we know which representations are valid for each field.

Please also avoid using an artificial state such as nodeId: 'temp-node' to define the domain model unless production code can really produce that value. The regression tests should reproduce the real sidebar data path as closely as possible.

While following this flow, please also check the test coverage of every method or branch that needs to be changed. If the relevant behavior is not already covered, please add a focused test before or together with the change. The tests should protect the real ID semantics and the complete regression path, not only the final serializer output.

This PR does not need to audit every ID in LibreSign. It should trace and fix the complete #8363 path. If that investigation exposes other leftovers from the old fileId = Nextcloud node ID model, we can handle those in a separate cleanup issue.

[maia-andre]

Thanks — agreed on all three points, and the trace changed my view of where the fix belongs. Below is the #8363 path on main, one row per place the value is read or transformed. The conclusion first: a numeric string in nodeId is a valid representation of a Nextcloud node by Nextcloud's own contract, the frontend serializer is the first place that loses it, and the backend would not accept it either. fileId/id never leave the number domain on this path, so they should not change.

Where the string comes from

resultToNode() in @nextcloud/files 4.0.0 (dist/dav.mjs:232) builds the node with id = props.fileid, where props come from the webdav 5.10 XML parser (fast-xml-parser). I checked the parser's behaviour with the exact options webdav uses (numberParseOptions: { hex: true, leadingZeros: false }):

oc:fileid in the PROPFIND	_data.id
123	number 123
00123	string
9007199254740993 (> MAX_SAFE_INTEGER)	string
12345678901234567890 (64-bit snowflake)	string

Then, in Node itself: get fileid() returns a number only if _data.id is a number, otherwise undefined; get id() returns String(_data.id). So with snowflake IDs fileid is always undefined and id is always a string — the string is not an anomaly of one code path, it is the representation Nextcloud is moving to. (In the reporter's scenario the node comes from client.stat() after copyFile() in apps/files/src/actions/moveOrCopyAction.ts, which goes through the same resultToNode().)

The path

#	Step	Value	Identifies	Renamed / transformed	Consumer
1	src/tab.ts mapNodeToFileInfo() (l. 60)	id: node.fileid ?? node.id ?? '' → number | string	Nextcloud node	none; prefers the deprecated numeric getter, falls back to the string one	AppFilesTab.update()
2	AppFilesTab.vue update() (l. 139–180)	fileInfo.id — the local FileInfo type says id: number, which is not what tab.ts sends	Nextcloud node	selectFileByNodeId(fileInfo.id) → getFileIdByNodeId() compares with ===, so a string never matches the int the API returned; it falls through to getAllFiles({ 'nodeIds[]': [id] }), which the backend binds as PARAM_INT_ARRAY (FileMapper.php:678) — works. If a libresign_file row exists the API record (int nodeId) is selected and the bug does not occur. Otherwise: addFile({ id: -fileInfo.id, nodeId: fileInfo.id, file: { url } }) — id is a local placeholder key (unary minus coerces the string), nodeId is passed through unchanged	files store
3	src/store/files.js apiFiles[-N]	{ id: -N, nodeId: '<string>', file: { url } }	id = local placeholder (never sent), nodeId = Nextcloud node	none	saveOrUpdateSignatureRequest()
4	saveOrUpdateSignatureRequest() (l. 1271–1284)	selectedFile.id (negative, truthy) selects patch; without uuid the controller calls createRequest(), same path as post. Third branch → serializeRequestFile(selectedFile, { preferNodeId: true })		value lost: every nodeId branch requires typeof === 'number'; url lives in file.url, not url; returns null → file omitted	RequestSignatureWorkflowService::createRequest() → $file === [] → 422 "File or files parameter is required"
5	backend, if the string were forwarded	FileInputValidator::validateFile() (l. 80–86) accepts it: is_numeric() + (int) cast	Nextcloud node	cast to int	validateIfNodeIdExists(), validateMimeTypeAcceptedByNodeId()
6	RequestSignatureService::saveFile() (l. 367–372)	$fileId = $data['file']['nodeId'] — the leftover you mentioned: a node id in a variable named $fileId	Nextcloud node	none; passed raw to FileMapper::getByNodeId(int) under strict_types → TypeError, swallowed by catch (\Throwable)	falls through to getNodeFromData()
7	FileService::getNodeFromData() (l. 144–145)	$data['file']['nodeId'] raw	Nextcloud node	none; FolderService::getFileByNodeId(int) under strict_types → TypeError propagates → controller's catch (\Throwable) → 422 with the type error as message	Folder::getById()

fileId and id on this path only ever come from LibreSign's own API (FileListService returns id/nodeId as int), so they stay in the number domain; nothing in the trace justifies accepting strings for them.

Where the fix belongs

Two places make the representation incorrect, and neither should convert with Number():

1. Frontend — serializeRequestFile() forwards nodeId as it is when it is a positive integer or a string of digits; fileId/id keep the number-only checks. AppFilesTab's FileInfo.id type becomes number | string to match tab.ts. Nothing else in the frontend path needs to change for this bug.
2. Backend — saveFile() and getNodeFromData() cast nodeId to int the way FileInputValidator already does (PHP int is 64-bit, so snowflake IDs are safe there), and the local $fileId in saveFile() is renamed to $nodeId. Without this, forwarding the string only moves the 422 from "parameter is required" to a TypeError message.

One decision I need from you — the API contract. LibresignNewFile.nodeId is declared non-negative-int (ResponseDefinitions.php:75, integer/int64 in openapi.json). Options:

• (a) declare nodeId?: non-negative-int|numeric-string and regenerate openapi.json and the TS types — consistent with the Nextcloud 33 note that APIs migrated to snowflake IDs use strings, and the frontend stops sending a value its own schema rejects;
• (b) keep integer in the spec and only tolerate the string server-side.

I lean to (a); it is the honest description of what the endpoint accepts, and it is one line plus generated files.

Tests (real sidebar data, no 'temp-node')

• tab.spec.ts: a real @nextcloud/files File with id: '9007199254740993' → enabled() → fileInfo.id is that string, untouched (fileid is undefined there).
• AppFilesTab.spec.ts: update({ id: '9007199254740993', name, path }) → addFile receives nodeId: '9007199254740993' unchanged.
• files.spec.ts: with that placeholder selected, saveOrUpdateSignatureRequest() sends file: { nodeId: '9007199254740993' }; a string fileId/id is still not accepted.
• RequestSignatureServiceTest: saveFile() with file.nodeId = '35523' calls getByNodeId(35523) — this branch has no coverage today, so I will add the int case as well.
• FileServiceTest: getNodeFromData() with file.nodeId = '35523' calls getFileByNodeId(35523) (the int case exists at l. 223).

Leftovers I will not touch here (for a separate cleanup issue, if you agree)

• src/actions/openInLibreSignAction.js:77 — files: nodes.map(node => ({ fileId: node.fileid ?? node.id })): a Nextcloud node id sent as fileId (the old model).
• AppFilesTab.vue:243 parseInt(rawNodeId, 10) and src/actions/showStatusInlineAction.js:12 — numeric conversions of node ids.
• lib/Service/SignFileService.php:143–144 — same raw nodeId → getByNodeId(int) pattern as saveFile().
• getFileIdByNodeId() strict === between a string node id and the API's int, and the placeholder key derived with unary minus — both lose meaning above MAX_SAFE_INTEGER.

If this reads right to you, I will rewrite the PR along these lines (it becomes frontend + backend; I will check the cherry-pick on stable32–stable35 before asking for backports).

[vitormattos]

Thanks, this trace is much clearer, and the proposed direction looks consistent with what I found as well.

I checked the flow against the current LibreSign code and the current Nextcloud contracts.

The important distinction seems to be:

• in the browser, @nextcloud/files exposes the current node identifier as Node.id: string | undefined;
• Node.fileid is the legacy numeric getter and is deprecated;
• in the Nextcloud PHP Files API, node IDs are still handled as int.

So the flow that makes the most sense for LibreSign is:

Nextcloud Node.id (string) -> LibreSign HTTP payload (string) -> validate/normalize at the PHP boundary -> Nextcloud OCP Files API (int)

This also means converting the value with Number() in JavaScript would be risky, because a valid 64-bit ID can be above Number.MAX_SAFE_INTEGER and lose precision before it reaches PHP.

I would also treat the fast-xml-parser behaviour as supporting context rather than the main reason for the change. It is part of the WebDAV dependency chain, not a LibreSign contract. The public @nextcloud/files Node.id contract already seems enough to define what LibreSign needs to support.

Your trace of the LibreSign path also looks correct to me:

• tab.ts can pass a string Nextcloud node ID;
• AppFilesTab currently declares FileInfo.id only as number, so that type looks inconsistent with what tab.ts can send;
• the serializer is the first place where the value is functionally lost;
• FileInputValidator already accepts a numeric string and converts it to int;
• later, RequestSignatureService::saveFile() and FileService::getNodeFromData() use the raw value again and can reach methods typed as int, so the backend path also needs to be aligned;
• nothing in this flow suggests that fileId or id should start accepting strings.

I think the typing is especially important here.

On the frontend, the type should reflect the real @nextcloud/files contract and keep nodeId in the string domain where required, instead of repeatedly coercing it with Number() or parseInt().

On the backend, I would prefer to validate and normalize the HTTP value once at a clear boundary, then let strong int typing carry that guarantee through the internal flow. If the types are correct, we should not need repeated (int) casts across different services.

The intended model would be:

HTTP nodeId: int | decimal string -> validate/normalize once -> internal nodeId: int -> OCP Files APIs

If the current data structure makes it difficult to propagate the normalized value, a small shared normalizer would be preferable to repeated casts in different methods.

Because of that, I think the safer frontend change would be to keep the fix specific to nodeId, instead of introducing one generic normalization helper for nodeId, fileId, and id.

For the API input, I think we can make the decision here: nodeId should accept both the existing integer representation and a decimal string. The string form is needed to safely carry the value from @nextcloud/files, while keeping integer input avoids breaking existing clients.

Could you also check what OpenAPI is actually generated from the proposed ResponseDefinitions change? I would prefer not to assume that numeric-string produces the schema we want. The important part is that the generated schema and TypeScript types clearly express integer | decimal-string.

Internally, PHP and database values can remain int / BIGINT, following the current Nextcloud server model.

I would keep changing nodeId in API responses from number to string out of this PR unless it turns out to be required to fix #8363. That is a broader compatibility question and should not block this regression fix.

The test plan looks good. Using a real node ID above Number.MAX_SAFE_INTEGER should protect the important part of this regression. It would also be useful to confirm that each method changed by the final fix keeps coverage for the existing integer path as well.

I also agree with leaving the other leftovers out of this PR. Since some old fileId names may only be naming leftovers while others may still represent an old data flow, tracing them individually before changing them seems safer.

For backports, I would check affectedness rather than only whether the patch cherry-picks cleanly.

stable34 is already confirmed by the reported bug. Since the string/snowflake node ID contract starts with Nextcloud 33, stable33 is also worth testing. I do not see a reason to include stable32 only because the patch applies cleanly.

With that, the direction of the rewrite looks good to me.