fix: accept string node ids from the Files sidebar when requesting a signature

[maia-andre]

Resolves: #8363

📝 Summary

Rewritten after the trace in the comments (previous approach — a generic Number() helper for nodeId/fileId/id — dropped).

@nextcloud/files exposes Node.id as a string; Node.fileid is the deprecated numeric getter and is undefined when the id does not fit a JavaScript number (Nextcloud ≥ 33). tab.ts sends that string to AppFilesTab, which stores it as nodeId, and serializeRequestFile() only accepted numbers — the request went out without file and the API answered 422 File or files parameter is required. Had the string been forwarded, the backend would have failed too: saveFile() swallowed the TypeError of getByNodeId(int) and getNodeFromData() propagated the one of getFileByNodeId(int).

The model is the one agreed above — Node.id (string) → HTTP payload (int | decimal string) → validate/normalize once at the PHP boundary → int → OCP Files API:

Backend (first commit)

• FileInputValidator::normalizeNodeId() turns a string of digits into an int (PHP integers are 64-bit) and leaves any other value untouched for validateFile() to reject as it already does.
• RequestSignatureWorkflowService::createRequest() / updateExistingRequest() apply it once to file and to each entry of files, before the payload reaches the services; the nodeId?: int contract of saveFile() then holds without casts in the services.
• saveFile(): the local $fileId that held a node id is renamed $nodeId.
• LibresignNewFile.nodeId is declared non-negative-int|numeric-string; the parameter description says "an integer or a string of digits". Responses are unchanged.

Frontend (second commit) — specific to nodeId, no Number()

• serializeRequestFile() forwards nodeId when it is a positive integer or a string of digits, as it is. fileId/id keep the number-only checks.
• AppFilesTab's FileInfo.id becomes number | string, matching what tab.ts sends.

About the generated OpenAPI

openapi-extractor maps numeric-string to a plain string and has no pattern support, so the generated schema is

"nodeId": { "oneOf": [ { "type": "integer", "format": "int64", "minimum": 0 }, { "type": "string" } ] }

and the TypeScript type is nodeId?: number | string. The "digits only" part lives in the parameter description and in the validator (422 Invalid fileID for anything else). If you would rather have a stricter schema, I can hand-write it, but it would be lost at the next composer openapi.

LibresignNewFile is also the input of POST /file and POST /id-docs. I kept the normalizer call to the request-signature path, as agreed; those two still pass nodeId raw to getNodeFromData()/saveFile(), so a string there yields the same TypeError 422 as before this PR (no regression). It is the same one-line call at each boundary — tell me if you want them here or in the leftovers issue.

🧪 How to test

Unit tests use 9007199254740993 (Number.MAX_SAFE_INTEGER + 2) and keep the integer path on each changed method:

• PHP: FileInputValidatorTest::testNormalizeNodeId (int, digits, 64-bit, above PHP_INT_MAX, non-numeric, signed, empty, absent); RequestSignatureWorkflowServiceTest — string normalized once for file, integer kept, each entry of files, updateExistingRequest; RequestSignatureServiceTest::testSaveFileReusesTheFileRegisteredForTheNodeId (the nodeId branch of saveFile() had no coverage).
• Vitest: tab.spec.ts keeps the string id of a node without fileid; AppFilesTab.spec.ts passes it through; files.spec.ts sends it unchanged for a single file and for envelope files (two of these fail on main), and the empty id tab.ts falls back to is still not sent.

Local checks: Vitest 127/127 on the three specs, ESLint and vue-tsc clean; php-cs-fixer clean; psalm on the changed files: only the pre-existing MissingDependency in RequestSignatureService; PHPUnit for the four touched PHP classes: OK.

Manual verification (devcontainer, Nextcloud 36 dev)

A real node id above Number.MAX_SAFE_INTEGER: PDF uploaded via WebDAV, its fileid set to 9007199254740993 in oc_filecache (PROPFIND returns it), then Files → Details → LibreSign tab → Add signer. window.OCA.Libresign.fileInfo.id is the string "9007199254740993" (fileid is undefined on that node).

Before (main bundle): saving the signer sends PATCH /request-signature with "file": null → 422 — the message from the issue:

