From d945d7514138ecf0ee38d2391b9d25336bb09094 Mon Sep 17 00:00:00 2001 From: "Reichenbach, Michael" <755327+Silthus@users.noreply.github.com> Date: Wed, 4 Nov 2026 06:59:00 +0000 Subject: [PATCH 1/4] feat(emails): send cc, bcc and attachments with emails.send The code form takes cc and bcc (one address or a list) and attachments: Hashes with filename, content, and an optional content_type and content_id. A binary String, such as File.binread returns, or a list of byte values is encoded as Base64 once, before the retry loop; any other String is taken as Base64 already and must look like it. Empty lists are left out, so a PostHog version without the fields still sends. A file the client can't send raises EmailError invalid_attachment with the field, before any request. Mirrors posthog-node (posthog-js#5514) and posthog-python#1172; the server holds the limits. Co-Authored-By: Claude Opus 5.5 --- .changeset/emails-attachments.md | 5 ++ lib/posthog/email_error.rb | 5 +- lib/posthog/emails.rb | 99 ++++++++++++++++++++++++++--- public_api_snapshot.txt | 2 +- spec/posthog/emails_spec.rb | 104 ++++++++++++++++++++++++++++++- 5 files changed, 204 insertions(+), 11 deletions(-) create mode 100644 .changeset/emails-attachments.md diff --git a/.changeset/emails-attachments.md b/.changeset/emails-attachments.md new file mode 100644 index 0000000..e3de20b --- /dev/null +++ b/.changeset/emails-attachments.md @@ -0,0 +1,5 @@ +--- +"posthog-ruby": minor +--- + +`client.emails.send` with `template_key:` takes `cc:`, `bcc:` and `attachments:`. `cc` and `bcc` take one address or a list. Each attachment is a Hash with `filename`, `content`, and an optional `content_type` and `content_id` (`content_id` shows the file inline). Pass the file's bytes as `content`, such as `File.binread('invoice.pdf')` or a list of byte values, and the client encodes them as Base64; any other String is taken as the file already encoded as Base64. A file the client can't send, such as one given by `path`, raises `PostHog::EmailError` with code `invalid_attachment` before any request. diff --git a/lib/posthog/email_error.rb b/lib/posthog/email_error.rb index 0133b7b..30cf847 100644 --- a/lib/posthog/email_error.rb +++ b/lib/posthog/email_error.rb @@ -37,10 +37,13 @@ def initialize(code:, message:, status_code: nil, why: nil, fix: nil, docs_url: @retry_after = retry_after end + SDK_ERROR_NAMES = { 'network_error' => 'service_unavailable', 'invalid_attachment' => 'validation_error' }.freeze + private_constant :SDK_ERROR_NAMES + # @api private def self.name_for(status_code, code) return 'missing_api_key' if code == 'missing_secret_key' || status_code == 401 - return code == 'network_error' ? 'service_unavailable' : 'application_error' if status_code.nil? + return SDK_ERROR_NAMES.fetch(code, 'application_error') if status_code.nil? case status_code when 400 then 'validation_error' diff --git a/lib/posthog/emails.rb b/lib/posthog/emails.rb index cf8953e..8137572 100644 --- a/lib/posthog/emails.rb +++ b/lib/posthog/emails.rb @@ -89,7 +89,9 @@ class Emails "project's engagement setting." CODE_FORM_WHY = 'A send with template_key takes its content from your code.' TEMPLATE_FIELDS = %i[template_data].freeze - CODE_FIELDS = %i[subject html text from reply_to capture_engagement].freeze + CODE_FIELDS = %i[subject html text from reply_to cc bcc attachments capture_engagement].freeze + ATTACHMENT_FIELDS = %w[filename content content_type content_id].freeze + BASE64_PATTERN = %r{\A(?:[A-Za-z0-9+/]{4})*(?:[A-Za-z0-9+/]{2}==|[A-Za-z0-9+/]{3}=)?\z} NETWORK_ERRORS = [ Timeout::Error, SocketError, SystemCallError, IOError, EOFError, OpenSSL::SSL::SSLError, Net::HTTPBadResponse, Net::HTTPHeaderSyntaxError, Net::ProtocolError @@ -97,7 +99,8 @@ class Emails private_constant :SEND_PATH, :STATUS_PATH, :US_APP_HOST, :EU_APP_HOST, :CLOUD_APP_HOSTS, :MAX_RETRY_DELAY_SECONDS, :UUID_PATTERN, :IDEMPOTENCY_KEY_PATTERN, :TEMPLATE_KEY_PATTERN, :HEADER_TOKEN_PATTERN, :SEND_KEY_NEEDED, :READ_KEY_NEEDED, :TEMPLATE_FORM_WHY, - :CODE_FORM_WHY, :TEMPLATE_FIELDS, :CODE_FIELDS, :NETWORK_ERRORS + :CODE_FORM_WHY, :TEMPLATE_FIELDS, :CODE_FIELDS, :ATTACHMENT_FIELDS, :BASE64_PATTERN, + :NETWORK_ERRORS # @api private def initialize(secret_key:, host:, app_host:, timeout:, max_retries:, migrated_from:, user_agent:, disabled:) @@ -133,6 +136,18 @@ def initialize(secret_key:, host:, app_host:, timeout:, max_retries:, migrated_f # @param from [String] Code form: a verified sender, such as `Acme `. Without it, the # project's default sender. # @param reply_to [String, Array] Code form: one address or a list of addresses. + # @param cc [String, Array] Code form: one address or a list of addresses that get a visible copy, at + # most 10. An empty list sends no copy. + # @param bcc [String, Array] Code form: one address or a list of addresses that get a hidden copy, at + # most 10. An empty list sends no copy. + # @param attachments [Array] Code form: files sent with the email, at most 10 and 512 KB in total. Each + # file is a Hash with: + # - +filename+ [String] the name the recipient sees, such as `invoice.pdf`; + # - +content+ [String, Array] the file's bytes, as a binary String such as `File.binread(path)` + # returns or as a list of byte values, which the client encodes as Base64. Any other String is taken as the + # file already Base64 encoded. PostHog can't fetch a file from a path or URL; + # - +content_type+ [String] the MIME type, optional. Without it, the type follows the file name; + # - +content_id+ [String] optional: shows the file inline where `html` has `` with this ID. # @param capture_engagement [Boolean] Code form: whether PostHog captures the email's opens and clicks as events # of the recipient. Defaults to +true+. A project whose admin turned engagement capture off captures nothing. # @param distinct_id [String] The recipient's distinct ID in your product. The email's events go to this @@ -145,14 +160,16 @@ def initialize(secret_key:, host:, app_host:, timeout:, max_retries:, migrated_f # retries off. # @return [SendResult] The queued email. # @raise [EmailError] PostHog did not queue the email, or the client could not tell whether it did. + # {EmailError#code} is `invalid_attachment`, before any request, for a file the client can't send. # @raise [ArgumentError] An argument has the wrong type or format, or the call mixes the two forms. def send( to:, template_id: nil, template_data: nil, template_key: nil, subject: nil, html: nil, text: nil, from: nil, - reply_to: nil, capture_engagement: nil, distinct_id: nil, idempotency_key: nil, max_retries: nil + reply_to: nil, cc: nil, bcc: nil, attachments: nil, capture_engagement: nil, distinct_id: nil, + idempotency_key: nil, max_retries: nil ) fields = { template_data: template_data, subject: subject, html: html, text: text, from: from, reply_to: reply_to, - capture_engagement: capture_engagement + cc: cc, bcc: bcc, attachments: attachments, capture_engagement: capture_engagement }.compact check_recipient(to, distinct_id, idempotency_key, max_retries) payload = content(template_id, template_key, fields).merge(to: to) @@ -292,7 +309,14 @@ def code_content(template_key, fields) end code = fields.slice(:subject, :html, :text, :from).merge(template_key: template_key) - code[:reply_to] = addresses(fields[:reply_to]) if fields.key?(:reply_to) + code[:reply_to] = addresses(:reply_to, fields[:reply_to]) if fields.key?(:reply_to) + # An empty list goes out as no list, because a PostHog version without the field refuses it even empty. + %i[cc bcc].each do |name| + list = addresses(name, fields[name]) if fields.key?(name) + code[name] = list unless list.nil? || list.empty? + end + files = attachment_list(fields[:attachments]) if fields.key?(:attachments) + code[:attachments] = files unless files.nil? || files.empty? code[:capture_engagement] = fields.fetch(:capture_engagement, true) code end @@ -305,11 +329,70 @@ def refuse_fields(fields, allowed:, form:, other_form:, why:) raise ArgumentError, "#{message} #{why} Remove #{field}, or send with #{other_form}." end - def addresses(reply_to) - list = reply_to.is_a?(String) ? [reply_to] : reply_to + def addresses(name, value) + list = value.is_a?(String) ? [value] : value return list if list.is_a?(Array) && list.all? { |address| nonempty_string?(address) } - raise ArgumentError, 'reply_to must be one address or a list of addresses.' + raise ArgumentError, "#{name} must be one address or a list of addresses." + end + + def attachment_list(files) + raise invalid_attachment('attachments must be a list of files.', 'attachments') unless files.is_a?(Array) + + files.each_with_index.map { |file, index| attachment(file, "attachments__#{index}") } + end + + def attachment(file, field) + unless file.is_a?(Hash) + raise invalid_attachment("An attachment is a Hash with filename and content. Pass the file's bytes as its " \ + 'content.', field) + end + + file = file.transform_keys(&:to_s).compact + if file.key?('path') + raise invalid_attachment("PostHog can't fetch an attachment from a path or URL. Read the file with " \ + 'File.binread and pass it as content instead.', "#{field}__path") + end + unknown = file.keys.find { |key| !ATTACHMENT_FIELDS.include?(key) } + if unknown + raise invalid_attachment("An attachment takes filename, content, content_type and content_id, not #{unknown}.", + "#{field}__#{unknown}") + end + + %w[filename content_type content_id].each do |name| + next if name != 'filename' && !file.key?(name) + next if file[name].is_a?(String) + + raise invalid_attachment("An attachment's #{name} must be a String.", "#{field}__#{name}") + end + file.merge('content' => base64_content(file['content'], "#{field}__content")) + end + + # A binary String or a list of byte values is the file itself. Any other String is the file already in Base64: + # Ruby has one String type for bytes and text, and File.binread returns a binary one. + def base64_content(content, field) + bytes = file_bytes(content) + return [bytes].pack('m0') if bytes + return content if content.is_a?(String) && BASE64_PATTERN.match?(content) + + if content.is_a?(String) + raise invalid_attachment("An attachment's content is text that isn't Base64. Pass the file's bytes, read " \ + 'with File.binread, or the file encoded as Base64.', field) + end + raise invalid_attachment('An attachment needs content: the bytes File.binread returns, a list of byte values, ' \ + 'or the file encoded as Base64.', field) + end + + def file_bytes(content) + return content if content.is_a?(String) && content.encoding == Encoding::BINARY + return unless content.is_a?(Array) && content.all? { |byte| byte.is_a?(Integer) && byte.between?(0, 255) } + + content.pack('C*') + end + + # The message never holds the file's content, so the error is safe to log. + def invalid_attachment(message, field) + EmailError.new(code: 'invalid_attachment', message: message, field: field) end def call(nothing_done, key_needed) diff --git a/public_api_snapshot.txt b/public_api_snapshot.txt index 16679ca..4417a85 100644 --- a/public_api_snapshot.txt +++ b/public_api_snapshot.txt @@ -74,7 +74,7 @@ instance_method PostHog::EmailError#status_code() instance_method PostHog::EmailError#why() class PostHog::Emails instance_method PostHog::Emails#get(message_id) -instance_method PostHog::Emails#send(to:, template_id: ..., template_data: ..., template_key: ..., subject: ..., html: ..., text: ..., from: ..., reply_to: ..., capture_engagement: ..., distinct_id: ..., idempotency_key: ..., max_retries: ...) +instance_method PostHog::Emails#send(to:, template_id: ..., template_data: ..., template_key: ..., subject: ..., html: ..., text: ..., from: ..., reply_to: ..., cc: ..., bcc: ..., attachments: ..., capture_engagement: ..., distinct_id: ..., idempotency_key: ..., max_retries: ...) class_method PostHog::Emails.logger() class PostHog::Emails::SendResult < Struct instance_method PostHog::Emails::SendResult#id() diff --git a/spec/posthog/emails_spec.rb b/spec/posthog/emails_spec.rb index 232b132..6616e36 100644 --- a/spec/posthog/emails_spec.rb +++ b/spec/posthog/emails_spec.rb @@ -3,6 +3,12 @@ require 'spec_helper' module PostHog + module Base64Check + def self.decode(text) + text.unpack1('m0') + end + end + describe Emails do let(:send_url) { 'https://us.posthog.com/api/projects/@current/messaging/send/email/' } let(:template_id) { '0190f6d2-7c1e-7b3a-9f5e-1c2d3e4f5a6b' } @@ -144,6 +150,102 @@ def send_code(**overrides) end end + describe 'copies and attachments' do + let(:pdf) { "%PDF-1.7\n\xFF\x00\xE2".b } + + def refused_attachment(**overrides) + send_code(**overrides) + rescue EmailError => e + e + end + + it 'sends cc and bcc as lists' do + stub_request(:post, send_url).to_return(accept) + + send_code(cc: 'finance@example.com', bcc: %w[audit@example.com archive@example.com]) + + expect(sent_bodies.first).to include('cc' => ['finance@example.com'], + 'bcc' => %w[audit@example.com archive@example.com]) + end + + it 'sends the bytes of each file as Base64, with its type and content ID' do + stub_request(:post, send_url).to_return(accept) + + send_code(attachments: [ + { filename: 'invoice.pdf', content: pdf, content_type: 'application/pdf' }, + { 'filename' => 'logo.png', 'content' => [137, 80, 78, 71], 'content_id' => 'logo' }, + { filename: 'notes.txt', content: 'aGVsbG8=' } + ]) + + expect(sent_bodies.first['attachments']).to eq([ + { 'filename' => 'invoice.pdf', + 'content' => 'JVBERi0xLjcK/wDi', + 'content_type' => 'application/pdf' }, + { 'filename' => 'logo.png', 'content' => 'iVBORw==', + 'content_id' => 'logo' }, + { 'filename' => 'notes.txt', 'content' => 'aGVsbG8=' } + ]) + expect(Base64Check.decode(sent_bodies.first['attachments'][0]['content'])).to eq(pdf) + end + + it 'leaves out empty lists, so a PostHog version without these fields still sends the email' do + stub_request(:post, send_url).to_return(accept) + + send_code(cc: [], bcc: [], attachments: []) + + expect(sent_bodies.first.keys).not_to include('cc', 'bcc', 'attachments') + end + + it 'sends the same encoded files on every attempt' do + stub_request(:post, send_url).to_return(refuse(503, 'service_unavailable'), accept) + + send_code(attachments: [{ filename: 'invoice.pdf', content: pdf }]) + + expect(a_request(:post, send_url)).to have_been_made.twice + expect(sent_bodies.map { |body| body['attachments'] }) + .to eq([[{ 'filename' => 'invoice.pdf', 'content' => 'JVBERi0xLjcK/wDi' }]]) + end + + it 'refuses a file it cannot send before any request, naming the field and never the content' do + cases = { + { attachments: { filename: 'a.pdf', content: pdf } } => ['attachments', /list of files/], + { attachments: ['a.pdf'] } => ['attachments__0', /Hash with filename and content/], + { attachments: [{ filename: 'a.pdf', path: '/tmp/a.pdf' }] } => ['attachments__0__path', /File.binread/], + { attachments: [{ filename: 'a.pdf', content: pdf, headers: {} }] } => + ['attachments__0__headers', /not headers/], + { attachments: [{ filename: 'a.pdf' }] } => ['attachments__0__content', /needs content/], + { attachments: [{ filename: 'a.pdf', content: [256] }] } => ['attachments__0__content', /needs content/], + { attachments: [{ filename: 'a.pdf', content: 'secret text, not Base64' }] } => + ['attachments__0__content', /isn't Base64/], + { attachments: [{ filename: :a, content: pdf }] } => ['attachments__0__filename', /filename/], + { attachments: [{ filename: 'a.pdf', content: pdf, content_type: 1 }] } => + ['attachments__0__content_type', /content_type/] + } + + cases.each do |arguments, (field, message)| + error = refused_attachment(**arguments) + expect(error).to be_a(EmailError), "#{arguments} raised #{error.inspect}" + expect([error.code, error.name, error.field]).to eq(['invalid_attachment', 'validation_error', field]) + expect(error.message).to match(message) + expect(error.message).not_to include('secret text') + end + expect(a_request(:any, %r{/messaging/})).not_to have_been_made + end + + it 'refuses an address list that is not one', :aggregate_failures do + expect { send_code(cc: [nil]) }.to raise_error(ArgumentError, /cc must be one address or a list/) + expect { send_code(bcc: 3) }.to raise_error(ArgumentError, /bcc must be one address or a list/) + end + + it 'refuses them on a template send, because the template sets its own recipients and files' do + %i[cc bcc attachments].each do |field| + expect { send_template(field => []) } + .to raise_error(ArgumentError, /template_id doesn't take #{field}.*send with template_key/) + end + expect(a_request(:any, %r{/messaging/})).not_to have_been_made + end + end + describe 'argument checks' do it 'needs exactly one form' do expect { emails.send(to: 'user@example.com') }.to raise_error(ArgumentError, /either template_id, or/) @@ -159,7 +261,7 @@ def send_code(**overrides) expect { send_template(template_data: []) }.to raise_error(ArgumentError, /template_data must be a Hash/) expect { send_code(capture_engagement: 'no') }.to raise_error(ArgumentError, /capture_engagement/) expect { send_code(reply_to: [1]) }.to raise_error(ArgumentError, /reply_to/) - expect { send_template(cc: 'x@example.com') }.to raise_error(ArgumentError, /unknown keyword: :cc/) + expect { send_template(headers: {}) }.to raise_error(ArgumentError, /unknown keyword: :headers/) expect(a_request(:any, %r{/messaging/})).not_to have_been_made end end From a3a1aab0703b822a56b2d64e92572acdef02b0fa Mon Sep 17 00:00:00 2001 From: "Reichenbach, Michael" <755327+Silthus@users.noreply.github.com> Date: Wed, 4 Nov 2026 07:40:03 +0000 Subject: [PATCH 2/4] fix(emails): refuse non-ASCII content as invalid_attachment, take wrapped Base64 - A String that isn't ASCII, such as a PDF read with File.read or UTF-16 text, raised a bare ArgumentError from the Base64 match. It now raises invalid_attachment with the field and the File.binread hint. - Base64 with line breaks, as Base64.encode64 writes it, is sent without them instead of being refused. - The retry spec checks both attempts carry the encoded file, and the Base64Check helper is inlined. Co-Authored-By: Claude Opus 5.5 --- lib/posthog/emails.rb | 18 +++++++++++------- spec/posthog/emails_spec.rb | 28 ++++++++++++++++++---------- 2 files changed, 29 insertions(+), 17 deletions(-) diff --git a/lib/posthog/emails.rb b/lib/posthog/emails.rb index 2a46c93..186c7f8 100644 --- a/lib/posthog/emails.rb +++ b/lib/posthog/emails.rb @@ -388,14 +388,18 @@ def attachment(file, field) def base64_content(content, field) bytes = file_bytes(content) return [bytes].pack('m0') if bytes - return content if content.is_a?(String) && BASE64_PATTERN.match?(content) - - if content.is_a?(String) - raise invalid_attachment("An attachment's content is text that isn't Base64. Pass the file's bytes, read " \ - 'with File.binread, or the file encoded as Base64.', field) + unless content.is_a?(String) + raise invalid_attachment('An attachment needs content: the bytes File.binread returns, a list of byte ' \ + 'values, or the file encoded as Base64.', field) end - raise invalid_attachment('An attachment needs content: the bytes File.binread returns, a list of byte values, ' \ - 'or the file encoded as Base64.', field) + + # Base64.encode64 breaks lines every 60 characters, and PostHog takes Base64 without them. A String that isn't + # ASCII, such as a PDF read with File.read, can't be Base64. + encoded = content.ascii_only? && content.delete("\r\n") + return encoded if encoded && BASE64_PATTERN.match?(encoded) + + raise invalid_attachment("An attachment's content is text that isn't Base64. Pass the file's bytes, read " \ + 'with File.binread, or the file encoded as Base64.', field) end def file_bytes(content) diff --git a/spec/posthog/emails_spec.rb b/spec/posthog/emails_spec.rb index 5abb19b..37ed20a 100644 --- a/spec/posthog/emails_spec.rb +++ b/spec/posthog/emails_spec.rb @@ -3,12 +3,6 @@ require 'spec_helper' module PostHog - module Base64Check - def self.decode(text) - text.unpack1('m0') - end - end - describe Emails do let(:send_url) { 'https://us.posthog.com/api/projects/@current/messaging/send/email/' } let(:template_id) { '0190f6d2-7c1e-7b3a-9f5e-1c2d3e4f5a6b' } @@ -239,7 +233,7 @@ def refused_attachment(**overrides) 'content_id' => 'logo' }, { 'filename' => 'notes.txt', 'content' => 'aGVsbG8=' } ]) - expect(Base64Check.decode(sent_bodies.first['attachments'][0]['content'])).to eq(pdf) + expect(sent_bodies.first['attachments'][0]['content'].unpack1('m0')).to eq(pdf) end it 'leaves out empty lists, so a PostHog version without these fields still sends the email' do @@ -255,9 +249,19 @@ def refused_attachment(**overrides) send_code(attachments: [{ filename: 'invoice.pdf', content: pdf }]) - expect(a_request(:post, send_url)).to have_been_made.twice - expect(sent_bodies.map { |body| body['attachments'] }) - .to eq([[{ 'filename' => 'invoice.pdf', 'content' => 'JVBERi0xLjcK/wDi' }]]) + encoded_once = [{ 'filename' => 'invoice.pdf', 'content' => 'JVBERi0xLjcK/wDi' }] + expect(a_request(:post, send_url).with { |request| JSON.parse(request.body)['attachments'] == encoded_once }) + .to have_been_made.twice + end + + it 'takes Base64 with line breaks, as Base64.encode64 writes it' do + stub_request(:post, send_url).to_return(accept) + file = (0..99).to_a.pack('C*') + + send_code(attachments: [{ filename: 'data.bin', content: [file].pack('m') }]) + + expect([file].pack('m')).to include("\n") + expect(sent_bodies.first['attachments'][0]['content']).to eq([file].pack('m0')) end it 'refuses a file it cannot send before any request, naming the field and never the content' do @@ -271,6 +275,10 @@ def refused_attachment(**overrides) { attachments: [{ filename: 'a.pdf', content: [256] }] } => ['attachments__0__content', /needs content/], { attachments: [{ filename: 'a.pdf', content: 'secret text, not Base64' }] } => ['attachments__0__content', /isn't Base64/], + { attachments: [{ filename: 'a.pdf', content: "%PDF secret text \xE2\xE3".dup.force_encoding('UTF-8') }] } => + ['attachments__0__content', /File.binread/], + { attachments: [{ filename: 'a.txt', content: 'aGVsbG8='.encode('UTF-16LE') }] } => + ['attachments__0__content', /isn't Base64/], { attachments: [{ filename: :a, content: pdf }] } => ['attachments__0__filename', /filename/], { attachments: [{ filename: 'a.pdf', content: pdf, content_type: 1 }] } => ['attachments__0__content_type', /content_type/] From e29b8a735c96434a3d3b733d9ef36cf552183dc8 Mon Sep 17 00:00:00 2001 From: "Reichenbach, Michael" <755327+Silthus@users.noreply.github.com> Date: Wed, 4 Nov 2026 07:44:58 +0000 Subject: [PATCH 3/4] fix(emails): take line-broken Base64 only in Base64.encode64's layout Removing every line break let a text file read with File.read, such as "ABCD1234\nEFGH5678\n", pass as Base64, so the recipient got a garbled file. Line breaks are now removed only when the content is exactly what Base64.encode64 writes; other text is refused as invalid_attachment. Co-Authored-By: Claude Opus 5.5 --- lib/posthog/emails.rb | 11 +++++++---- spec/posthog/emails_spec.rb | 4 +++- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/lib/posthog/emails.rb b/lib/posthog/emails.rb index 186c7f8..c7d2946 100644 --- a/lib/posthog/emails.rb +++ b/lib/posthog/emails.rb @@ -393,10 +393,13 @@ def base64_content(content, field) 'values, or the file encoded as Base64.', field) end - # Base64.encode64 breaks lines every 60 characters, and PostHog takes Base64 without them. A String that isn't - # ASCII, such as a PDF read with File.read, can't be Base64. - encoded = content.ascii_only? && content.delete("\r\n") - return encoded if encoded && BASE64_PATTERN.match?(encoded) + # A String that isn't ASCII, such as a PDF read with File.read, can't be Base64. + if content.ascii_only? + return content if BASE64_PATTERN.match?(content) + # PostHog takes Base64 without line breaks. Only Base64.encode64's own layout loses them, so a text file + # that happens to hold Base64 characters on several lines is still refused. + return content.delete("\n") if content.include?("\n") && [content.unpack1('m')].pack('m') == content + end raise invalid_attachment("An attachment's content is text that isn't Base64. Pass the file's bytes, read " \ 'with File.binread, or the file encoded as Base64.', field) diff --git a/spec/posthog/emails_spec.rb b/spec/posthog/emails_spec.rb index 37ed20a..e455a60 100644 --- a/spec/posthog/emails_spec.rb +++ b/spec/posthog/emails_spec.rb @@ -276,7 +276,9 @@ def refused_attachment(**overrides) { attachments: [{ filename: 'a.pdf', content: 'secret text, not Base64' }] } => ['attachments__0__content', /isn't Base64/], { attachments: [{ filename: 'a.pdf', content: "%PDF secret text \xE2\xE3".dup.force_encoding('UTF-8') }] } => - ['attachments__0__content', /File.binread/], + ['attachments__0__content', /isn't Base64/], + { attachments: [{ filename: 'codes.txt', content: "ABCD1234\nEFGH5678\n" }] } => + ['attachments__0__content', /isn't Base64/], { attachments: [{ filename: 'a.txt', content: 'aGVsbG8='.encode('UTF-16LE') }] } => ['attachments__0__content', /isn't Base64/], { attachments: [{ filename: :a, content: pdf }] } => ['attachments__0__filename', /filename/], From df3ff9200273011a7c1079b4d097252d5b2b985d Mon Sep 17 00:00:00 2001 From: "Reichenbach, Michael" <755327+Silthus@users.noreply.github.com> Date: Wed, 4 Nov 2026 09:16:35 +0000 Subject: [PATCH 4/4] refactor(emails): name the attachment rules instead of commenting them Co-Authored-By: Claude Opus 5.5 --- lib/posthog/emails.rb | 34 ++++++++++++++++------------------ 1 file changed, 16 insertions(+), 18 deletions(-) diff --git a/lib/posthog/emails.rb b/lib/posthog/emails.rb index c7d2946..59e2e13 100644 --- a/lib/posthog/emails.rb +++ b/lib/posthog/emails.rb @@ -325,13 +325,8 @@ def code_content(template_key, fields) code = fields.slice(:subject, :html, :text, :from).merge(template_key: template_key) code[:reply_to] = addresses(:reply_to, fields[:reply_to]) if fields.key?(:reply_to) - # An empty list goes out as no list, because a PostHog version without the field refuses it even empty. - %i[cc bcc].each do |name| - list = addresses(name, fields[name]) if fields.key?(name) - code[name] = list unless list.nil? || list.empty? - end - files = attachment_list(fields[:attachments]) if fields.key?(:attachments) - code[:attachments] = files unless files.nil? || files.empty? + %i[cc bcc].each { |name| put_unless_empty(code, name, addresses(name, fields[name])) if fields.key?(name) } + put_unless_empty(code, :attachments, attachment_list(fields[:attachments])) if fields.key?(:attachments) code[:capture_engagement] = fields.fetch(:capture_engagement, true) code end @@ -344,6 +339,10 @@ def refuse_fields(fields, allowed:, form:, other_form:, why:) raise ArgumentError, "#{message} #{why} Remove #{field}, or send with #{other_form}." end + def put_unless_empty(code, name, list) + code[name] = list unless list.empty? + end + def addresses(name, value) list = value.is_a?(String) ? [value] : value return list if list.is_a?(Array) && list.all? { |address| nonempty_string?(address) } @@ -383,8 +382,6 @@ def attachment(file, field) file.merge('content' => base64_content(file['content'], "#{field}__content")) end - # A binary String or a list of byte values is the file itself. Any other String is the file already in Base64: - # Ruby has one String type for bytes and text, and File.binread returns a binary one. def base64_content(content, field) bytes = file_bytes(content) return [bytes].pack('m0') if bytes @@ -392,19 +389,21 @@ def base64_content(content, field) raise invalid_attachment('An attachment needs content: the bytes File.binread returns, a list of byte ' \ 'values, or the file encoded as Base64.', field) end - - # A String that isn't ASCII, such as a PDF read with File.read, can't be Base64. - if content.ascii_only? - return content if BASE64_PATTERN.match?(content) - # PostHog takes Base64 without line breaks. Only Base64.encode64's own layout loses them, so a text file - # that happens to hold Base64 characters on several lines is still refused. - return content.delete("\n") if content.include?("\n") && [content.unpack1('m')].pack('m') == content - end + return content if single_line_base64?(content) + return content.delete("\n") if encode64_layout?(content) raise invalid_attachment("An attachment's content is text that isn't Base64. Pass the file's bytes, read " \ 'with File.binread, or the file encoded as Base64.', field) end + def single_line_base64?(content) + content.ascii_only? && BASE64_PATTERN.match?(content) + end + + def encode64_layout?(content) + content.ascii_only? && content.include?("\n") && [content.unpack1('m')].pack('m') == content + end + def file_bytes(content) return content if content.is_a?(String) && content.encoding == Encoding::BINARY return unless content.is_a?(Array) && content.all? { |byte| byte.is_a?(Integer) && byte.between?(0, 255) } @@ -412,7 +411,6 @@ def file_bytes(content) content.pack('C*') end - # The message never holds the file's content, so the error is safe to log. def invalid_attachment(message, field) EmailError.new(code: 'invalid_attachment', message: message, field: field) end