>> I think the current docstring of ‘assert-valid-signature’ is not correct >> since ‘signature’ must be a string (as produced by >> ‘canonical-sexp->string’), not an sexp. > In guix/nar.scm, the comment is: > (define (assert-valid-signature signature hash file) > ;; Bail out if SIGNATURE, an sexp, doesn't match HASH, a bytevector > ;; containing the expected hash for FILE. > and indeed, SIGNATURE must be a string here. >> Similarly, the “signature is not a valid s-expression” and “corrupt >> signature data” messages are a bit confusing due to the way >> ‘string->canonical-sexp’ works (try ‘string->canonical-sexp "foo"’). >> But I may be wrong about the latter. > Ah right, you could get “corrupt signature data” when > (string->canonical-sexp signature) returns the null canonical sexp, > whereas you’d want “not a valid s-expression”. > Well, we can fix that in a separate patch if you want. Do you have time for this? I think it would be much easier for you than for me because you wrote the bindings. >> +(define* (assert-valid-signature signature hash port >> + #:optional (acl (current-acl))) >> + ;; Bail out if SIGNATURE, a string, doesn't match HASH, a bytevector >> + ;; containing the expected hash for PORT. > Make it a docstring. > Also, please make this change a separate patch. OK. >> + (let* ((file (port-filename port)) > I don’t think this will work, because most of the time PORT is a pipe > (an input port), whereas FILE is supposed to be the name of the file > being restored. What can we do about it? Should the function accept ‘file’ and ‘port’? > > + (raise (condition (&message (message "invalid hash")) >> + (&nar-invalid-hash-error >> + (port port) (file file) >> + (signature signature) >> + (expected (hash-data->bytevector data)) >> + (actual hash))))) >> + (raise (condition (&message (message "unauthorized public key")) >> + (&nar-signature-error >> + (signature signature) (file file) (port port))))) >> + (raise (condition >> + (&message (message "corrupt signature data")) >> + (&nar-signature-error >> + (signature signature) (file file) (port port))))))) > Actually, the problem with making ‘assert-valid-signature’ public is > that it raises &nar error conditions. > It could be changed to raise a more generic &signature-error, but then > ‘restore-file-set’ would have to guard against it to re-throw it along > with a &nar-error (making a compound condition.) And then ui.scm would > figure it out. Blech. > It’s worth factorizing, but I don’t see how to do it nicely. Thoughts? Haven’t thought of it yet. But I’ll try to take care of this one.