Fix: reject non-default filename/mtime in crypto-refresh builds for #2470

[MakerYuichi]

Description

Fixes an issue where rnp_op_sign_set_file_name() / rnp_op_sign_set_file_mtime() (and the rnp_op_encrypt_* equivalents) silently accept and then discard the filename/mtime when the library is built with ENABLE_CRYPTO_REFRESH, since build_literal_hdr() never writes them to the literal data packet per RFC 9580. FFI callers had no way to detect that their configuration was dropped — it only surfaced later via rnp_op_verify_get_file_info() reporting an empty filename.

This PR makes the setters return RNP_ERROR_NOT_SUPPORTED for non-default values (non-empty filename, non-zero mtime) in crypto-refresh builds. Default values (NULL/empty string, 0) remain accepted as no-ops. Making the setters fail loudly surfaced a second issue: the CLI (fficli.cpp) unconditionally calls these setters with the real source filename/mtime during normal sign/encrypt, not just when the user passes --set-filename, so the new rejection broke default CLI sign/encrypt under crypto-refresh. This PR also updates fficli.cpp to skip that default embedding under ENABLE_CRYPTO_REFRESH, while explicit --set-filename still surfaces a clean error via the same setter.

Fixes #2470

PR Checklist

• Branch is rebased on the latest main.
• clang-format clean (verified via git clang-format --diff).
• All tests pass on at least one backend (Verified 283/283 on legacy config and 291/291 on ENABLE_CRYPTO_REFRESH config, via Botan/macOS local build).
• Added a dedicated regression test that fails without this change (src/tests/issues/2470.cpp).
• Description explains the why and links the relevant issue.
• No observable API changes to public headers in include/rnp/rnp.h beyond the documented new error return for the four affected setters.

[codecov]

Codecov Report

❌ Patch coverage is 85.71429% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.45%. Comparing base (470695b) to head (cb1376a).

Files with missing lines	Patch %	Lines
src/rnp/fficli.cpp	75.00%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2486      +/-   ##
==========================================
- Coverage   85.46%   85.45%   -0.02%     
==========================================
  Files         125      125              
  Lines       23034    23038       +4     
==========================================
+ Hits        19687    19688       +1     
- Misses       3347     3350       +3     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[ronaldtse]

Thank you for this PR — the investigation is spot-on (finding that build_literal_hdr() drops the fields under ENABLE_CRYPTO_REFRESH, and that the CLI itself calls the setters unconditionally during normal sign/encrypt, is a genuinely useful catch).

After discussing on #2470, we've concluded the fix belongs at the packet-writing layer rather than rejecting at the setters: the build flag doesn't actually determine whether filename/mtime can be written — the message version does. In a crypto-refresh build, v4 messages should still carry filename/mtime (RFC 9580 only advises omitting them for v6), so failing the setters for the whole build flavor would close off legitimate v4 use.

Two paths forward — your choice, both appreciated:

(a) You rework this PR onto the version-aware approach; your tests and the CLI adjustment stay, only the FFI-rejection part is replaced by the lower-layer fix. Sketch:

--- a/src/librepgp/stream-write.cpp
+++ b/src/librepgp/stream-write.cpp
@@ build_literal_hdr(const rnp_ctx_t &ctx, pgp_literal_hdr_t &hdr)
-#if defined(ENABLE_CRYPTO_REFRESH)
-    // filename and timestamp SHOULD NOT be set (struct is zero-initialized)
-    return;
-#endif
+    /* RFC 9580: filename and timestamp SHOULD NOT be written for v6 messages;
+     * v4/v5 output keeps them so legacy consumers still see the name. */
+    if (v6_output) {
+        return;
+    }

Call sites: on the encrypt path pass the existing v6 decision (v2 SEIPD, computed in init_encrypted_dst around stream-write.cpp:1116); on the sign path v6_output = all signer keys are v6 (key->version() == PGP_V6, already used at ~1544). Happy to review quickly and help with any part.

(b) We take it from here: we'd open a maintainer PR building on your tests and findings, with Co-authored-by credit to you, and close this one with a note that your report directly shaped the fix.

If we don't hear back within a week or so we'll assume (b) — no rush either way, and thanks again for digging into this.

[ni4]

As for me, it would make sence to introduce some function/method ’bool require_empty_literal_filename()' at some layer so all the check(s) will be done at one place and could be easily update or changed later.