After: the same step sends "file": {"nodeId": "9007199254740993"} → 200; the libresign_file row has node_id = 9007199254740993 (BIGINT), and Request signatures → Send completes:

Signer saved (draft)	Request sent

Two things seen on the way, not touched here: the Files app itself logs Failed to open sidebar on file 9007199254740992 (core, fileid coerced to a number), and the placeholder -fileInfo.id produces a GET /file/validate/file_id/-9007199254740992 404 (the unary-minus leftover already listed).

⚙️ API / Back‑end changes

• LibresignNewFile.nodeId accepts integer | string of digits (request input only); generated openapi*.json and src/types/openapi/*.ts updated
• New FileInputValidator::normalizeNodeId(array $file): array
• Unit tests added (PHP and Vitest)

🚧 Backport

Affected: stable34 (reported) and stable33 (string node ids start there); not stable32. The frontend commit cherry-picks cleanly on both; the backend one does not (no RequestSignatureWorkflowService/FileInputValidator there — the boundary is the controller and the validator is ValidateHelper). After merge I will open manual backports for stable33 and stable34 rather than /backport.

Leftovers for a separate issue (unchanged, as agreed)

openInLibreSignAction.js:77 (fileId carrying a node id), AppFilesTab.vue parseInt on handleNodeDeleted, showStatusInlineAction.js:12, SignFileService.php:143, getFileIdByNodeId() strict === and the -fileInfo.id placeholder key, getSelectedFileView() dropping a string nodeId, and the two other LibresignNewFile consumers above.

✅ Checklist

• I have read and followed the contribution guide.
• Commits 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

✅ All modified and coverable lines are covered by tests.
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Files with missing lines	Coverage Δ
src/components/RightSidebar/AppFilesTab.vue	82.52% <ø> (ø)
src/store/files.js	75.76% <100.00%> (+0.90%)	⬆️

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

[maia-andre]

Rewritten along the lines you described — the description is updated with the new shape, the generated schema and the manual verification.

Short version of the points you asked me to check:

• Boundary: FileInputValidator::normalizeNodeId() runs once in RequestSignatureWorkflowService::createRequest() / updateExistingRequest() (for file and each files[]); nothing else casts. The nodeId?: int contract of saveFile() now holds from there on, and its local $fileId is renamed $nodeId.
• Frontend: the change is specific to nodeId and keeps the string; no Number() / parseInt() added. FileInfo.id in AppFilesTab is number | string, as tab.ts sends.
• Generated OpenAPI: non-negative-int|numeric-string becomes oneOf: [integer/int64/min 0, string] and nodeId?: number | string in TypeScript. openapi-extractor has no pattern support, so "digits only" is stated in the parameter description and enforced by the validator (422 as before for anything else). If you want a stricter schema I can hand-edit it, at the cost of losing it on the next composer openapi.
• Tests: each changed method has the string and the integer path, with 9007199254740993. Manual run with that value as a real fileid: before → 422 "File or files parameter is required"; after → 200 and node_id = 9007199254740993 in libresign_file (screenshots in the description).
• Backports: stable33 and stable34 only. The frontend commit cherry-picks cleanly on both; the backend one needs a manual patch there (the boundary is the controller and the validator is ValidateHelper), so I will open the two backports by hand after the merge.

One open point, in the description: LibresignNewFile is also the input of POST /file and POST /id-docs. I left those untouched (same behaviour as before for a string), it is the same one-line call at each boundary — here or in the leftovers issue, as you prefer.

[vitormattos]

The new direction looks good to me. I think there are only two points left to clarify before this is ready:

1. LibresignNewFile is shared by other endpoints. If nodeId now accepts integer | string, could you check the other consumers and make sure they also support that input? Otherwise, it may be better to keep this wider input type specific to the request-signature API.

2. Since normalizeNodeId() is the boundary where the HTTP value becomes a PHP int, I think it would be safer if it either returns a valid int or rejects the value. Leaving an invalid numeric string untouched can still allow later code to cast it differently.

Other than that, the rewrite looks aligned with the #8363 flow and the test coverage is much better now.