diff --git a/app/models/assistant/function/upload_account_statement.rb b/app/models/assistant/function/upload_account_statement.rb index 2faf230af..f557e0818 100644 --- a/app/models/assistant/function/upload_account_statement.rb +++ b/app/models/assistant/function/upload_account_statement.rb @@ -118,10 +118,12 @@ class Assistant::Function::UploadAccountStatement < Assistant::Function error("validation_failed", e.record.errors.full_messages.join("; ")) rescue => e # The shared upload path can raise from content sniffing, storage or a - # validation hook. Those would otherwise surface to the agent as a raw - # exception string; give it something it can act on instead. + # validation hook. The agent gets a fixed message, never the exception text: + # a storage failure can carry bucket names, object keys, paths or request + # details, and this response crosses out to an external client. Diagnostics + # stay in the server log. Rails.logger.error("[UploadAccountStatement] #{e.class}: #{e.message}") - error("upload_failed", "The statement could not be stored: #{e.message.truncate(200)}") + error("upload_failed", "The statement could not be stored due to an unexpected error. It has been logged for the administrator.") end private diff --git a/test/models/assistant/function/upload_account_statement_test.rb b/test/models/assistant/function/upload_account_statement_test.rb index 6a115fd5b..3fcf55734 100644 --- a/test/models/assistant/function/upload_account_statement_test.rb +++ b/test/models/assistant/function/upload_account_statement_test.rb @@ -98,29 +98,40 @@ class Assistant::Function::UploadAccountStatementTest < ActiveSupport::TestCase end test "accepts urlsafe base64 without padding" do - encoded = Base64.urlsafe_encode64(@content, padding: false) + # Chosen so the encoding actually uses the URL-safe alphabet; the usual + # fixture encodes to plain base64, which would exercise the padding branch + # while leaving the "-_" translation untested. + content = "a,b\n1,2\n~~~???\n" + encoded = Base64.urlsafe_encode64(content, padding: false) + assert_match(/-/, encoded, "fixture must exercise the urlsafe alphabet") + assert_match(/_/, encoded, "fixture must exercise the urlsafe alphabet") + assert_no_match(/=/, encoded, "fixture must be unpadded") result = @function.call("filename" => "statement.csv", "content_base64" => encoded) assert result[:success] - assert_equal Digest::SHA256.hexdigest(@content), result[:statement][:content_sha256] + assert_equal Digest::SHA256.hexdigest(content), result[:statement][:content_sha256] end - test "rejects content that decodes to zero bytes" do + test "rejects blank content" do + # Base64.strict_encode64("") is "", which is blank, so this never reaches + # the decoder — hence invalid_content rather than empty_file. result = @function.call("filename" => "statement.csv", "content_base64" => Base64.strict_encode64("")) assert_not result[:success] assert_equal "invalid_content", result[:error] end - test "reports an unexpected storage failure as a tool error" do - AccountStatement.stubs(:create_from_prepared_upload!).raises(StandardError, "storage exploded") + test "reports an unexpected storage failure without leaking the exception" do + AccountStatement.stubs(:create_from_prepared_upload!).raises(StandardError, "s3://bucket/secret-key exploded") result = @function.call(params(filename: "statement.csv")) assert_not result[:success] assert_equal "upload_failed", result[:error] - assert_match(/storage exploded/, result[:message]) + # The response crosses out to an external agent, so it must carry none of + # the exception's detail. + assert_no_match(/s3:|bucket|secret-key|exploded/, result[:message]) end test "rejects an unknown account_id rather than silently uploading unlinked" do