From 9b87e57fa14bb6fc58e4d95e97bc38003e8fcae1 Mon Sep 17 00:00:00 2001 From: Rob Zolkos Date: Mon, 12 Oct 2026 15:25:57 -0400 Subject: [PATCH 1/6] Set and remove a contact's photo with hey contact photo `hey contact photo set ` uploads a local image as the contact's avatar (Attachments.Upload, then Contacts.UpdateAvatar with the blob's signed ID); `hey contact photo remove ` takes it off (Contacts.DeleteAvatar). `avatar` is an alias for `photo`. The file is checked before anything is sent, because HEY stores any blob it is handed and only fails later, when it resizes the photo to show it: - PNG or JPEG only, the formats HEY's photo picker accepts, judged by the bytes rather than the extension. A HEIC from a phone would otherwise leave the contact with a broken avatar. - The header must decode with real dimensions, which catches a truncated or mislabelled file. - At most 40 MiB, the most HEY's web form can post through its proxy. The direct upload has no cap of its own. - A regular, non-empty file. The contact is read before the upload, so a mistyped ID costs a read rather than an upload. HEY answers 404 for a contact you cannot edit (hidden, an alias, another HEY user), so a not_found from either write says so in its hint. --- .surface | 3 + docs/cli.md | 4 +- internal/cmd/contact_photo.go | 253 ++++++++++++++++++ internal/cmd/contact_photo_test.go | 406 +++++++++++++++++++++++++++++ internal/cmd/contacts.go | 5 +- skills/hey/SKILL.md | 3 + tests/smoke/contacts_test.go | 57 ++++ 7 files changed, 728 insertions(+), 3 deletions(-) create mode 100644 internal/cmd/contact_photo.go create mode 100644 internal/cmd/contact_photo_test.go diff --git a/.surface b/.surface index 44c309a70..d0359079d 100644 --- a/.surface +++ b/.surface @@ -135,6 +135,9 @@ hey contact note set hey contact note set --note hey contact note set --note-html hey contact note show +hey contact photo +hey contact photo remove +hey contact photo set hey contact show hey contact show-again hey contact threads diff --git a/docs/cli.md b/docs/cli.md index 1bd714132..a4f8cfd2c 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -213,6 +213,8 @@ hey contact note set 12345 "Prefers email" hey contact note show 12345 # read the private note hey contact note show 12345 --jq '.data.note_markdown' # the note as Markdown; set it back only if note_markdown_lossless is true hey contact note delete 12345 +hey contact photo set 12345 ~/Pictures/jane-doe.jpg # upload a PNG or JPEG as the contact's photo +hey contact photo remove 12345 # back to HEY's default avatar hey screener list # who is waiting to be screened hey screener list --count # just the number waiting hey screener approve 91 # let a sender through @@ -286,7 +288,7 @@ Drafts are the review-before-send lane: `hey compose --draft` (and `hey reply -- Search accepts free text plus `--required`, `--any`, `--none`, `--exact`, `--from`, `--to`, `--subject`, `--date`, `--in`, `--label`, and `--attachment`. `--in`, `--date`, `--label` and `--attachment` take one of the values `hey search filters` lists — the attachment kinds are `any`, `images`, `pdfs`, `calendar_invites`, `documents`, `spreadsheets`, `presentations`, `media` and `zip_files`, so it is `--attachment pdfs` rather than `pdf`, and an unrecognized `--in`, `--date` or `--attachment` is refused as a usage error, with the values it accepts, before any search is sent. `--in` also takes a box's kind or display name (`feedbox`, `"Paper Trail"`), the spellings `hey box view` takes, including a name you gave one of those boxes in HEY; a name it does not recognize costs one read of the box list to find out which box it is. Search can narrow only to the Imbox, The Feed, the Paper Trail and Trash: any other box, and a name no box has, is a usage error rather than `not_found`. Use `--page` for one page or `--all` to fetch up to 100 pages; capped searches report the next page for continuation. Search results include `topic_id` for reading the thread and the matching message summaries. Results also include `id` for organization actions, except when you have no box item for the thread. -Contact updates preserve omitted name, email, and alias fields. Supplying `--alias` replaces the complete alias list; `--alias=` clears it. Contact notes accept positional content, `--note`, stdin, or `$EDITOR`. `hey contact note set` replaces the whole note — HEY has no append — so adding a detail means reading the note, changing it and writing all of it back. `hey contact note show --json` (and `hey contact show --json`) answers the note three ways, each an empty string for a contact with no note: `note`, HEY's plain text, which drops formatting (bold is lost, list items become `•`); `note_html`, as HEY serves it; and `note_markdown`, the note as Markdown. `note_markdown_lossless` says whether that Markdown holds everything in the note. When it is `true`, `note set` writes the Markdown back as the same note — HEY's editor markup becomes plain paragraphs on the first write, and from then on a read and a write give the same Markdown and the same HTML. When it is `false`, the note holds an attachment, an image, a table or other markup Markdown has no syntax for, and setting Markdown would drop it: change `note_html` and write it back with `--note-html` instead. HTML passed to `--note-html` has the `
` HEY serves a note in taken off, so writing back `note_html` does not nest the note one level deeper each time. Nothing guards a read and a write against someone else saving in between: the last save wins, as it does in the web app. HEY hides contacts rather than permanently deleting them; hidden contacts leave lists, autocomplete, and search, and can be shown again by ID. `hey contact deliver --to imbox|feed|papertrail|screened-out` changes where a contact's future email arrives. It takes a contact ID — not an email address, clearance ID, box item ID, or box ID — and is write-only because HEY does not expose the current setting as structured data. Imbox removes a custom box preference; choosing a box does not screen a denied contact back in. Screened Out is the blocking choice and applies only to external email contacts. Selecting The Feed removes an existing bundle because HEY cannot keep Feed-designated contacts bundled; Imbox and Paper Trail preserve eligible bundles. Existing mail can move asynchronously, so success means HEY accepted the change, not that every affected thread has already moved. Bundling groups a contact's mail into one row without merging or deleting the underlying threads; unbundling lists those threads separately again. HEY bundles only a contact with no box preference or one sent to the Paper Trail; for any other, bundling answers success and changes nothing. A contact that is hidden, an alias, or another HEY user cannot be edited: updating it or setting or deleting its note answers `not_found`. `hey contact list` holds only non-HEY senders you have screened in — other HEY users, senders still in or denied by The Screener, aliases and hidden contacts are left out. It has no search: HEY ignores a query on that list, so finding a contact by email means reading every page (`--all`) and filtering, which matches primary addresses only. +Contact updates preserve omitted name, email, and alias fields. Supplying `--alias` replaces the complete alias list; `--alias=` clears it. Contact notes accept positional content, `--note`, stdin, or `$EDITOR`. `hey contact note set` replaces the whole note — HEY has no append — so adding a detail means reading the note, changing it and writing all of it back. `hey contact note show --json` (and `hey contact show --json`) answers the note three ways, each an empty string for a contact with no note: `note`, HEY's plain text, which drops formatting (bold is lost, list items become `•`); `note_html`, as HEY serves it; and `note_markdown`, the note as Markdown. `note_markdown_lossless` says whether that Markdown holds everything in the note. When it is `true`, `note set` writes the Markdown back as the same note — HEY's editor markup becomes plain paragraphs on the first write, and from then on a read and a write give the same Markdown and the same HTML. When it is `false`, the note holds an attachment, an image, a table or other markup Markdown has no syntax for, and setting Markdown would drop it: change `note_html` and write it back with `--note-html` instead. HTML passed to `--note-html` has the `
` HEY serves a note in taken off, so writing back `note_html` does not nest the note one level deeper each time. Nothing guards a read and a write against someone else saving in between: the last save wins, as it does in the web app. HEY hides contacts rather than permanently deleting them; hidden contacts leave lists, autocomplete, and search, and can be shown again by ID. `hey contact deliver --to imbox|feed|papertrail|screened-out` changes where a contact's future email arrives. It takes a contact ID — not an email address, clearance ID, box item ID, or box ID — and is write-only because HEY does not expose the current setting as structured data. Imbox removes a custom box preference; choosing a box does not screen a denied contact back in. Screened Out is the blocking choice and applies only to external email contacts. Selecting The Feed removes an existing bundle because HEY cannot keep Feed-designated contacts bundled; Imbox and Paper Trail preserve eligible bundles. Existing mail can move asynchronously, so success means HEY accepted the change, not that every affected thread has already moved. Bundling groups a contact's mail into one row without merging or deleting the underlying threads; unbundling lists those threads separately again. HEY bundles only a contact with no box preference or one sent to the Paper Trail; for any other, bundling answers success and changes nothing. `hey contact photo set ` uploads a photo for a contact, replacing any it had, and answers the contact with its new `avatar_url`; `hey contact photo remove` takes it off again, and removing from a contact with no photo succeeds. The file must be a PNG or JPEG — the formats HEY's own photo picker takes — of at most 40 MiB, the most HEY's web form can send. It is judged by its bytes rather than its name, and its header must decode, because HEY stores any file it is handed and fails only later, when it resizes the photo to show it: a HEIC from a phone, or a file that is not an image, would leave the contact with a broken avatar. HEY crops the photo to a square, and caches avatars, so the old photo can still show in places for up to 30 minutes. Your own contact takes a photo too. A contact that is hidden, an alias, or another HEY user cannot be edited: updating it, setting or deleting its note, or setting or removing its photo answers `not_found`. `hey contact list` holds only non-HEY senders you have screened in — other HEY users, senders still in or denied by The Screener, aliases and hidden contacts are left out. It has no search: HEY ignores a query on that list, so finding a contact by email means reading every page (`--all`) and filtering, which matches primary addresses only. The Screener is where first-time senders wait. `hey screener list` returns clearance IDs — not contact IDs — with the sender and the subject of what they sent, plus `topic_id` for reading the thread before deciding. `--count` asks for the number alone, which is a far cheaper request than the queue, and prints it as a bare number like every other command's `--count`, so `n=$(hey screener list --count)` reads it directly. Approving delivers everything the sender has waiting; denying hides it. Either is reversible with the opposite command, and `hey screener history` shows what was already decided. Both listings page the way `hey box view` does: `--all` follows HEY's cursor to the end of the queue (up to 100 pages), and a single-page or capped read reports `next_page` in its JSON meta, which `--page ` continues from — the cursor is opaque, so a page *number* does not name a position. `hey screener list` also reports `total_count`, the whole queue's size, next to what the read returned. `--box` and `--seen` approve one sender at a time; several IDs go through HEY's bulk endpoint, which takes neither. `--seen` reliably marks the mail read only for a sender with one waiting thread. `--spam` marks what they sent as spam and, for a single ID, trains HEY's filter, which is harder to undo than denying. `hey screener clear` moves everything waiting to Trash (a shared thread loses your access instead) without approving or denying anyone — those senders reappear on their next email. diff --git a/internal/cmd/contact_photo.go b/internal/cmd/contact_photo.go new file mode 100644 index 000000000..b7a4c649f --- /dev/null +++ b/internal/cmd/contact_photo.go @@ -0,0 +1,253 @@ +package cmd + +import ( + "errors" + "fmt" + "image" + _ "image/jpeg" // registers the decoder contactPhoto checks a JPEG with + _ "image/png" // registers the decoder contactPhoto checks a PNG with + "io" + "net/http" + "os" + "path/filepath" + + "github.com/spf13/cobra" + + "github.com/basecamp/hey-cli/internal/apierr" + "github.com/basecamp/hey-cli/internal/output" + "github.com/basecamp/hey-cli/internal/terminal" +) + +// contactPhotoMaxBytes is the largest photo `hey contact photo set` uploads. HEY's own photo +// form posts through a proxy that refuses a body over 40 MiB, so nothing larger reaches a +// contact from the web; the direct upload used here has no cap of its own, so the CLI keeps +// to the web's. HEY shows a photo at 300×300, so a real one is far smaller. +const contactPhotoMaxBytes = 40 << 20 + +// contactPhotoFormats are the formats HEY's photo picker accepts, keyed by the content +// type sniffed from the bytes and naming the decoder that must agree. HEY stores whatever +// blob it is handed and only fails later, when it resizes the photo to show it: a HEIC or +// an AVIF, which HEY cannot decode, or a file that is not an image at all, then breaks the +// contact's avatar everywhere it is drawn. So the bytes are judged, not the extension. +var contactPhotoFormats = map[string]string{ + "image/png": "png", + "image/jpeg": "jpeg", +} + +// contactPhotoNotEditableHint is what a not_found from the photo routes usually means: +// HEY answers 404 for a contact it will not let you edit, not 403. +const contactPhotoNotEditableHint = "HEY changes the photo of a contact you can edit, or your own: not a hidden contact, an alias, or another HEY user." + +type contactPhotoCommand struct { + cmd *cobra.Command +} + +func newContactPhotoCommand() *contactPhotoCommand { + photoCommand := &contactPhotoCommand{} + photoCommand.cmd = &cobra.Command{ + Use: "photo", + Aliases: []string{"avatar"}, + Short: "Set or remove a contact's photo", + Annotations: map[string]string{ + "agent_notes": "Set takes a contact ID and a local PNG or JPEG file of at most 40 MiB; remove takes a contact ID. The photo is the contact's avatar_url. Hidden contacts, aliases and other HEY users answer not_found.", + }, + } + photoCommand.cmd.AddCommand(newContactPhotoSetCommand().cmd) + photoCommand.cmd.AddCommand(newContactPhotoRemoveCommand().cmd) + return photoCommand +} + +type contactPhotoSetCommand struct { + cmd *cobra.Command + contactID int64 + path string +} + +func newContactPhotoSetCommand() *contactPhotoSetCommand { + setCommand := &contactPhotoSetCommand{} + setCommand.cmd = &cobra.Command{ + Use: "set ", + Short: "Upload a photo for a contact", + Long: "Upload a local image as a contact's photo, replacing any photo they already have. " + + "The file must be a PNG or JPEG of at most 40 MiB, judged by its contents rather than its name; HEY crops it to a square. " + + "Only a contact you can edit, or your own, takes a photo: a hidden contact, an alias or another HEY user answers not_found. " + + "HEY caches avatars, so the old photo may still show in places for up to 30 minutes.", + Example: ` hey contact photo set 12345 ~/Pictures/jane-doe.jpg + hey contact photo set 12345 headshot.png --json`, + Args: setCommand.validateArgs, + RunE: setCommand.run, + } + return setCommand +} + +func (c *contactPhotoSetCommand) validateArgs(cmd *cobra.Command, args []string) error { + if err := usageExactArgs(2)(cmd, args); err != nil { + return err + } + contactID, err := parseContactID(args[0]) + if err != nil { + return err + } + c.contactID = contactID + c.path = args[1] + return nil +} + +func (c *contactPhotoSetCommand) run(cmd *cobra.Command, _ []string) error { + photo, err := openContactPhoto(c.path) + if err != nil { + return err + } + defer closePreparedAttachments([]preparedAttachment{photo}) + + if err = requireAuth(); err != nil { + return err + } + + // Read the contact first, so a mistyped ID costs a read rather than an upload. + contact, err := sdk.Contacts().Get(cmd.Context(), c.contactID) + if err != nil { + return apierr.FromSDK(err) + } + if contact == nil { + return apierr.ErrNotFound("contact", fmt.Sprintf("%d", c.contactID)) + } + + upload, err := sdk.Attachments().Upload(cmd.Context(), photo.filename, photo.contentType, photo.file) + if err != nil { + return apierr.FromSDK(err) + } + if upload == nil || upload.SignedId == "" { + return fmt.Errorf("HEY returned an empty upload reference for %q", c.path) + } + + updated, err := sdk.Contacts().UpdateAvatar(cmd.Context(), c.contactID, upload.SignedId) + if err != nil { + return contactPhotoError(err, c.contactID) + } + // HEY answers the contact it saved; should it answer nothing, the photo is still saved. + var data any = map[string]int64{"id": c.contactID} + if updated != nil { + data = updated + } + + return writeMutationLine(cmd, + fmt.Sprintf("Photo saved for contact %d (%s). The old photo may still show in places for up to 30 minutes.", c.contactID, terminal.SanitizeLine(contact.Name)), + "Contact photo saved", + data, + output.WithBreadcrumbs(output.Breadcrumb{Action: "remove", Command: fmt.Sprintf("hey contact photo remove %d", c.contactID), Description: "Remove the photo"}), + ) +} + +type contactPhotoRemoveCommand struct { + cmd *cobra.Command +} + +func newContactPhotoRemoveCommand() *contactPhotoRemoveCommand { + removeCommand := &contactPhotoRemoveCommand{} + removeCommand.cmd = &cobra.Command{ + Use: "remove ", + Short: "Remove a contact's photo", + Long: "Remove the photo uploaded for a contact, so HEY shows its default avatar for them again. " + + "Removing from a contact with no photo succeeds and changes nothing. " + + "A hidden contact, an alias or another HEY user answers not_found.", + Example: ` hey contact photo remove 12345`, + Args: usageExactOneArg(), + RunE: removeCommand.run, + } + return removeCommand +} + +func (c *contactPhotoRemoveCommand) run(cmd *cobra.Command, args []string) error { + contactID, err := parseContactID(args[0]) + if err != nil { + return err + } + if err := requireAuth(); err != nil { + return err + } + if err := sdk.Contacts().DeleteAvatar(cmd.Context(), contactID); err != nil { + return contactPhotoError(err, contactID) + } + return writeMutationLine(cmd, + fmt.Sprintf("Photo removed from contact %d.", contactID), + "Contact photo removed", + map[string]int64{"id": contactID}, + output.WithBreadcrumbs(output.Breadcrumb{Action: "set", Command: fmt.Sprintf("hey contact photo set %d ", contactID), Description: "Upload a photo"}), + ) +} + +// contactPhotoError says why a photo write answered not_found, since that is how HEY +// refuses a contact you cannot edit. +func contactPhotoError(err error, contactID int64) error { + converted := apierr.FromSDK(err) + if apierr.AsError(converted).Code == apierr.CodeNotFound { + return apierr.ErrNotFoundHint("contact", fmt.Sprintf("%d", contactID), contactPhotoNotEditableHint) + } + return converted +} + +// openContactPhoto opens path and checks it is a photo HEY can show, before anything is +// sent: a regular, non-empty file within contactPhotoMaxBytes whose bytes are a PNG or a +// JPEG with a readable header and real dimensions. The file is left open at its start. +func openContactPhoto(path string) (preparedAttachment, error) { + file, err := os.Open(path) // #nosec G304 -- the user explicitly selected this photo + if err != nil { + return preparedAttachment{}, apierr.ErrUsage(fmt.Sprintf("could not open photo %q: %v", path, err)) + } + photo, err := checkContactPhoto(file, path) + if err != nil { + _ = file.Close() + return preparedAttachment{}, err + } + return photo, nil +} + +func checkContactPhoto(file *os.File, path string) (preparedAttachment, error) { + info, err := file.Stat() + if err != nil { + return preparedAttachment{}, apierr.ErrUsage(fmt.Sprintf("could not inspect photo %q: %v", path, err)) + } + if !info.Mode().IsRegular() { + return preparedAttachment{}, apierr.ErrUsage(fmt.Sprintf("photo %q is not a regular file", path)) + } + if info.Size() == 0 { + return preparedAttachment{}, apierr.ErrUsage(fmt.Sprintf("photo %q is empty", path)) + } + if info.Size() > contactPhotoMaxBytes { + return preparedAttachment{}, apierr.ErrUsage(fmt.Sprintf("photo %q exceeds the %d MiB contact photo limit", + path, contactPhotoMaxBytes>>20)) + } + + head := make([]byte, 512) + read, err := io.ReadFull(file, head) + if err != nil && !errors.Is(err, io.ErrUnexpectedEOF) { + return preparedAttachment{}, apierr.ErrUsage(fmt.Sprintf("could not read photo %q: %v", path, err)) + } + contentType := attachmentMediaType(http.DetectContentType(head[:read])) + format, ok := contactPhotoFormats[contentType] + if !ok { + return preparedAttachment{}, apierr.ErrUsageHint( + fmt.Sprintf("photo %q is %s, not a PNG or JPEG", path, contentType), + "Convert it to PNG or JPEG first; HEY cannot show other formats as a contact photo.") + } + + if _, err = file.Seek(0, io.SeekStart); err != nil { + return preparedAttachment{}, fmt.Errorf("rewind photo %q: %w", path, err) + } + config, decoded, err := image.DecodeConfig(file) + if err != nil || decoded != format || config.Width <= 0 || config.Height <= 0 { + return preparedAttachment{}, apierr.ErrUsage(fmt.Sprintf("photo %q is not a readable %s image", path, format)) + } + if _, err = file.Seek(0, io.SeekStart); err != nil { + return preparedAttachment{}, fmt.Errorf("rewind photo %q: %w", path, err) + } + + return preparedAttachment{ + path: path, + filename: filepath.Base(path), + contentType: contentType, + byteSize: info.Size(), + file: file, + }, nil +} diff --git a/internal/cmd/contact_photo_test.go b/internal/cmd/contact_photo_test.go new file mode 100644 index 000000000..7c9b4a84c --- /dev/null +++ b/internal/cmd/contact_photo_test.go @@ -0,0 +1,406 @@ +package cmd + +import ( + "bytes" + "encoding/json" + "errors" + "image" + "image/color" + "image/jpeg" + "image/png" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/basecamp/hey-cli/internal/apierr" +) + +type contactPhotoServer struct { + server *httptest.Server + recorded *recordedContacts + contact string +} + +func newContactPhotoServer(t *testing.T) *contactPhotoServer { + t.Helper() + state := &contactPhotoServer{ + recorded: &recordedContacts{statuses: make(map[string]int)}, + contact: `{"id":7,"account_id":1,"name":"Jane Doe","email_address":"jane@example.com","contactable_type":"Person","avatar_url":"https://app.hey.com/contacts/7/avatar"}`, + } + var server *httptest.Server + server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + body := new(bytes.Buffer) + _, _ = body.ReadFrom(req.Body) + state.recorded.mu.Lock() + state.recorded.requests = append(state.recorded.requests, recordedContactRequest{ + Method: req.Method, Path: req.URL.Path, Query: req.URL.RawQuery, Body: body.Bytes(), + }) + status := state.recorded.statuses[req.Method+" "+req.URL.Path] + state.recorded.mu.Unlock() + + w.Header().Set("Content-Type", "application/json") + if status != 0 { + w.WriteHeader(status) + _, _ = w.Write([]byte(`{"error":"request failed"}`)) + return + } + switch { + case req.Method == http.MethodGet && req.URL.Path == "/contacts/7.json": + _, _ = w.Write([]byte(state.contact)) + case req.Method == http.MethodPost && req.URL.Path == "/rails/active_storage/direct_uploads.json": + _, _ = w.Write([]byte(`{"signed_id":"signed-avatar","attachable_sgid":"sgid-avatar","direct_upload":{"url":"` + + server.URL + `/storage/avatar","headers":{"Content-Type":"image/png"}}}`)) + case req.Method == http.MethodPut && req.URL.Path == "/storage/avatar": + w.WriteHeader(http.StatusNoContent) + case req.Method == http.MethodPut && req.URL.Path == "/contacts/7/uploaded_avatar.json": + _, _ = w.Write([]byte(`{"id":7,"account_id":1,"name":"Jane Doe","email_address":"jane@example.com","contactable_type":"Person","avatar_url":"https://app.hey.com/contacts/7/avatar?v=2"}`)) + case req.Method == http.MethodDelete && req.URL.Path == "/contacts/7/uploaded_avatar.json": + w.WriteHeader(http.StatusNoContent) + default: + http.NotFound(w, req) + } + })) + state.server = server + t.Cleanup(server.Close) + return state +} + +func (s *contactPhotoServer) requestLines() []string { + requests := s.recorded.snapshot() + lines := make([]string, 0, len(requests)) + for _, request := range requests { + lines = append(lines, request.Method+" "+request.Path) + } + return lines +} + +func writePhotoFixture(t *testing.T, name string, content []byte) string { + t.Helper() + path := filepath.Join(t.TempDir(), name) + if err := os.WriteFile(path, content, 0o600); err != nil { + t.Fatal(err) + } + return path +} + +func pngPhoto(t *testing.T) []byte { + t.Helper() + picture := image.NewRGBA(image.Rect(0, 0, 4, 3)) + picture.Set(1, 1, color.RGBA{R: 200, G: 120, B: 40, A: 255}) + var encoded bytes.Buffer + if err := png.Encode(&encoded, picture); err != nil { + t.Fatal(err) + } + return encoded.Bytes() +} + +func jpegPhoto(t *testing.T) []byte { + t.Helper() + var encoded bytes.Buffer + if err := jpeg.Encode(&encoded, image.NewGray(image.Rect(0, 0, 4, 3)), nil); err != nil { + t.Fatal(err) + } + return encoded.Bytes() +} + +func TestContactPhotoSetUploadsThenAttachesTheSignedBlob(t *testing.T) { + state := newContactPhotoServer(t) + photo := writePhotoFixture(t, "jane-doe.png", pngPhoto(t)) + + resp, err := runContacts(t, state.server, "photo", "set", "7", photo) + if err != nil { + t.Fatal(err) + } + + want := []string{ + "GET /contacts/7.json", + "POST /rails/active_storage/direct_uploads.json", + "PUT /storage/avatar", + "PUT /contacts/7/uploaded_avatar.json", + } + if got := state.requestLines(); strings.Join(got, "\n") != strings.Join(want, "\n") { + t.Fatalf("requests =\n%s\nwant\n%s", strings.Join(got, "\n"), strings.Join(want, "\n")) + } + + requests := state.recorded.snapshot() + var reservation struct { + Blob struct { + Filename string `json:"filename"` + ContentType string `json:"content_type"` + ByteSize int64 `json:"byte_size"` + } `json:"blob"` + } + if err := json.Unmarshal(requests[1].Body, &reservation); err != nil { + t.Fatal(err) + } + if reservation.Blob.Filename != "jane-doe.png" || reservation.Blob.ContentType != "image/png" || reservation.Blob.ByteSize != int64(len(pngPhoto(t))) { + t.Errorf("direct upload blob = %+v", reservation.Blob) + } + if !bytes.Equal(requests[2].Body, pngPhoto(t)) { + t.Error("the storage upload did not carry the photo's bytes") + } + if got, want := string(requests[3].Body), `{"contact":{"uploaded_avatar":"signed-avatar"}}`; got != want { + t.Errorf("avatar body = %s, want %s", got, want) + } + + contact := decodeContactData[struct { + ID int64 `json:"id"` + AvatarURL string `json:"avatar_url"` + }](t, resp.Data) + if contact.ID != 7 || contact.AvatarURL != "https://app.hey.com/contacts/7/avatar?v=2" { + t.Errorf("data = %+v, want the contact HEY answered", contact) + } + if resp.Summary != "Contact photo saved" { + t.Errorf("summary = %q", resp.Summary) + } +} + +func TestContactPhotoSetJudgesTheBytesNotTheName(t *testing.T) { + tests := []struct { + name string + filename string + content func(*testing.T) []byte + wantContentType string + }{ + {name: "jpeg", filename: "jane-doe.jpg", content: jpegPhoto, wantContentType: "image/jpeg"}, + {name: "png named jpg", filename: "jane-doe.jpg", content: pngPhoto, wantContentType: "image/png"}, + {name: "jpeg with no extension", filename: "jane-doe", content: jpegPhoto, wantContentType: "image/jpeg"}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + state := newContactPhotoServer(t) + photo := writePhotoFixture(t, tc.filename, tc.content(t)) + if _, err := runContacts(t, state.server, "photo", "set", "7", photo); err != nil { + t.Fatal(err) + } + var reservation struct { + Blob struct { + ContentType string `json:"content_type"` + } `json:"blob"` + } + if err := json.Unmarshal(state.recorded.snapshot()[1].Body, &reservation); err != nil { + t.Fatal(err) + } + if reservation.Blob.ContentType != tc.wantContentType { + t.Errorf("declared content type = %q, want %q", reservation.Blob.ContentType, tc.wantContentType) + } + }) + } +} + +func TestContactPhotoSetRefusesBadFilesBeforeAnyRequest(t *testing.T) { + pngSignature := []byte("\x89PNG\r\n\x1a\n") + tests := []struct { + name string + path func(t *testing.T) string + message string + }{ + {name: "missing file", path: func(t *testing.T) string { return filepath.Join(t.TempDir(), "jane-doe.png") }, message: "could not open photo"}, + {name: "directory", path: func(t *testing.T) string { return t.TempDir() }, message: "not a regular file"}, + {name: "empty", path: func(t *testing.T) string { return writePhotoFixture(t, "jane-doe.png", nil) }, message: "is empty"}, + {name: "heic from a phone", path: func(t *testing.T) string { + return writePhotoFixture(t, "IMG_0042.HEIC", append([]byte("\x00\x00\x00\x18ftypheic\x00\x00\x00\x00mif1heic"), make([]byte, 64)...)) + }, message: "not a PNG or JPEG"}, + {name: "gif", path: func(t *testing.T) string { + return writePhotoFixture(t, "jane-doe.gif", []byte("GIF89a\x01\x00\x01\x00\x00\x00\x00;")) + }, message: "is image/gif, not a PNG or JPEG"}, + {name: "pdf named png", path: func(t *testing.T) string { + return writePhotoFixture(t, "jane-doe.png", []byte("%PDF-1.7\n1 0 obj\n<<>>\nendobj\n")) + }, message: "is application/pdf, not a PNG or JPEG"}, + {name: "text named jpg", path: func(t *testing.T) string { + return writePhotoFixture(t, "jane-doe.jpg", []byte("Jane Doe, Head of Partnerships")) + }, message: "not a PNG or JPEG"}, + {name: "png signature with a broken header", path: func(t *testing.T) string { + return writePhotoFixture(t, "jane-doe.png", append(pngSignature, []byte("this is not an IHDR chunk")...)) + }, message: "is not a readable png image"}, + {name: "png cut off after its signature", path: func(t *testing.T) string { + return writePhotoFixture(t, "jane-doe.png", pngSignature) + }, message: "is not a readable png image"}, + {name: "jpeg cut off before its frame header", path: func(t *testing.T) string { + return writePhotoFixture(t, "jane-doe.jpg", jpegPhoto(t)[:24]) + }, message: "is not a readable jpeg image"}, + {name: "over the size limit", path: func(t *testing.T) string { + path := writePhotoFixture(t, "jane-doe.png", pngPhoto(t)) + if err := os.Truncate(path, contactPhotoMaxBytes+1); err != nil { + t.Fatal(err) + } + return path + }, message: "exceeds the 40 MiB contact photo limit"}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + state := newContactPhotoServer(t) + _, err := runContacts(t, state.server, "photo", "set", "7", tc.path(t)) + var cliErr *apierr.Error + if !errors.As(err, &cliErr) || cliErr.Code != apierr.CodeUsage { + t.Fatalf("error = %v, want usage error", err) + } + if !strings.Contains(cliErr.Message, tc.message) { + t.Errorf("message = %q, want it to contain %q", cliErr.Message, tc.message) + } + if requests := state.requestLines(); len(requests) != 0 { + t.Fatalf("a refused photo sent requests: %v", requests) + } + }) + } +} + +func TestContactPhotoSetAcceptsAPhotoAtTheSizeLimit(t *testing.T) { + photo := writePhotoFixture(t, "jane-doe.png", pngPhoto(t)) + // A PNG decoder stops at IEND, so padding the file out keeps it a valid photo. + if err := os.Truncate(photo, contactPhotoMaxBytes); err != nil { + t.Fatal(err) + } + prepared, err := openContactPhoto(photo) + if err != nil { + t.Fatalf("a photo of exactly %d bytes was refused: %v", contactPhotoMaxBytes, err) + } + closePreparedAttachments([]preparedAttachment{prepared}) +} + +func TestContactPhotoSetRejectsBadArgumentsBeforeAnyRequest(t *testing.T) { + photo := writePhotoFixture(t, "jane-doe.png", pngPhoto(t)) + tests := []struct { + name string + args []string + }{ + {name: "no arguments", args: []string{"photo", "set"}}, + {name: "no file", args: []string{"photo", "set", "7"}}, + {name: "extra argument", args: []string{"photo", "set", "7", photo, photo}}, + {name: "malformed id", args: []string{"photo", "set", "jane", photo}}, + {name: "zero id", args: []string{"photo", "set", "0", photo}}, + {name: "remove without id", args: []string{"photo", "remove"}}, + {name: "remove malformed id", args: []string{"photo", "remove", "jane"}}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + state := newContactPhotoServer(t) + // A wrong count is answered by the shared argument check, the rest as usage errors. + if _, err := runContacts(t, state.server, tc.args...); err == nil { + t.Fatal("expected an error") + } + if requests := state.requestLines(); len(requests) != 0 { + t.Fatalf("bad arguments sent requests: %v", requests) + } + }) + } +} + +func TestContactPhotoSetDoesNotUploadForAMissingContact(t *testing.T) { + for _, status := range []int{http.StatusNotFound, http.StatusNoContent} { + state := newContactPhotoServer(t) + state.recorded.statuses["GET /contacts/7.json"] = status + photo := writePhotoFixture(t, "jane-doe.png", pngPhoto(t)) + + resp, err := runContacts(t, state.server, "photo", "set", "7", photo) + var cliErr *apierr.Error + if !errors.As(err, &cliErr) || cliErr.Code != apierr.CodeNotFound { + t.Fatalf("status %d: error = %v, want not_found", status, err) + } + if resp.OK { + t.Errorf("status %d: response = %+v, want failure", status, resp) + } + if got := state.requestLines(); len(got) != 1 { + t.Errorf("status %d: requests = %v, want only the contact read", status, got) + } + } +} + +func TestContactPhotoSaysWhyHEYRefusedTheContact(t *testing.T) { + tests := []struct { + name string + args func(t *testing.T) []string + statusKey string + }{ + {name: "set", statusKey: "PUT /contacts/7/uploaded_avatar.json", args: func(t *testing.T) []string { + return []string{"photo", "set", "7", writePhotoFixture(t, "jane-doe.png", pngPhoto(t))} + }}, + {name: "remove", statusKey: "DELETE /contacts/7/uploaded_avatar.json", args: func(*testing.T) []string { + return []string{"photo", "remove", "7"} + }}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + state := newContactPhotoServer(t) + state.recorded.statuses[tc.statusKey] = http.StatusNotFound + _, err := runContacts(t, state.server, tc.args(t)...) + var cliErr *apierr.Error + if !errors.As(err, &cliErr) || cliErr.Code != apierr.CodeNotFound { + t.Fatalf("error = %v, want not_found", err) + } + if cliErr.Hint != contactPhotoNotEditableHint { + t.Errorf("hint = %q, want %q", cliErr.Hint, contactPhotoNotEditableHint) + } + }) + } +} + +func TestContactPhotoSetStopsWhenTheUploadFails(t *testing.T) { + for _, statusKey := range []string{"POST /rails/active_storage/direct_uploads.json", "PUT /storage/avatar"} { + state := newContactPhotoServer(t) + state.recorded.statuses[statusKey] = http.StatusUnprocessableEntity + photo := writePhotoFixture(t, "jane-doe.png", pngPhoto(t)) + + if _, err := runContacts(t, state.server, "photo", "set", "7", photo); err == nil { + t.Fatalf("%s failing: expected an error", statusKey) + } + for _, line := range state.requestLines() { + if line == "PUT /contacts/7/uploaded_avatar.json" { + t.Errorf("%s failing: the avatar was still set", statusKey) + } + } + } +} + +func TestContactPhotoRemove(t *testing.T) { + state := newContactPhotoServer(t) + resp, err := runContacts(t, state.server, "photo", "remove", "7") + if err != nil { + t.Fatal(err) + } + if got := state.requestLines(); len(got) != 1 || got[0] != "DELETE /contacts/7/uploaded_avatar.json" { + t.Errorf("requests = %v", got) + } + data := decodeContactData[map[string]int64](t, resp.Data) + if data["id"] != 7 || resp.Summary != "Contact photo removed" { + t.Errorf("response = %+v", resp) + } +} + +func TestContactPhotoStyledOutput(t *testing.T) { + state := newContactPhotoServer(t) + photo := writePhotoFixture(t, "jane-doe.png", pngPhoto(t)) + + got, err := runStyledCommand(t, state.server.Config.Handler, "contact", "photo", "set", "7", photo) + if err != nil { + t.Fatal(err) + } + if want := "Photo saved for contact 7 (Jane Doe). The old photo may still show in places for up to 30 minutes.\n"; got != want { + t.Errorf("set output = %q, want %q", got, want) + } + + got, err = runStyledCommand(t, state.server.Config.Handler, "contact", "avatar", "remove", "7") + if err != nil { + t.Fatal(err) + } + if want := "Photo removed from contact 7.\n"; got != want { + t.Errorf("remove output = %q, want %q", got, want) + } +} + +func TestContactPhotoStyledOutputSanitizesTheName(t *testing.T) { + state := newContactPhotoServer(t) + state.contact = `{"id":7,"name":"Jane \u001b[31mDoe","email_address":"jane@example.com"}` + photo := writePhotoFixture(t, "jane-doe.png", pngPhoto(t)) + + got, err := runStyledCommand(t, state.server.Config.Handler, "contact", "photo", "set", "7", photo) + if err != nil { + t.Fatal(err) + } + if strings.ContainsRune(got, '\x1b') { + t.Errorf("output carried an escape sequence: %q", got) + } +} diff --git a/internal/cmd/contacts.go b/internal/cmd/contacts.go index b5885eea2..acc1b9b2e 100644 --- a/internal/cmd/contacts.go +++ b/internal/cmd/contacts.go @@ -22,9 +22,9 @@ func newContactsCommand() *contactsCommand { contactsCommand.cmd = &cobra.Command{ Use: "contact", Short: "Manage contacts", - Long: "List, view, add, edit, hide, route, bundle, and annotate HEY contacts.", + Long: "List, view, add, edit, hide, route, bundle, and annotate HEY contacts, and set their photos.", Annotations: map[string]string{ - "agent_notes": "Contact IDs come from `hey contact list`. Delivery is managed with `hey contact deliver`; hiding and bundling are reversible. Private notes are managed under `hey contact note`.", + "agent_notes": "Contact IDs come from `hey contact list`. Delivery is managed with `hey contact deliver`; hiding and bundling are reversible. Private notes are managed under `hey contact note`, photos under `hey contact photo`.", }, } @@ -39,6 +39,7 @@ func newContactsCommand() *contactsCommand { contactsCommand.cmd.AddCommand(newContactsBundleCommand().cmd) contactsCommand.cmd.AddCommand(newContactsUnbundleCommand().cmd) contactsCommand.cmd.AddCommand(newContactNoteCommand().cmd) + contactsCommand.cmd.AddCommand(newContactPhotoCommand().cmd) return contactsCommand } diff --git a/skills/hey/SKILL.md b/skills/hey/SKILL.md index a42a8b6b9..30407d984 100644 --- a/skills/hey/SKILL.md +++ b/skills/hey/SKILL.md @@ -91,6 +91,7 @@ triggers: - bundle contact mail - unbundle contact mail - contact note + - contact photo - find a contact by email - check calendar - add todo @@ -211,6 +212,8 @@ postings, not the box, label or contact around them. | Read private contact note | `hey contact note show --json` (`note_markdown` is the form `note set` takes when `note_markdown_lossless` is true) | | Replace private contact note | `hey contact note set "Prefers email"` (replaces the whole note) | | Delete private contact note | `hey contact note delete ` | +| Set a contact's photo | `hey contact photo set ` (PNG or JPEG, at most 40 MiB) | +| Remove a contact's photo | `hey contact photo remove ` | | Read email thread | `hey thread read --json` | | Rename a thread | `hey thread update --name "Kitchen renovation quotes"` | | Get a sharing link | `hey share ` | diff --git a/tests/smoke/contacts_test.go b/tests/smoke/contacts_test.go index 8628ccc12..08c269d54 100644 --- a/tests/smoke/contacts_test.go +++ b/tests/smoke/contacts_test.go @@ -1,8 +1,14 @@ package smoke_test import ( + "bytes" "encoding/json" "fmt" + "image" + "image/color" + "image/png" + "os" + "path/filepath" "testing" "time" ) @@ -106,6 +112,53 @@ func TestContactLifecycleAndPrivateNote(t *testing.T) { } } +func TestContactPhoto(t *testing.T) { + unique := fmt.Sprintf("%d", time.Now().UnixNano()) + created := dataAs[struct { + ID int `json:"id"` + AvatarURL string `json:"avatar_url"` + }](t, contactWriteJSON(t, "contact", "add", "--name", "Priya Shah", "--email", "priya.shah+"+unique+"@example.com")) + id := intStr(created.ID) + t.Cleanup(func() { _, _, _ = hey(t, "contact", "hide", id) }) + + portrait := image.NewRGBA(image.Rect(0, 0, 64, 64)) + for x := 0; x < 64; x++ { + for y := 0; y < 64; y++ { + portrait.Set(x, y, color.RGBA{R: uint8(x * 4), G: 96, B: uint8(y * 4), A: 255}) + } + } + var encoded bytes.Buffer + if err := png.Encode(&encoded, portrait); err != nil { + t.Fatal(err) + } + photo := filepath.Join(t.TempDir(), "priya-shah.png") + if err := os.WriteFile(photo, encoded.Bytes(), 0o600); err != nil { + t.Fatal(err) + } + + withPhoto := dataAs[struct { + ID int `json:"id"` + AvatarURL string `json:"avatar_url"` + }](t, contactWriteJSON(t, "contact", "photo", "set", id, photo)) + if withPhoto.ID != created.ID { + t.Fatalf("photo set answered contact %d, want %d", withPhoto.ID, created.ID) + } + // The avatar URL carries a version of whatever the avatar is drawn from. + if withPhoto.AvatarURL == "" || withPhoto.AvatarURL == created.AvatarURL { + t.Errorf("avatar_url did not change after setting a photo: %q", withPhoto.AvatarURL) + } + + contactWriteJSON(t, "contact", "photo", "remove", id) + withoutPhoto := dataAs[struct { + AvatarURL string `json:"avatar_url"` + }](t, heyJSON(t, "contact", "show", id)) + if withoutPhoto.AvatarURL != created.AvatarURL { + t.Errorf("avatar_url after removing the photo = %q, want the default %q", withoutPhoto.AvatarURL, created.AvatarURL) + } + // Removing again, from a contact with no photo, still succeeds. + contactWriteJSON(t, "contact", "photo", "remove", id) +} + func TestContactThreads(t *testing.T) { // An Imbox sender is a contact with at least one thread on record, so the // listing has something real to answer with. @@ -166,4 +219,8 @@ func TestContactCommandsValidateInput(t *testing.T) { heyFail(t, "contact", "deliver", "not-an-id", "--to", "imbox") heyFail(t, "contact", "bundle", "not-an-id") heyFail(t, "contact", "unbundle", "0") + heyFail(t, "contact", "photo", "set", "12345") + heyFail(t, "contact", "photo", "set", "not-an-id", "priya-shah.png") + heyFail(t, "contact", "photo", "set", "12345", filepath.Join(t.TempDir(), "missing.png")) + heyFail(t, "contact", "photo", "remove", "not-an-id") } From 2fecc02222356219c04380f927d99ee99ca45248 Mon Sep 17 00:00:00 2001 From: Rob Zolkos Date: Mon, 12 Oct 2026 15:34:07 -0400 Subject: [PATCH 2/6] Refuse photos too large for HEY to resize, and say what works best HEY resizes a contact photo with ImageMagick under hotcell/policy.xml, which refuses an image wider or taller than 16KP and spills one over 128MP to a disk cache that its disk ceiling refuses (haystack's imagemagick_policy_test proves 15000x15000). HEY would store such a photo and then fail to draw it, as with a HEIC, so the CLI refuses it from the dimensions it already decodes. A 200MP phone photo is past both limits. The help also repeats HEY's own advice from the profile photo page: a square image without transparency works best. --- docs/cli.md | 2 +- internal/cmd/contact_photo.go | 23 +++++++++-- internal/cmd/contact_photo_test.go | 63 ++++++++++++++++++++++++++++++ skills/hey/SKILL.md | 2 +- 4 files changed, 85 insertions(+), 5 deletions(-) diff --git a/docs/cli.md b/docs/cli.md index a4f8cfd2c..70854ae87 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -288,7 +288,7 @@ Drafts are the review-before-send lane: `hey compose --draft` (and `hey reply -- Search accepts free text plus `--required`, `--any`, `--none`, `--exact`, `--from`, `--to`, `--subject`, `--date`, `--in`, `--label`, and `--attachment`. `--in`, `--date`, `--label` and `--attachment` take one of the values `hey search filters` lists — the attachment kinds are `any`, `images`, `pdfs`, `calendar_invites`, `documents`, `spreadsheets`, `presentations`, `media` and `zip_files`, so it is `--attachment pdfs` rather than `pdf`, and an unrecognized `--in`, `--date` or `--attachment` is refused as a usage error, with the values it accepts, before any search is sent. `--in` also takes a box's kind or display name (`feedbox`, `"Paper Trail"`), the spellings `hey box view` takes, including a name you gave one of those boxes in HEY; a name it does not recognize costs one read of the box list to find out which box it is. Search can narrow only to the Imbox, The Feed, the Paper Trail and Trash: any other box, and a name no box has, is a usage error rather than `not_found`. Use `--page` for one page or `--all` to fetch up to 100 pages; capped searches report the next page for continuation. Search results include `topic_id` for reading the thread and the matching message summaries. Results also include `id` for organization actions, except when you have no box item for the thread. -Contact updates preserve omitted name, email, and alias fields. Supplying `--alias` replaces the complete alias list; `--alias=` clears it. Contact notes accept positional content, `--note`, stdin, or `$EDITOR`. `hey contact note set` replaces the whole note — HEY has no append — so adding a detail means reading the note, changing it and writing all of it back. `hey contact note show --json` (and `hey contact show --json`) answers the note three ways, each an empty string for a contact with no note: `note`, HEY's plain text, which drops formatting (bold is lost, list items become `•`); `note_html`, as HEY serves it; and `note_markdown`, the note as Markdown. `note_markdown_lossless` says whether that Markdown holds everything in the note. When it is `true`, `note set` writes the Markdown back as the same note — HEY's editor markup becomes plain paragraphs on the first write, and from then on a read and a write give the same Markdown and the same HTML. When it is `false`, the note holds an attachment, an image, a table or other markup Markdown has no syntax for, and setting Markdown would drop it: change `note_html` and write it back with `--note-html` instead. HTML passed to `--note-html` has the `
` HEY serves a note in taken off, so writing back `note_html` does not nest the note one level deeper each time. Nothing guards a read and a write against someone else saving in between: the last save wins, as it does in the web app. HEY hides contacts rather than permanently deleting them; hidden contacts leave lists, autocomplete, and search, and can be shown again by ID. `hey contact deliver --to imbox|feed|papertrail|screened-out` changes where a contact's future email arrives. It takes a contact ID — not an email address, clearance ID, box item ID, or box ID — and is write-only because HEY does not expose the current setting as structured data. Imbox removes a custom box preference; choosing a box does not screen a denied contact back in. Screened Out is the blocking choice and applies only to external email contacts. Selecting The Feed removes an existing bundle because HEY cannot keep Feed-designated contacts bundled; Imbox and Paper Trail preserve eligible bundles. Existing mail can move asynchronously, so success means HEY accepted the change, not that every affected thread has already moved. Bundling groups a contact's mail into one row without merging or deleting the underlying threads; unbundling lists those threads separately again. HEY bundles only a contact with no box preference or one sent to the Paper Trail; for any other, bundling answers success and changes nothing. `hey contact photo set ` uploads a photo for a contact, replacing any it had, and answers the contact with its new `avatar_url`; `hey contact photo remove` takes it off again, and removing from a contact with no photo succeeds. The file must be a PNG or JPEG — the formats HEY's own photo picker takes — of at most 40 MiB, the most HEY's web form can send. It is judged by its bytes rather than its name, and its header must decode, because HEY stores any file it is handed and fails only later, when it resizes the photo to show it: a HEIC from a phone, or a file that is not an image, would leave the contact with a broken avatar. HEY crops the photo to a square, and caches avatars, so the old photo can still show in places for up to 30 minutes. Your own contact takes a photo too. A contact that is hidden, an alias, or another HEY user cannot be edited: updating it, setting or deleting its note, or setting or removing its photo answers `not_found`. `hey contact list` holds only non-HEY senders you have screened in — other HEY users, senders still in or denied by The Screener, aliases and hidden contacts are left out. It has no search: HEY ignores a query on that list, so finding a contact by email means reading every page (`--all`) and filtering, which matches primary addresses only. +Contact updates preserve omitted name, email, and alias fields. Supplying `--alias` replaces the complete alias list; `--alias=` clears it. Contact notes accept positional content, `--note`, stdin, or `$EDITOR`. `hey contact note set` replaces the whole note — HEY has no append — so adding a detail means reading the note, changing it and writing all of it back. `hey contact note show --json` (and `hey contact show --json`) answers the note three ways, each an empty string for a contact with no note: `note`, HEY's plain text, which drops formatting (bold is lost, list items become `•`); `note_html`, as HEY serves it; and `note_markdown`, the note as Markdown. `note_markdown_lossless` says whether that Markdown holds everything in the note. When it is `true`, `note set` writes the Markdown back as the same note — HEY's editor markup becomes plain paragraphs on the first write, and from then on a read and a write give the same Markdown and the same HTML. When it is `false`, the note holds an attachment, an image, a table or other markup Markdown has no syntax for, and setting Markdown would drop it: change `note_html` and write it back with `--note-html` instead. HTML passed to `--note-html` has the `
` HEY serves a note in taken off, so writing back `note_html` does not nest the note one level deeper each time. Nothing guards a read and a write against someone else saving in between: the last save wins, as it does in the web app. HEY hides contacts rather than permanently deleting them; hidden contacts leave lists, autocomplete, and search, and can be shown again by ID. `hey contact deliver --to imbox|feed|papertrail|screened-out` changes where a contact's future email arrives. It takes a contact ID — not an email address, clearance ID, box item ID, or box ID — and is write-only because HEY does not expose the current setting as structured data. Imbox removes a custom box preference; choosing a box does not screen a denied contact back in. Screened Out is the blocking choice and applies only to external email contacts. Selecting The Feed removes an existing bundle because HEY cannot keep Feed-designated contacts bundled; Imbox and Paper Trail preserve eligible bundles. Existing mail can move asynchronously, so success means HEY accepted the change, not that every affected thread has already moved. Bundling groups a contact's mail into one row without merging or deleting the underlying threads; unbundling lists those threads separately again. HEY bundles only a contact with no box preference or one sent to the Paper Trail; for any other, bundling answers success and changes nothing. `hey contact photo set ` uploads a photo for a contact, replacing any it had, and answers the contact with its new `avatar_url`; `hey contact photo remove` takes it off again, and removing from a contact with no photo succeeds. The file must be a PNG or JPEG — the formats HEY's own photo picker takes — of at most 40 MiB, the most HEY's web form can send, and at most 16000 pixels a side and 128 megapixels, the most HEY's image processing will resize. It is judged by its bytes rather than its name, and its header must decode, because HEY stores any file it is handed and fails only later, when it resizes the photo to show it: a HEIC from a phone, a 200-megapixel photo, or a file that is not an image would leave the contact with a broken avatar. HEY crops the photo to a square, so a square image without transparency works best, and it caches avatars, so the old photo can still show in places for up to 30 minutes. Your own contact takes a photo too. A contact that is hidden, an alias, or another HEY user cannot be edited: updating it, setting or deleting its note, or setting or removing its photo answers `not_found`. `hey contact list` holds only non-HEY senders you have screened in — other HEY users, senders still in or denied by The Screener, aliases and hidden contacts are left out. It has no search: HEY ignores a query on that list, so finding a contact by email means reading every page (`--all`) and filtering, which matches primary addresses only. The Screener is where first-time senders wait. `hey screener list` returns clearance IDs — not contact IDs — with the sender and the subject of what they sent, plus `topic_id` for reading the thread before deciding. `--count` asks for the number alone, which is a far cheaper request than the queue, and prints it as a bare number like every other command's `--count`, so `n=$(hey screener list --count)` reads it directly. Approving delivers everything the sender has waiting; denying hides it. Either is reversible with the opposite command, and `hey screener history` shows what was already decided. Both listings page the way `hey box view` does: `--all` follows HEY's cursor to the end of the queue (up to 100 pages), and a single-page or capped read reports `next_page` in its JSON meta, which `--page ` continues from — the cursor is opaque, so a page *number* does not name a position. `hey screener list` also reports `total_count`, the whole queue's size, next to what the read returned. `--box` and `--seen` approve one sender at a time; several IDs go through HEY's bulk endpoint, which takes neither. `--seen` reliably marks the mail read only for a sender with one waiting thread. `--spam` marks what they sent as spam and, for a single ID, trains HEY's filter, which is harder to undo than denying. `hey screener clear` moves everything waiting to Trash (a shared thread loses your access instead) without approving or denying anyone — those senders reappear on their next email. diff --git a/internal/cmd/contact_photo.go b/internal/cmd/contact_photo.go index b7a4c649f..d4fe2b423 100644 --- a/internal/cmd/contact_photo.go +++ b/internal/cmd/contact_photo.go @@ -34,6 +34,16 @@ var contactPhotoFormats = map[string]string{ "image/jpeg": "jpeg", } +// HEY resizes a photo with ImageMagick under hotcell/policy.xml, which refuses an image +// wider or taller than 16KP outright and moves one over 128MP to a disk cache that its +// disk ceiling then refuses (haystack's imagemagick_policy_test proves 15000×15000). +// HEY would store such a photo and then fail to draw it, so the CLI refuses it first. +// A 200MP phone photo is past both. +const ( + contactPhotoMaxSide = 16_000 + contactPhotoMaxPixels = 128_000_000 +) + // contactPhotoNotEditableHint is what a not_found from the photo routes usually means: // HEY answers 404 for a contact it will not let you edit, not 403. const contactPhotoNotEditableHint = "HEY changes the photo of a contact you can edit, or your own: not a hidden contact, an alias, or another HEY user." @@ -49,7 +59,7 @@ func newContactPhotoCommand() *contactPhotoCommand { Aliases: []string{"avatar"}, Short: "Set or remove a contact's photo", Annotations: map[string]string{ - "agent_notes": "Set takes a contact ID and a local PNG or JPEG file of at most 40 MiB; remove takes a contact ID. The photo is the contact's avatar_url. Hidden contacts, aliases and other HEY users answer not_found.", + "agent_notes": "Set takes a contact ID and a local PNG or JPEG file of at most 40 MiB, 16000 pixels a side and 128 megapixels; remove takes a contact ID. The photo is the contact's avatar_url. Hidden contacts, aliases and other HEY users answer not_found.", }, } photoCommand.cmd.AddCommand(newContactPhotoSetCommand().cmd) @@ -69,7 +79,8 @@ func newContactPhotoSetCommand() *contactPhotoSetCommand { Use: "set ", Short: "Upload a photo for a contact", Long: "Upload a local image as a contact's photo, replacing any photo they already have. " + - "The file must be a PNG or JPEG of at most 40 MiB, judged by its contents rather than its name; HEY crops it to a square. " + + "The file must be a PNG or JPEG of at most 40 MiB, 16000 pixels a side and 128 megapixels, judged by its contents rather than its name. " + + "HEY crops it to a square, so a square image without transparency works best. " + "Only a contact you can edit, or your own, takes a photo: a hidden contact, an alias or another HEY user answers not_found. " + "HEY caches avatars, so the old photo may still show in places for up to 30 minutes.", Example: ` hey contact photo set 12345 ~/Pictures/jane-doe.jpg @@ -189,7 +200,7 @@ func contactPhotoError(err error, contactID int64) error { // openContactPhoto opens path and checks it is a photo HEY can show, before anything is // sent: a regular, non-empty file within contactPhotoMaxBytes whose bytes are a PNG or a -// JPEG with a readable header and real dimensions. The file is left open at its start. +// JPEG with a readable header and dimensions HEY can resize. The file is left open at its start. func openContactPhoto(path string) (preparedAttachment, error) { file, err := os.Open(path) // #nosec G304 -- the user explicitly selected this photo if err != nil { @@ -239,6 +250,12 @@ func checkContactPhoto(file *os.File, path string) (preparedAttachment, error) { if err != nil || decoded != format || config.Width <= 0 || config.Height <= 0 { return preparedAttachment{}, apierr.ErrUsage(fmt.Sprintf("photo %q is not a readable %s image", path, format)) } + if config.Width > contactPhotoMaxSide || config.Height > contactPhotoMaxSide || + int64(config.Width)*int64(config.Height) > contactPhotoMaxPixels { + return preparedAttachment{}, apierr.ErrUsageHint( + fmt.Sprintf("photo %q is %d×%d pixels, too large for HEY to resize", path, config.Width, config.Height), + "Scale it down to at most 16000 pixels a side and 128 megapixels in all; HEY shows a contact photo at 300×300.") + } if _, err = file.Seek(0, io.SeekStart); err != nil { return preparedAttachment{}, fmt.Errorf("rewind photo %q: %w", path, err) } diff --git a/internal/cmd/contact_photo_test.go b/internal/cmd/contact_photo_test.go index 7c9b4a84c..7317c96e0 100644 --- a/internal/cmd/contact_photo_test.go +++ b/internal/cmd/contact_photo_test.go @@ -2,8 +2,10 @@ package cmd import ( "bytes" + "encoding/binary" "encoding/json" "errors" + "hash/crc32" "image" "image/color" "image/jpeg" @@ -261,6 +263,67 @@ func TestContactPhotoSetAcceptsAPhotoAtTheSizeLimit(t *testing.T) { closePreparedAttachments([]preparedAttachment{prepared}) } +// pngHeader is the start of a PNG claiming the given dimensions: the signature and a +// well-formed IHDR, which is all a decoder reads to answer them. It lets a test name a +// photo of any size without allocating its pixels. +func pngHeader(width, height uint32) []byte { + ihdr := make([]byte, 13) + binary.BigEndian.PutUint32(ihdr[0:], width) + binary.BigEndian.PutUint32(ihdr[4:], height) + ihdr[8] = 8 // bit depth + ihdr[9] = 6 // RGBA + chunk := append([]byte("IHDR"), ihdr...) + header := []byte("\x89PNG\r\n\x1a\n") + header = binary.BigEndian.AppendUint32(header, uint32(len(ihdr))) + header = append(header, chunk...) + return binary.BigEndian.AppendUint32(header, crc32.ChecksumIEEE(chunk)) +} + +func TestContactPhotoSetRefusesPhotosTooLargeForHEYToResize(t *testing.T) { + tests := []struct { + name string + width, height uint32 + }{ + {name: "too wide", width: contactPhotoMaxSide + 1, height: 1}, + {name: "too tall", width: 1, height: contactPhotoMaxSide + 1}, + {name: "too many pixels inside the side limit", width: 15_000, height: 15_000}, + {name: "a 200 megapixel phone photo", width: 16_320, height: 12_240}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + state := newContactPhotoServer(t) + photo := writePhotoFixture(t, "jane-doe.png", pngHeader(tc.width, tc.height)) + _, err := runContacts(t, state.server, "photo", "set", "7", photo) + var cliErr *apierr.Error + if !errors.As(err, &cliErr) || cliErr.Code != apierr.CodeUsage { + t.Fatalf("error = %v, want usage error", err) + } + if !strings.Contains(cliErr.Message, "too large for HEY to resize") { + t.Errorf("message = %q", cliErr.Message) + } + if requests := state.requestLines(); len(requests) != 0 { + t.Fatalf("a refused photo sent requests: %v", requests) + } + }) + } +} + +func TestContactPhotoSetAcceptsPhotosAtTheResizeLimits(t *testing.T) { + for _, size := range [][2]uint32{ + {contactPhotoMaxSide, 1}, + {1, contactPhotoMaxSide}, + {contactPhotoMaxSide, contactPhotoMaxPixels / contactPhotoMaxSide}, + } { + photo := writePhotoFixture(t, "jane-doe.png", pngHeader(size[0], size[1])) + prepared, err := openContactPhoto(photo) + if err != nil { + t.Errorf("%d×%d was refused: %v", size[0], size[1], err) + continue + } + closePreparedAttachments([]preparedAttachment{prepared}) + } +} + func TestContactPhotoSetRejectsBadArgumentsBeforeAnyRequest(t *testing.T) { photo := writePhotoFixture(t, "jane-doe.png", pngPhoto(t)) tests := []struct { diff --git a/skills/hey/SKILL.md b/skills/hey/SKILL.md index 30407d984..f57e2f78a 100644 --- a/skills/hey/SKILL.md +++ b/skills/hey/SKILL.md @@ -212,7 +212,7 @@ postings, not the box, label or contact around them. | Read private contact note | `hey contact note show --json` (`note_markdown` is the form `note set` takes when `note_markdown_lossless` is true) | | Replace private contact note | `hey contact note set "Prefers email"` (replaces the whole note) | | Delete private contact note | `hey contact note delete ` | -| Set a contact's photo | `hey contact photo set ` (PNG or JPEG, at most 40 MiB) | +| Set a contact's photo | `hey contact photo set ` (PNG or JPEG, at most 40 MiB, 16000 px a side and 128 megapixels) | | Remove a contact's photo | `hey contact photo remove ` | | Read email thread | `hey thread read --json` | | Rename a thread | `hey thread update --name "Kitchen renovation quotes"` | From 1de5b0a46fc0e582792fcfcb91587daee691f31d Mon Sep 17 00:00:00 2001 From: Rob Zolkos Date: Mon, 12 Oct 2026 16:32:15 -0400 Subject: [PATCH 3/6] Size a contact photo against the memory HEY resizes it in The 128MP cap came from policy.xml, but the cell that resizes a photo runs under tighter limits of its own (hotcell/Dockerfile: MAGICK_MEMORY_LIMIT 896MiB, and a 256MB file_size that kills a worker whose cache spills to disk). Running HEY's variant command under that policy and those limits in the hotcell image (ImageMagick 6.9.12 Q16) puts the line at 896MiB of pixel cache: - 8 bytes a pixel, so an upright photo tops out near 117MP (10800x10800 resizes, 11000x11000 does not, grayscale too); - 10 bytes a pixel for a CMYK JPEG, near 94MP (9600x9600 / 9800x9600); - twice that for a JPEG whose EXIF orientation is not upright, because auto-orienting it holds a second copy: near 58.7MP (7600x7600 / 7700x7700, for a flip, a 180 and a 90 alike) and 47MP for CMYK. Past it the worker is killed, a verdict HEY records as permanent. A PNG's orientation is not applied, so a PNG never pays twice. A 108MP phone photo taken in portrait is past the budget; 48MP ones are not. The CLI reads the JPEG's EXIF orientation itself (jpegOrientation, which reads only the markers before the scan) and the colour model from the header it already decodes. All 28 measured files get the verdict HEY gave them. --- docs/cli.md | 2 +- internal/cmd/contact_photo.go | 158 ++++++++++++++++++++++++++--- internal/cmd/contact_photo_test.go | 141 ++++++++++++++++++++++--- skills/hey/SKILL.md | 2 +- 4 files changed, 274 insertions(+), 29 deletions(-) diff --git a/docs/cli.md b/docs/cli.md index 70854ae87..86c7daab7 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -288,7 +288,7 @@ Drafts are the review-before-send lane: `hey compose --draft` (and `hey reply -- Search accepts free text plus `--required`, `--any`, `--none`, `--exact`, `--from`, `--to`, `--subject`, `--date`, `--in`, `--label`, and `--attachment`. `--in`, `--date`, `--label` and `--attachment` take one of the values `hey search filters` lists — the attachment kinds are `any`, `images`, `pdfs`, `calendar_invites`, `documents`, `spreadsheets`, `presentations`, `media` and `zip_files`, so it is `--attachment pdfs` rather than `pdf`, and an unrecognized `--in`, `--date` or `--attachment` is refused as a usage error, with the values it accepts, before any search is sent. `--in` also takes a box's kind or display name (`feedbox`, `"Paper Trail"`), the spellings `hey box view` takes, including a name you gave one of those boxes in HEY; a name it does not recognize costs one read of the box list to find out which box it is. Search can narrow only to the Imbox, The Feed, the Paper Trail and Trash: any other box, and a name no box has, is a usage error rather than `not_found`. Use `--page` for one page or `--all` to fetch up to 100 pages; capped searches report the next page for continuation. Search results include `topic_id` for reading the thread and the matching message summaries. Results also include `id` for organization actions, except when you have no box item for the thread. -Contact updates preserve omitted name, email, and alias fields. Supplying `--alias` replaces the complete alias list; `--alias=` clears it. Contact notes accept positional content, `--note`, stdin, or `$EDITOR`. `hey contact note set` replaces the whole note — HEY has no append — so adding a detail means reading the note, changing it and writing all of it back. `hey contact note show --json` (and `hey contact show --json`) answers the note three ways, each an empty string for a contact with no note: `note`, HEY's plain text, which drops formatting (bold is lost, list items become `•`); `note_html`, as HEY serves it; and `note_markdown`, the note as Markdown. `note_markdown_lossless` says whether that Markdown holds everything in the note. When it is `true`, `note set` writes the Markdown back as the same note — HEY's editor markup becomes plain paragraphs on the first write, and from then on a read and a write give the same Markdown and the same HTML. When it is `false`, the note holds an attachment, an image, a table or other markup Markdown has no syntax for, and setting Markdown would drop it: change `note_html` and write it back with `--note-html` instead. HTML passed to `--note-html` has the `
` HEY serves a note in taken off, so writing back `note_html` does not nest the note one level deeper each time. Nothing guards a read and a write against someone else saving in between: the last save wins, as it does in the web app. HEY hides contacts rather than permanently deleting them; hidden contacts leave lists, autocomplete, and search, and can be shown again by ID. `hey contact deliver --to imbox|feed|papertrail|screened-out` changes where a contact's future email arrives. It takes a contact ID — not an email address, clearance ID, box item ID, or box ID — and is write-only because HEY does not expose the current setting as structured data. Imbox removes a custom box preference; choosing a box does not screen a denied contact back in. Screened Out is the blocking choice and applies only to external email contacts. Selecting The Feed removes an existing bundle because HEY cannot keep Feed-designated contacts bundled; Imbox and Paper Trail preserve eligible bundles. Existing mail can move asynchronously, so success means HEY accepted the change, not that every affected thread has already moved. Bundling groups a contact's mail into one row without merging or deleting the underlying threads; unbundling lists those threads separately again. HEY bundles only a contact with no box preference or one sent to the Paper Trail; for any other, bundling answers success and changes nothing. `hey contact photo set ` uploads a photo for a contact, replacing any it had, and answers the contact with its new `avatar_url`; `hey contact photo remove` takes it off again, and removing from a contact with no photo succeeds. The file must be a PNG or JPEG — the formats HEY's own photo picker takes — of at most 40 MiB, the most HEY's web form can send, and at most 16000 pixels a side and 128 megapixels, the most HEY's image processing will resize. It is judged by its bytes rather than its name, and its header must decode, because HEY stores any file it is handed and fails only later, when it resizes the photo to show it: a HEIC from a phone, a 200-megapixel photo, or a file that is not an image would leave the contact with a broken avatar. HEY crops the photo to a square, so a square image without transparency works best, and it caches avatars, so the old photo can still show in places for up to 30 minutes. Your own contact takes a photo too. A contact that is hidden, an alias, or another HEY user cannot be edited: updating it, setting or deleting its note, or setting or removing its photo answers `not_found`. `hey contact list` holds only non-HEY senders you have screened in — other HEY users, senders still in or denied by The Screener, aliases and hidden contacts are left out. It has no search: HEY ignores a query on that list, so finding a contact by email means reading every page (`--all`) and filtering, which matches primary addresses only. +Contact updates preserve omitted name, email, and alias fields. Supplying `--alias` replaces the complete alias list; `--alias=` clears it. Contact notes accept positional content, `--note`, stdin, or `$EDITOR`. `hey contact note set` replaces the whole note — HEY has no append — so adding a detail means reading the note, changing it and writing all of it back. `hey contact note show --json` (and `hey contact show --json`) answers the note three ways, each an empty string for a contact with no note: `note`, HEY's plain text, which drops formatting (bold is lost, list items become `•`); `note_html`, as HEY serves it; and `note_markdown`, the note as Markdown. `note_markdown_lossless` says whether that Markdown holds everything in the note. When it is `true`, `note set` writes the Markdown back as the same note — HEY's editor markup becomes plain paragraphs on the first write, and from then on a read and a write give the same Markdown and the same HTML. When it is `false`, the note holds an attachment, an image, a table or other markup Markdown has no syntax for, and setting Markdown would drop it: change `note_html` and write it back with `--note-html` instead. HTML passed to `--note-html` has the `
` HEY serves a note in taken off, so writing back `note_html` does not nest the note one level deeper each time. Nothing guards a read and a write against someone else saving in between: the last save wins, as it does in the web app. HEY hides contacts rather than permanently deleting them; hidden contacts leave lists, autocomplete, and search, and can be shown again by ID. `hey contact deliver --to imbox|feed|papertrail|screened-out` changes where a contact's future email arrives. It takes a contact ID — not an email address, clearance ID, box item ID, or box ID — and is write-only because HEY does not expose the current setting as structured data. Imbox removes a custom box preference; choosing a box does not screen a denied contact back in. Screened Out is the blocking choice and applies only to external email contacts. Selecting The Feed removes an existing bundle because HEY cannot keep Feed-designated contacts bundled; Imbox and Paper Trail preserve eligible bundles. Existing mail can move asynchronously, so success means HEY accepted the change, not that every affected thread has already moved. Bundling groups a contact's mail into one row without merging or deleting the underlying threads; unbundling lists those threads separately again. HEY bundles only a contact with no box preference or one sent to the Paper Trail; for any other, bundling answers success and changes nothing. `hey contact photo set ` uploads a photo for a contact, replacing any it had, and answers the contact with its new `avatar_url`; `hey contact photo remove` takes it off again, and removing from a contact with no photo succeeds. The file must be a PNG or JPEG — the formats HEY's own photo picker takes — of at most 40 MiB, the most HEY's web form can send, and at most 16000 pixels a side. It must also fit the memory HEY resizes a photo in, 896 MiB at 8 bytes a pixel: about 117 megapixels, or 58 for a JPEG whose EXIF orientation rotates or flips it, since turning it upright holds a second copy; a CMYK JPEG costs 10 bytes a pixel, so 94, or 47 rotated. Those limits were measured against HEY's own resize, and a photo past them is not resized at all. It is judged by its bytes rather than its name, and its header must decode, because HEY stores any file it is handed and fails only later, when it resizes the photo to show it: a HEIC from a phone, a 200-megapixel photo, a 108-megapixel one taken in portrait, or a file that is not an image would leave the contact with a broken avatar. HEY crops the photo to a square, so a square image without transparency works best, and it caches avatars, so the old photo can still show in places for up to 30 minutes. Your own contact takes a photo too. A contact that is hidden, an alias, or another HEY user cannot be edited: updating it, setting or deleting its note, or setting or removing its photo answers `not_found`. `hey contact list` holds only non-HEY senders you have screened in — other HEY users, senders still in or denied by The Screener, aliases and hidden contacts are left out. It has no search: HEY ignores a query on that list, so finding a contact by email means reading every page (`--all`) and filtering, which matches primary addresses only. The Screener is where first-time senders wait. `hey screener list` returns clearance IDs — not contact IDs — with the sender and the subject of what they sent, plus `topic_id` for reading the thread before deciding. `--count` asks for the number alone, which is a far cheaper request than the queue, and prints it as a bare number like every other command's `--count`, so `n=$(hey screener list --count)` reads it directly. Approving delivers everything the sender has waiting; denying hides it. Either is reversible with the opposite command, and `hey screener history` shows what was already decided. Both listings page the way `hey box view` does: `--all` follows HEY's cursor to the end of the queue (up to 100 pages), and a single-page or capped read reports `next_page` in its JSON meta, which `--page ` continues from — the cursor is opaque, so a page *number* does not name a position. `hey screener list` also reports `total_count`, the whole queue's size, next to what the read returned. `--box` and `--seen` approve one sender at a time; several IDs go through HEY's bulk endpoint, which takes neither. `--seen` reliably marks the mail read only for a sender with one waiting thread. `--spam` marks what they sent as spam and, for a single ID, trains HEY's filter, which is harder to undo than denying. `hey screener clear` moves everything waiting to Trash (a shared thread loses your access instead) without approving or denying anyone — those senders reappear on their next email. diff --git a/internal/cmd/contact_photo.go b/internal/cmd/contact_photo.go index d4fe2b423..f64e973e4 100644 --- a/internal/cmd/contact_photo.go +++ b/internal/cmd/contact_photo.go @@ -1,9 +1,12 @@ package cmd import ( + "bufio" + "encoding/binary" "errors" "fmt" "image" + "image/color" _ "image/jpeg" // registers the decoder contactPhoto checks a JPEG with _ "image/png" // registers the decoder contactPhoto checks a PNG with "io" @@ -34,14 +37,26 @@ var contactPhotoFormats = map[string]string{ "image/jpeg": "jpeg", } -// HEY resizes a photo with ImageMagick under hotcell/policy.xml, which refuses an image -// wider or taller than 16KP outright and moves one over 128MP to a disk cache that its -// disk ceiling then refuses (haystack's imagemagick_policy_test proves 15000×15000). -// HEY would store such a photo and then fail to draw it, so the CLI refuses it first. -// A 200MP phone photo is past both. +// HEY resizes a photo with ImageMagick 6 (Q16) in its hotcell, and a photo it cannot +// resize is stored anyway and then never drawn. Two ceilings decide it, measured by running +// HEY's own variant command under hotcell/policy.xml and the cell's limits: +// +// - policy.xml refuses an image wider or taller than 16KP outright; +// - the cell's MAGICK_MEMORY_LIMIT is 896MiB per image (hotcell/Dockerfile), and a pixel +// costs 8 bytes in it, 10 in a CMYK JPEG. Past that the cache spills to disk, where the +// worker's 256MB file_size kills it. Auto-orienting a JPEG whose EXIF orientation is +// anything but upright — a rotation or a flip — holds a second full copy, halving what +// fits. A PNG's orientation is not applied. +// +// So an upright photo tops out near 117 megapixels, a rotated one near 58, a CMYK one near +// 94 and a rotated CMYK one near 47. policy.xml's 128MP area ceiling is never the one that +// binds. A 200MP phone photo is past the side limit; a 108MP one is past the pixel budget +// whenever it was taken in portrait. const ( - contactPhotoMaxSide = 16_000 - contactPhotoMaxPixels = 128_000_000 + contactPhotoMaxSide = 16_000 + contactPhotoPixelCacheBytes = 896 << 20 + contactPhotoBytesPerPixel = 8 + contactPhotoCMYKBytesPerPixel = 10 ) // contactPhotoNotEditableHint is what a not_found from the photo routes usually means: @@ -59,7 +74,7 @@ func newContactPhotoCommand() *contactPhotoCommand { Aliases: []string{"avatar"}, Short: "Set or remove a contact's photo", Annotations: map[string]string{ - "agent_notes": "Set takes a contact ID and a local PNG or JPEG file of at most 40 MiB, 16000 pixels a side and 128 megapixels; remove takes a contact ID. The photo is the contact's avatar_url. Hidden contacts, aliases and other HEY users answer not_found.", + "agent_notes": "Set takes a contact ID and a local PNG or JPEG file of at most 40 MiB and 16000 pixels a side, and of at most about 117 megapixels (58 when its EXIF orientation rotates or flips it, 94 or 47 for a CMYK JPEG); remove takes a contact ID. The photo is the contact's avatar_url. Hidden contacts, aliases and other HEY users answer not_found.", }, } photoCommand.cmd.AddCommand(newContactPhotoSetCommand().cmd) @@ -79,7 +94,8 @@ func newContactPhotoSetCommand() *contactPhotoSetCommand { Use: "set ", Short: "Upload a photo for a contact", Long: "Upload a local image as a contact's photo, replacing any photo they already have. " + - "The file must be a PNG or JPEG of at most 40 MiB, 16000 pixels a side and 128 megapixels, judged by its contents rather than its name. " + + "The file must be a PNG or JPEG of at most 40 MiB and 16000 pixels a side, judged by its contents rather than its name. " + + "It must also fit the memory HEY resizes a photo in: about 117 megapixels, 58 for a JPEG its EXIF orientation rotates or flips, and 94 or 47 for a CMYK JPEG. " + "HEY crops it to a square, so a square image without transparency works best. " + "Only a contact you can edit, or your own, takes a photo: a hidden contact, an alias or another HEY user answers not_found. " + "HEY caches avatars, so the old photo may still show in places for up to 30 minutes.", @@ -250,15 +266,27 @@ func checkContactPhoto(file *os.File, path string) (preparedAttachment, error) { if err != nil || decoded != format || config.Width <= 0 || config.Height <= 0 { return preparedAttachment{}, apierr.ErrUsage(fmt.Sprintf("photo %q is not a readable %s image", path, format)) } - if config.Width > contactPhotoMaxSide || config.Height > contactPhotoMaxSide || - int64(config.Width)*int64(config.Height) > contactPhotoMaxPixels { + if config.Width > contactPhotoMaxSide || config.Height > contactPhotoMaxSide { return preparedAttachment{}, apierr.ErrUsageHint( fmt.Sprintf("photo %q is %d×%d pixels, too large for HEY to resize", path, config.Width, config.Height), - "Scale it down to at most 16000 pixels a side and 128 megapixels in all; HEY shows a contact photo at 300×300.") + fmt.Sprintf("Scale it down to at most %d pixels a side; HEY shows a contact photo at 300×300.", contactPhotoMaxSide)) } if _, err = file.Seek(0, io.SeekStart); err != nil { return preparedAttachment{}, fmt.Errorf("rewind photo %q: %w", path, err) } + budget := contactPhotoBudget{cmyk: config.ColorModel == color.CMYKModel} + if format == "jpeg" { + budget.reoriented = jpegOrientation(file) > 1 + if _, err = file.Seek(0, io.SeekStart); err != nil { + return preparedAttachment{}, fmt.Errorf("rewind photo %q: %w", path, err) + } + } + if pixels := int64(config.Width) * int64(config.Height); pixels > budget.maxPixels() { + return preparedAttachment{}, apierr.ErrUsageHint( + fmt.Sprintf("photo %q is %d×%d pixels, too large for HEY to resize", path, config.Width, config.Height), + fmt.Sprintf("HEY can resize %s of at most %.1f megapixels; scale it down first. HEY shows a contact photo at 300×300.", + budget.describe(), float64(budget.maxPixels())/1e6)) + } return preparedAttachment{ path: path, @@ -268,3 +296,109 @@ func checkContactPhoto(file *os.File, path string) (preparedAttachment, error) { file: file, }, nil } + +// contactPhotoBudget is what one photo costs HEY's ImageMagick per pixel. +type contactPhotoBudget struct { + cmyk bool + reoriented bool +} + +func (b contactPhotoBudget) maxPixels() int64 { + perPixel := int64(contactPhotoBytesPerPixel) + if b.cmyk { + perPixel = contactPhotoCMYKBytesPerPixel + } + if b.reoriented { + perPixel *= 2 + } + return contactPhotoPixelCacheBytes / perPixel +} + +func (b contactPhotoBudget) describe() string { + kind := "a photo" + if b.cmyk { + kind = "a CMYK JPEG" + } + if b.reoriented { + kind += " that its EXIF orientation rotates or flips" + } + return kind +} + +// jpegOrientation is the EXIF orientation of the JPEG r starts, 1 (upright) when it has +// none or it cannot be read — as it is for ImageMagick, which then leaves the image alone. +// It reads the markers before the scan and only an Exif APP1 segment's IFD0. +func jpegOrientation(r io.Reader) int { + br := bufio.NewReader(r) + var soi [2]byte + if _, err := io.ReadFull(br, soi[:]); err != nil || soi != [2]byte{0xFF, 0xD8} { + return 1 + } + for { + // A marker is 0xFF, any number of 0xFF fill bytes, then its code. + b, err := br.ReadByte() + if err != nil || b != 0xFF { + return 1 + } + for b == 0xFF { + if b, err = br.ReadByte(); err != nil { + return 1 + } + } + switch { + case b == 0xDA || b == 0xD9: // start of scan, end of image + return 1 + case b == 0x01 || (b >= 0xD0 && b <= 0xD7): // markers without a length + continue + } + var length [2]byte + if _, err := io.ReadFull(br, length[:]); err != nil { + return 1 + } + size := int(binary.BigEndian.Uint16(length[:])) - 2 + if size < 0 { + return 1 + } + segment := make([]byte, size) + if _, err := io.ReadFull(br, segment); err != nil { + return 1 + } + if b == 0xE1 && len(segment) >= 6 && string(segment[:6]) == "Exif\x00\x00" { + return exifOrientation(segment[6:]) + } + } +} + +// exifOrientation reads tag 0x0112 from the IFD0 of a TIFF-structured EXIF block. +func exifOrientation(tiff []byte) int { + if len(tiff) < 8 { + return 1 + } + var order binary.ByteOrder + switch string(tiff[:2]) { + case "II": + order = binary.LittleEndian + case "MM": + order = binary.BigEndian + default: + return 1 + } + ifd := int64(order.Uint32(tiff[4:8])) + if ifd < 8 || ifd+2 > int64(len(tiff)) { + return 1 + } + entries := int64(order.Uint16(tiff[ifd:])) + for i := int64(0); i < entries; i++ { + entry := ifd + 2 + i*12 + if entry+12 > int64(len(tiff)) { + return 1 + } + if order.Uint16(tiff[entry:]) == 0x0112 && order.Uint16(tiff[entry+2:]) == 3 { // SHORT + if orientation := int(order.Uint16(tiff[entry+8:])); orientation >= 1 && orientation <= 8 { + return orientation + } + return 1 + } + } + return 1 +} diff --git a/internal/cmd/contact_photo_test.go b/internal/cmd/contact_photo_test.go index 7317c96e0..661ab0e64 100644 --- a/internal/cmd/contact_photo_test.go +++ b/internal/cmd/contact_photo_test.go @@ -10,6 +10,7 @@ import ( "image/color" "image/jpeg" "image/png" + "io" "net/http" "net/http/httptest" "os" @@ -279,27 +280,128 @@ func pngHeader(width, height uint32) []byte { return binary.BigEndian.AppendUint32(header, crc32.ChecksumIEEE(chunk)) } +type exifByteOrder interface { + binary.ByteOrder + binary.AppendByteOrder +} + +// jpegHeader is the start of a JPEG claiming the given dimensions and component count +// (1 gray, 3 color, 4 CMYK), with an EXIF orientation when orientation is non-zero, in the +// byte order named: everything a decoder reads before the scan, and no pixels. +func jpegHeader(width, height uint16, components byte, orientation uint16, order exifByteOrder) []byte { + header := []byte{0xFF, 0xD8} + if orientation != 0 { + tiff := []byte("MM") + if order == binary.LittleEndian { + tiff = []byte("II") + } + tiff = order.AppendUint16(tiff, 42) + tiff = order.AppendUint32(tiff, 8) // IFD0 offset + tiff = order.AppendUint16(tiff, 2) // entries + tiff = order.AppendUint16(tiff, 0x010F) // Make, which is not the orientation + tiff = order.AppendUint16(tiff, 2) + tiff = order.AppendUint32(tiff, 4) + tiff = append(tiff, "HEY\x00"...) + tiff = order.AppendUint16(tiff, 0x0112) + tiff = order.AppendUint16(tiff, 3) // SHORT + tiff = order.AppendUint32(tiff, 1) + tiff = order.AppendUint16(tiff, orientation) + tiff = order.AppendUint16(tiff, 0) + tiff = order.AppendUint32(tiff, 0) // no next IFD + app1 := append([]byte("Exif\x00\x00"), tiff...) + header = append(header, 0xFF, 0xE1) + header = binary.BigEndian.AppendUint16(header, uint16(len(app1)+2)) + header = append(header, app1...) + } + sof := []byte{8} + sof = binary.BigEndian.AppendUint16(sof, height) + sof = binary.BigEndian.AppendUint16(sof, width) + sof = append(sof, components) + for id := byte(1); id <= components; id++ { + sof = append(sof, id, 0x11, 0) + } + header = append(header, 0xFF, 0xC0) + header = binary.BigEndian.AppendUint16(header, uint16(len(sof)+2)) + header = append(header, sof...) + return append(header, 0xFF, 0xDA, 0x00, 0x08, 0x01, 0x01, 0x00, 0x00, 0x3F, 0x00) +} + +func TestJPEGOrientationReadsTheExifTag(t *testing.T) { + for _, order := range []exifByteOrder{binary.BigEndian, binary.LittleEndian} { + for orientation := uint16(1); orientation <= 8; orientation++ { + got := jpegOrientation(bytes.NewReader(jpegHeader(640, 480, 3, orientation, order))) + if got != int(orientation) { + t.Errorf("%v orientation %d read as %d", order, orientation, got) + } + } + } + upright := jpegHeader(640, 480, 3, 6, binary.BigEndian) + for name, content := range map[string][]byte{ + "no exif": jpegHeader(640, 480, 3, 0, nil), + "out of range": jpegHeader(640, 480, 3, 9, binary.BigEndian), + "a real encoder's": jpegPhoto(t), + "not a jpeg": pngPhoto(t), + "cut off in the exif": upright[:30], + "empty": nil, + } { + if got := jpegOrientation(bytes.NewReader(content)); got != 1 { + t.Errorf("%s: orientation = %d, want 1", name, got) + } + } +} + +func TestJPEGOrientationSkipsFillBytesBeforeAMarker(t *testing.T) { + rotated := jpegHeader(640, 480, 3, 6, binary.BigEndian) + for fill := 1; fill <= 3; fill++ { + padded := append([]byte{0xFF, 0xD8}, bytes.Repeat([]byte{0xFF}, fill)...) + padded = append(padded, rotated[2:]...) + if got := jpegOrientation(bytes.NewReader(padded)); got != 6 { + t.Errorf("%d fill bytes: orientation = %d, want 6", fill, got) + } + } +} + +func TestJPEGOrientationSurvivesEveryTruncation(t *testing.T) { + rotated := jpegHeader(640, 480, 3, 6, binary.LittleEndian) + for cut := range rotated { + _ = jpegOrientation(bytes.NewReader(rotated[:cut])) + } +} + +// The boundaries below are HEY's, measured by running its variant command under +// hotcell/policy.xml and the cell's limits: 10800×10800 resizes and 11000×11000 does not; +// rotated, 7600×7600 does and 7700×7700 does not; CMYK, 9600×9600 does and 9800×9600 does +// not; rotated CMYK, 6000×6000 does and 7000×7000 does not. Each case sits on the line the +// budget draws between them. func TestContactPhotoSetRefusesPhotosTooLargeForHEYToResize(t *testing.T) { tests := []struct { - name string - width, height uint32 + name string + content []byte + hint string }{ - {name: "too wide", width: contactPhotoMaxSide + 1, height: 1}, - {name: "too tall", width: 1, height: contactPhotoMaxSide + 1}, - {name: "too many pixels inside the side limit", width: 15_000, height: 15_000}, - {name: "a 200 megapixel phone photo", width: 16_320, height: 12_240}, + {name: "too wide", content: pngHeader(contactPhotoMaxSide+1, 1), hint: "16000 pixels a side"}, + {name: "too tall", content: jpegHeader(1, contactPhotoMaxSide+1, 3, 0, nil), hint: "16000 pixels a side"}, + {name: "a 200 megapixel phone photo", content: jpegHeader(16_320, 12_240, 3, 0, nil), hint: "16000 pixels a side"}, + {name: "upright png", content: pngHeader(16_000, 7_341), hint: "a photo of at most 117.4 megapixels"}, + {name: "upright jpeg", content: jpegHeader(16_000, 7_341, 3, 1, binary.BigEndian), hint: "a photo of at most 117.4 megapixels"}, + {name: "grayscale jpeg", content: jpegHeader(16_000, 7_341, 1, 0, nil), hint: "a photo of at most 117.4 megapixels"}, + {name: "a 108 megapixel portrait", content: jpegHeader(12_000, 9_000, 3, 6, binary.LittleEndian), hint: "a photo that its EXIF orientation rotates or flips of at most 58.7 megapixels"}, + {name: "rotated jpeg", content: jpegHeader(8_000, 7_341, 3, 8, binary.BigEndian), hint: "rotates or flips of at most 58.7 megapixels"}, + {name: "mirrored jpeg", content: jpegHeader(8_000, 7_341, 3, 2, binary.BigEndian), hint: "rotates or flips of at most 58.7 megapixels"}, + {name: "cmyk jpeg", content: jpegHeader(16_000, 5_873, 4, 0, nil), hint: "a CMYK JPEG of at most 94.0 megapixels"}, + {name: "rotated cmyk jpeg", content: jpegHeader(8_000, 5_873, 4, 6, binary.BigEndian), hint: "a CMYK JPEG that its EXIF orientation rotates or flips of at most 47.0 megapixels"}, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { state := newContactPhotoServer(t) - photo := writePhotoFixture(t, "jane-doe.png", pngHeader(tc.width, tc.height)) + photo := writePhotoFixture(t, "jane-doe.jpg", tc.content) _, err := runContacts(t, state.server, "photo", "set", "7", photo) var cliErr *apierr.Error if !errors.As(err, &cliErr) || cliErr.Code != apierr.CodeUsage { t.Fatalf("error = %v, want usage error", err) } - if !strings.Contains(cliErr.Message, "too large for HEY to resize") { - t.Errorf("message = %q", cliErr.Message) + if !strings.Contains(cliErr.Message, "too large for HEY to resize") || !strings.Contains(cliErr.Hint, tc.hint) { + t.Errorf("message = %q, hint = %q, want hint containing %q", cliErr.Message, cliErr.Hint, tc.hint) } if requests := state.requestLines(); len(requests) != 0 { t.Fatalf("a refused photo sent requests: %v", requests) @@ -309,17 +411,26 @@ func TestContactPhotoSetRefusesPhotosTooLargeForHEYToResize(t *testing.T) { } func TestContactPhotoSetAcceptsPhotosAtTheResizeLimits(t *testing.T) { - for _, size := range [][2]uint32{ - {contactPhotoMaxSide, 1}, - {1, contactPhotoMaxSide}, - {contactPhotoMaxSide, contactPhotoMaxPixels / contactPhotoMaxSide}, + for name, content := range map[string][]byte{ + "widest png": pngHeader(contactPhotoMaxSide, 1), + "tallest jpeg": jpegHeader(1, contactPhotoMaxSide, 3, 0, nil), + "largest upright png": pngHeader(16_000, 7_340), + "largest upright jpeg": jpegHeader(16_000, 7_340, 3, 1, binary.LittleEndian), + "png whatever its exif": pngHeader(16_000, 7_340), + "largest rotated jpeg": jpegHeader(8_000, 7_340, 3, 6, binary.BigEndian), + "largest cmyk jpeg": jpegHeader(16_000, 5_872, 4, 0, nil), + "largest rotated cmyk": jpegHeader(8_000, 5_872, 4, 5, binary.LittleEndian), + "a 48 megapixel portrait": jpegHeader(8_064, 6_048, 3, 6, binary.LittleEndian), } { - photo := writePhotoFixture(t, "jane-doe.png", pngHeader(size[0], size[1])) + photo := writePhotoFixture(t, "jane-doe.jpg", content) prepared, err := openContactPhoto(photo) if err != nil { - t.Errorf("%d×%d was refused: %v", size[0], size[1], err) + t.Errorf("%s was refused: %v", name, err) continue } + if offset, _ := prepared.file.Seek(0, io.SeekCurrent); offset != 0 { + t.Errorf("%s: the photo was left at offset %d, not its start", name, offset) + } closePreparedAttachments([]preparedAttachment{prepared}) } } diff --git a/skills/hey/SKILL.md b/skills/hey/SKILL.md index f57e2f78a..9351a56e2 100644 --- a/skills/hey/SKILL.md +++ b/skills/hey/SKILL.md @@ -212,7 +212,7 @@ postings, not the box, label or contact around them. | Read private contact note | `hey contact note show --json` (`note_markdown` is the form `note set` takes when `note_markdown_lossless` is true) | | Replace private contact note | `hey contact note set "Prefers email"` (replaces the whole note) | | Delete private contact note | `hey contact note delete ` | -| Set a contact's photo | `hey contact photo set ` (PNG or JPEG, at most 40 MiB, 16000 px a side and 128 megapixels) | +| Set a contact's photo | `hey contact photo set ` (PNG or JPEG, at most 40 MiB, 16000 px a side and ~117 megapixels; ~58 if EXIF-rotated) | | Remove a contact's photo | `hey contact photo remove ` | | Read email thread | `hey thread read --json` | | Rename a thread | `hey thread update --name "Kitchen renovation quotes"` | From 1e450bebc70ce9c9d3a61cef8055fad9f825c3c2 Mon Sep 17 00:00:00 2001 From: Rob Zolkos Date: Mon, 12 Oct 2026 16:38:55 -0400 Subject: [PATCH 4/6] Cap a contact photo at 58 megapixels rather than modelling HEY's resize MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit HEY shows a contact photo at 300×300 at most and does the resizing itself, so the CLI only needs a sane ceiling, not a model of ImageMagick's memory. 58MP admits a full-resolution photo from a modern phone (an iPhone's 48MP, 8064×6048; 50MP Android cameras) and sits under the tightest ceiling measured against HEY's own variant command: about 58.7MP for a JPEG whose EXIF orientation has to be applied (7600² resizes, 7700² kills the worker). The 16000-pixel side limit from policy.xml stays. That retires the per-colour-model budget and the EXIF orientation reader. The one case left uncovered is a rotated CMYK JPEG over 47MP, which no camera produces. --- docs/cli.md | 2 +- internal/cmd/contact_photo.go | 165 ++++------------------------- internal/cmd/contact_photo_test.go | 140 +++++------------------- skills/hey/SKILL.md | 2 +- 4 files changed, 44 insertions(+), 265 deletions(-) diff --git a/docs/cli.md b/docs/cli.md index 86c7daab7..2404b2644 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -288,7 +288,7 @@ Drafts are the review-before-send lane: `hey compose --draft` (and `hey reply -- Search accepts free text plus `--required`, `--any`, `--none`, `--exact`, `--from`, `--to`, `--subject`, `--date`, `--in`, `--label`, and `--attachment`. `--in`, `--date`, `--label` and `--attachment` take one of the values `hey search filters` lists — the attachment kinds are `any`, `images`, `pdfs`, `calendar_invites`, `documents`, `spreadsheets`, `presentations`, `media` and `zip_files`, so it is `--attachment pdfs` rather than `pdf`, and an unrecognized `--in`, `--date` or `--attachment` is refused as a usage error, with the values it accepts, before any search is sent. `--in` also takes a box's kind or display name (`feedbox`, `"Paper Trail"`), the spellings `hey box view` takes, including a name you gave one of those boxes in HEY; a name it does not recognize costs one read of the box list to find out which box it is. Search can narrow only to the Imbox, The Feed, the Paper Trail and Trash: any other box, and a name no box has, is a usage error rather than `not_found`. Use `--page` for one page or `--all` to fetch up to 100 pages; capped searches report the next page for continuation. Search results include `topic_id` for reading the thread and the matching message summaries. Results also include `id` for organization actions, except when you have no box item for the thread. -Contact updates preserve omitted name, email, and alias fields. Supplying `--alias` replaces the complete alias list; `--alias=` clears it. Contact notes accept positional content, `--note`, stdin, or `$EDITOR`. `hey contact note set` replaces the whole note — HEY has no append — so adding a detail means reading the note, changing it and writing all of it back. `hey contact note show --json` (and `hey contact show --json`) answers the note three ways, each an empty string for a contact with no note: `note`, HEY's plain text, which drops formatting (bold is lost, list items become `•`); `note_html`, as HEY serves it; and `note_markdown`, the note as Markdown. `note_markdown_lossless` says whether that Markdown holds everything in the note. When it is `true`, `note set` writes the Markdown back as the same note — HEY's editor markup becomes plain paragraphs on the first write, and from then on a read and a write give the same Markdown and the same HTML. When it is `false`, the note holds an attachment, an image, a table or other markup Markdown has no syntax for, and setting Markdown would drop it: change `note_html` and write it back with `--note-html` instead. HTML passed to `--note-html` has the `
` HEY serves a note in taken off, so writing back `note_html` does not nest the note one level deeper each time. Nothing guards a read and a write against someone else saving in between: the last save wins, as it does in the web app. HEY hides contacts rather than permanently deleting them; hidden contacts leave lists, autocomplete, and search, and can be shown again by ID. `hey contact deliver --to imbox|feed|papertrail|screened-out` changes where a contact's future email arrives. It takes a contact ID — not an email address, clearance ID, box item ID, or box ID — and is write-only because HEY does not expose the current setting as structured data. Imbox removes a custom box preference; choosing a box does not screen a denied contact back in. Screened Out is the blocking choice and applies only to external email contacts. Selecting The Feed removes an existing bundle because HEY cannot keep Feed-designated contacts bundled; Imbox and Paper Trail preserve eligible bundles. Existing mail can move asynchronously, so success means HEY accepted the change, not that every affected thread has already moved. Bundling groups a contact's mail into one row without merging or deleting the underlying threads; unbundling lists those threads separately again. HEY bundles only a contact with no box preference or one sent to the Paper Trail; for any other, bundling answers success and changes nothing. `hey contact photo set ` uploads a photo for a contact, replacing any it had, and answers the contact with its new `avatar_url`; `hey contact photo remove` takes it off again, and removing from a contact with no photo succeeds. The file must be a PNG or JPEG — the formats HEY's own photo picker takes — of at most 40 MiB, the most HEY's web form can send, and at most 16000 pixels a side. It must also fit the memory HEY resizes a photo in, 896 MiB at 8 bytes a pixel: about 117 megapixels, or 58 for a JPEG whose EXIF orientation rotates or flips it, since turning it upright holds a second copy; a CMYK JPEG costs 10 bytes a pixel, so 94, or 47 rotated. Those limits were measured against HEY's own resize, and a photo past them is not resized at all. It is judged by its bytes rather than its name, and its header must decode, because HEY stores any file it is handed and fails only later, when it resizes the photo to show it: a HEIC from a phone, a 200-megapixel photo, a 108-megapixel one taken in portrait, or a file that is not an image would leave the contact with a broken avatar. HEY crops the photo to a square, so a square image without transparency works best, and it caches avatars, so the old photo can still show in places for up to 30 minutes. Your own contact takes a photo too. A contact that is hidden, an alias, or another HEY user cannot be edited: updating it, setting or deleting its note, or setting or removing its photo answers `not_found`. `hey contact list` holds only non-HEY senders you have screened in — other HEY users, senders still in or denied by The Screener, aliases and hidden contacts are left out. It has no search: HEY ignores a query on that list, so finding a contact by email means reading every page (`--all`) and filtering, which matches primary addresses only. +Contact updates preserve omitted name, email, and alias fields. Supplying `--alias` replaces the complete alias list; `--alias=` clears it. Contact notes accept positional content, `--note`, stdin, or `$EDITOR`. `hey contact note set` replaces the whole note — HEY has no append — so adding a detail means reading the note, changing it and writing all of it back. `hey contact note show --json` (and `hey contact show --json`) answers the note three ways, each an empty string for a contact with no note: `note`, HEY's plain text, which drops formatting (bold is lost, list items become `•`); `note_html`, as HEY serves it; and `note_markdown`, the note as Markdown. `note_markdown_lossless` says whether that Markdown holds everything in the note. When it is `true`, `note set` writes the Markdown back as the same note — HEY's editor markup becomes plain paragraphs on the first write, and from then on a read and a write give the same Markdown and the same HTML. When it is `false`, the note holds an attachment, an image, a table or other markup Markdown has no syntax for, and setting Markdown would drop it: change `note_html` and write it back with `--note-html` instead. HTML passed to `--note-html` has the `
` HEY serves a note in taken off, so writing back `note_html` does not nest the note one level deeper each time. Nothing guards a read and a write against someone else saving in between: the last save wins, as it does in the web app. HEY hides contacts rather than permanently deleting them; hidden contacts leave lists, autocomplete, and search, and can be shown again by ID. `hey contact deliver --to imbox|feed|papertrail|screened-out` changes where a contact's future email arrives. It takes a contact ID — not an email address, clearance ID, box item ID, or box ID — and is write-only because HEY does not expose the current setting as structured data. Imbox removes a custom box preference; choosing a box does not screen a denied contact back in. Screened Out is the blocking choice and applies only to external email contacts. Selecting The Feed removes an existing bundle because HEY cannot keep Feed-designated contacts bundled; Imbox and Paper Trail preserve eligible bundles. Existing mail can move asynchronously, so success means HEY accepted the change, not that every affected thread has already moved. Bundling groups a contact's mail into one row without merging or deleting the underlying threads; unbundling lists those threads separately again. HEY bundles only a contact with no box preference or one sent to the Paper Trail; for any other, bundling answers success and changes nothing. `hey contact photo set ` uploads a photo for a contact, replacing any it had, and answers the contact with its new `avatar_url`; `hey contact photo remove` takes it off again, and removing from a contact with no photo succeeds. The file must be a PNG or JPEG — the formats HEY's own photo picker takes — of at most 40 MiB, the most HEY's web form can send, and at most 58 megapixels and 16000 pixels a side. That admits a full-resolution photo from a modern phone (an iPhone's 48 megapixels, or 50 on Android) and stays inside what HEY can resize, which for a JPEG it has to rotate upright is about 58.7 megapixels; HEY shows a contact photo at 300×300 at most anyway. It is judged by its bytes rather than its name, and its header must decode, because HEY stores any file it is handed and fails only later, when it resizes the photo to show it: a HEIC from a phone, a photo too large to resize, or a file that is not an image would leave the contact with a broken avatar. HEY crops the photo to a square, so a square image without transparency works best, and it caches avatars, so the old photo can still show in places for up to 30 minutes. Your own contact takes a photo too. A contact that is hidden, an alias, or another HEY user cannot be edited: updating it, setting or deleting its note, or setting or removing its photo answers `not_found`. `hey contact list` holds only non-HEY senders you have screened in — other HEY users, senders still in or denied by The Screener, aliases and hidden contacts are left out. It has no search: HEY ignores a query on that list, so finding a contact by email means reading every page (`--all`) and filtering, which matches primary addresses only. The Screener is where first-time senders wait. `hey screener list` returns clearance IDs — not contact IDs — with the sender and the subject of what they sent, plus `topic_id` for reading the thread before deciding. `--count` asks for the number alone, which is a far cheaper request than the queue, and prints it as a bare number like every other command's `--count`, so `n=$(hey screener list --count)` reads it directly. Approving delivers everything the sender has waiting; denying hides it. Either is reversible with the opposite command, and `hey screener history` shows what was already decided. Both listings page the way `hey box view` does: `--all` follows HEY's cursor to the end of the queue (up to 100 pages), and a single-page or capped read reports `next_page` in its JSON meta, which `--page ` continues from — the cursor is opaque, so a page *number* does not name a position. `hey screener list` also reports `total_count`, the whole queue's size, next to what the read returned. `--box` and `--seen` approve one sender at a time; several IDs go through HEY's bulk endpoint, which takes neither. `--seen` reliably marks the mail read only for a sender with one waiting thread. `--spam` marks what they sent as spam and, for a single ID, trains HEY's filter, which is harder to undo than denying. `hey screener clear` moves everything waiting to Trash (a shared thread loses your access instead) without approving or denying anyone — those senders reappear on their next email. diff --git a/internal/cmd/contact_photo.go b/internal/cmd/contact_photo.go index f64e973e4..3b8c1416f 100644 --- a/internal/cmd/contact_photo.go +++ b/internal/cmd/contact_photo.go @@ -1,12 +1,9 @@ package cmd import ( - "bufio" - "encoding/binary" "errors" "fmt" "image" - "image/color" _ "image/jpeg" // registers the decoder contactPhoto checks a JPEG with _ "image/png" // registers the decoder contactPhoto checks a PNG with "io" @@ -37,26 +34,19 @@ var contactPhotoFormats = map[string]string{ "image/jpeg": "jpeg", } -// HEY resizes a photo with ImageMagick 6 (Q16) in its hotcell, and a photo it cannot -// resize is stored anyway and then never drawn. Two ceilings decide it, measured by running -// HEY's own variant command under hotcell/policy.xml and the cell's limits: -// -// - policy.xml refuses an image wider or taller than 16KP outright; -// - the cell's MAGICK_MEMORY_LIMIT is 896MiB per image (hotcell/Dockerfile), and a pixel -// costs 8 bytes in it, 10 in a CMYK JPEG. Past that the cache spills to disk, where the -// worker's 256MB file_size kills it. Auto-orienting a JPEG whose EXIF orientation is -// anything but upright — a rotation or a flip — holds a second full copy, halving what -// fits. A PNG's orientation is not applied. -// -// So an upright photo tops out near 117 megapixels, a rotated one near 58, a CMYK one near -// 94 and a rotated CMYK one near 47. policy.xml's 128MP area ceiling is never the one that -// binds. A 200MP phone photo is past the side limit; a 108MP one is past the pixel budget -// whenever it was taken in portrait. +// contactPhotoMaxPixels and contactPhotoMaxSide are the largest photo the CLI uploads. HEY +// shows a contact photo at 300×300 at most, so the cap is not about quality: it admits a +// full-resolution photo from a modern phone — an iPhone's 48MP is 8064×6048, and 50MP +// Android cameras are common — and stays inside what HEY can resize. HEY stores whatever it +// is handed and only then resizes it, in ImageMagick under hotcell/policy.xml and the +// cell's 896MiB memory limit; a photo past those is never drawn. Measured against HEY's own +// variant command, the tightest ceiling is a JPEG whose EXIF orientation has to be applied, +// which holds a second copy: 7600×7600 resizes and 7700×7700 kills the worker, about 58.7MP. +// An upright photo goes to about 117MP, and policy.xml refuses any side over 16000 pixels. +// 58MP clears all of them but a rotated CMYK JPEG over 47MP, which no camera produces. const ( - contactPhotoMaxSide = 16_000 - contactPhotoPixelCacheBytes = 896 << 20 - contactPhotoBytesPerPixel = 8 - contactPhotoCMYKBytesPerPixel = 10 + contactPhotoMaxPixels = 58_000_000 + contactPhotoMaxSide = 16_000 ) // contactPhotoNotEditableHint is what a not_found from the photo routes usually means: @@ -74,7 +64,7 @@ func newContactPhotoCommand() *contactPhotoCommand { Aliases: []string{"avatar"}, Short: "Set or remove a contact's photo", Annotations: map[string]string{ - "agent_notes": "Set takes a contact ID and a local PNG or JPEG file of at most 40 MiB and 16000 pixels a side, and of at most about 117 megapixels (58 when its EXIF orientation rotates or flips it, 94 or 47 for a CMYK JPEG); remove takes a contact ID. The photo is the contact's avatar_url. Hidden contacts, aliases and other HEY users answer not_found.", + "agent_notes": "Set takes a contact ID and a local PNG or JPEG file of at most 40 MiB, 58 megapixels and 16000 pixels a side; remove takes a contact ID. The photo is the contact's avatar_url. Hidden contacts, aliases and other HEY users answer not_found.", }, } photoCommand.cmd.AddCommand(newContactPhotoSetCommand().cmd) @@ -94,8 +84,7 @@ func newContactPhotoSetCommand() *contactPhotoSetCommand { Use: "set ", Short: "Upload a photo for a contact", Long: "Upload a local image as a contact's photo, replacing any photo they already have. " + - "The file must be a PNG or JPEG of at most 40 MiB and 16000 pixels a side, judged by its contents rather than its name. " + - "It must also fit the memory HEY resizes a photo in: about 117 megapixels, 58 for a JPEG its EXIF orientation rotates or flips, and 94 or 47 for a CMYK JPEG. " + + "The file must be a PNG or JPEG of at most 40 MiB, 58 megapixels and 16000 pixels a side, judged by its contents rather than its name; a full-resolution photo from a modern phone fits. " + "HEY crops it to a square, so a square image without transparency works best. " + "Only a contact you can edit, or your own, takes a photo: a hidden contact, an alias or another HEY user answers not_found. " + "HEY caches avatars, so the old photo may still show in places for up to 30 minutes.", @@ -266,27 +255,15 @@ func checkContactPhoto(file *os.File, path string) (preparedAttachment, error) { if err != nil || decoded != format || config.Width <= 0 || config.Height <= 0 { return preparedAttachment{}, apierr.ErrUsage(fmt.Sprintf("photo %q is not a readable %s image", path, format)) } - if config.Width > contactPhotoMaxSide || config.Height > contactPhotoMaxSide { + if pixels := int64(config.Width) * int64(config.Height); pixels > contactPhotoMaxPixels || + config.Width > contactPhotoMaxSide || config.Height > contactPhotoMaxSide { return preparedAttachment{}, apierr.ErrUsageHint( - fmt.Sprintf("photo %q is %d×%d pixels, too large for HEY to resize", path, config.Width, config.Height), - fmt.Sprintf("Scale it down to at most %d pixels a side; HEY shows a contact photo at 300×300.", contactPhotoMaxSide)) + fmt.Sprintf("photo %q is %d×%d pixels, larger than a contact photo can be", path, config.Width, config.Height), + "Scale it down to at most 58 megapixels and 16000 pixels a side; HEY shows a contact photo at 300×300.") } if _, err = file.Seek(0, io.SeekStart); err != nil { return preparedAttachment{}, fmt.Errorf("rewind photo %q: %w", path, err) } - budget := contactPhotoBudget{cmyk: config.ColorModel == color.CMYKModel} - if format == "jpeg" { - budget.reoriented = jpegOrientation(file) > 1 - if _, err = file.Seek(0, io.SeekStart); err != nil { - return preparedAttachment{}, fmt.Errorf("rewind photo %q: %w", path, err) - } - } - if pixels := int64(config.Width) * int64(config.Height); pixels > budget.maxPixels() { - return preparedAttachment{}, apierr.ErrUsageHint( - fmt.Sprintf("photo %q is %d×%d pixels, too large for HEY to resize", path, config.Width, config.Height), - fmt.Sprintf("HEY can resize %s of at most %.1f megapixels; scale it down first. HEY shows a contact photo at 300×300.", - budget.describe(), float64(budget.maxPixels())/1e6)) - } return preparedAttachment{ path: path, @@ -296,109 +273,3 @@ func checkContactPhoto(file *os.File, path string) (preparedAttachment, error) { file: file, }, nil } - -// contactPhotoBudget is what one photo costs HEY's ImageMagick per pixel. -type contactPhotoBudget struct { - cmyk bool - reoriented bool -} - -func (b contactPhotoBudget) maxPixels() int64 { - perPixel := int64(contactPhotoBytesPerPixel) - if b.cmyk { - perPixel = contactPhotoCMYKBytesPerPixel - } - if b.reoriented { - perPixel *= 2 - } - return contactPhotoPixelCacheBytes / perPixel -} - -func (b contactPhotoBudget) describe() string { - kind := "a photo" - if b.cmyk { - kind = "a CMYK JPEG" - } - if b.reoriented { - kind += " that its EXIF orientation rotates or flips" - } - return kind -} - -// jpegOrientation is the EXIF orientation of the JPEG r starts, 1 (upright) when it has -// none or it cannot be read — as it is for ImageMagick, which then leaves the image alone. -// It reads the markers before the scan and only an Exif APP1 segment's IFD0. -func jpegOrientation(r io.Reader) int { - br := bufio.NewReader(r) - var soi [2]byte - if _, err := io.ReadFull(br, soi[:]); err != nil || soi != [2]byte{0xFF, 0xD8} { - return 1 - } - for { - // A marker is 0xFF, any number of 0xFF fill bytes, then its code. - b, err := br.ReadByte() - if err != nil || b != 0xFF { - return 1 - } - for b == 0xFF { - if b, err = br.ReadByte(); err != nil { - return 1 - } - } - switch { - case b == 0xDA || b == 0xD9: // start of scan, end of image - return 1 - case b == 0x01 || (b >= 0xD0 && b <= 0xD7): // markers without a length - continue - } - var length [2]byte - if _, err := io.ReadFull(br, length[:]); err != nil { - return 1 - } - size := int(binary.BigEndian.Uint16(length[:])) - 2 - if size < 0 { - return 1 - } - segment := make([]byte, size) - if _, err := io.ReadFull(br, segment); err != nil { - return 1 - } - if b == 0xE1 && len(segment) >= 6 && string(segment[:6]) == "Exif\x00\x00" { - return exifOrientation(segment[6:]) - } - } -} - -// exifOrientation reads tag 0x0112 from the IFD0 of a TIFF-structured EXIF block. -func exifOrientation(tiff []byte) int { - if len(tiff) < 8 { - return 1 - } - var order binary.ByteOrder - switch string(tiff[:2]) { - case "II": - order = binary.LittleEndian - case "MM": - order = binary.BigEndian - default: - return 1 - } - ifd := int64(order.Uint32(tiff[4:8])) - if ifd < 8 || ifd+2 > int64(len(tiff)) { - return 1 - } - entries := int64(order.Uint16(tiff[ifd:])) - for i := int64(0); i < entries; i++ { - entry := ifd + 2 + i*12 - if entry+12 > int64(len(tiff)) { - return 1 - } - if order.Uint16(tiff[entry:]) == 0x0112 && order.Uint16(tiff[entry+2:]) == 3 { // SHORT - if orientation := int(order.Uint16(tiff[entry+8:])); orientation >= 1 && orientation <= 8 { - return orientation - } - return 1 - } - } - return 1 -} diff --git a/internal/cmd/contact_photo_test.go b/internal/cmd/contact_photo_test.go index 661ab0e64..8a34ca2e3 100644 --- a/internal/cmd/contact_photo_test.go +++ b/internal/cmd/contact_photo_test.go @@ -251,7 +251,7 @@ func TestContactPhotoSetRefusesBadFilesBeforeAnyRequest(t *testing.T) { } } -func TestContactPhotoSetAcceptsAPhotoAtTheSizeLimit(t *testing.T) { +func TestContactPhotoSetAcceptsAPhotoAtTheByteLimit(t *testing.T) { photo := writePhotoFixture(t, "jane-doe.png", pngPhoto(t)) // A PNG decoder stops at IEND, so padding the file out keeps it a valid photo. if err := os.Truncate(photo, contactPhotoMaxBytes); err != nil { @@ -280,116 +280,27 @@ func pngHeader(width, height uint32) []byte { return binary.BigEndian.AppendUint32(header, crc32.ChecksumIEEE(chunk)) } -type exifByteOrder interface { - binary.ByteOrder - binary.AppendByteOrder -} - -// jpegHeader is the start of a JPEG claiming the given dimensions and component count -// (1 gray, 3 color, 4 CMYK), with an EXIF orientation when orientation is non-zero, in the -// byte order named: everything a decoder reads before the scan, and no pixels. -func jpegHeader(width, height uint16, components byte, orientation uint16, order exifByteOrder) []byte { - header := []byte{0xFF, 0xD8} - if orientation != 0 { - tiff := []byte("MM") - if order == binary.LittleEndian { - tiff = []byte("II") - } - tiff = order.AppendUint16(tiff, 42) - tiff = order.AppendUint32(tiff, 8) // IFD0 offset - tiff = order.AppendUint16(tiff, 2) // entries - tiff = order.AppendUint16(tiff, 0x010F) // Make, which is not the orientation - tiff = order.AppendUint16(tiff, 2) - tiff = order.AppendUint32(tiff, 4) - tiff = append(tiff, "HEY\x00"...) - tiff = order.AppendUint16(tiff, 0x0112) - tiff = order.AppendUint16(tiff, 3) // SHORT - tiff = order.AppendUint32(tiff, 1) - tiff = order.AppendUint16(tiff, orientation) - tiff = order.AppendUint16(tiff, 0) - tiff = order.AppendUint32(tiff, 0) // no next IFD - app1 := append([]byte("Exif\x00\x00"), tiff...) - header = append(header, 0xFF, 0xE1) - header = binary.BigEndian.AppendUint16(header, uint16(len(app1)+2)) - header = append(header, app1...) - } - sof := []byte{8} - sof = binary.BigEndian.AppendUint16(sof, height) - sof = binary.BigEndian.AppendUint16(sof, width) - sof = append(sof, components) - for id := byte(1); id <= components; id++ { - sof = append(sof, id, 0x11, 0) - } - header = append(header, 0xFF, 0xC0) - header = binary.BigEndian.AppendUint16(header, uint16(len(sof)+2)) - header = append(header, sof...) +// jpegHeader is the start of a JPEG claiming the given dimensions: everything a decoder +// reads before the scan, and no pixels. +func jpegHeader(width, height uint16) []byte { + header := []byte{0xFF, 0xD8, 0xFF, 0xC0, 0x00, 0x11, 8} + header = binary.BigEndian.AppendUint16(header, height) + header = binary.BigEndian.AppendUint16(header, width) + header = append(header, 3, 1, 0x11, 0, 2, 0x11, 0, 3, 0x11, 0) return append(header, 0xFF, 0xDA, 0x00, 0x08, 0x01, 0x01, 0x00, 0x00, 0x3F, 0x00) } -func TestJPEGOrientationReadsTheExifTag(t *testing.T) { - for _, order := range []exifByteOrder{binary.BigEndian, binary.LittleEndian} { - for orientation := uint16(1); orientation <= 8; orientation++ { - got := jpegOrientation(bytes.NewReader(jpegHeader(640, 480, 3, orientation, order))) - if got != int(orientation) { - t.Errorf("%v orientation %d read as %d", order, orientation, got) - } - } - } - upright := jpegHeader(640, 480, 3, 6, binary.BigEndian) - for name, content := range map[string][]byte{ - "no exif": jpegHeader(640, 480, 3, 0, nil), - "out of range": jpegHeader(640, 480, 3, 9, binary.BigEndian), - "a real encoder's": jpegPhoto(t), - "not a jpeg": pngPhoto(t), - "cut off in the exif": upright[:30], - "empty": nil, - } { - if got := jpegOrientation(bytes.NewReader(content)); got != 1 { - t.Errorf("%s: orientation = %d, want 1", name, got) - } - } -} - -func TestJPEGOrientationSkipsFillBytesBeforeAMarker(t *testing.T) { - rotated := jpegHeader(640, 480, 3, 6, binary.BigEndian) - for fill := 1; fill <= 3; fill++ { - padded := append([]byte{0xFF, 0xD8}, bytes.Repeat([]byte{0xFF}, fill)...) - padded = append(padded, rotated[2:]...) - if got := jpegOrientation(bytes.NewReader(padded)); got != 6 { - t.Errorf("%d fill bytes: orientation = %d, want 6", fill, got) - } - } -} - -func TestJPEGOrientationSurvivesEveryTruncation(t *testing.T) { - rotated := jpegHeader(640, 480, 3, 6, binary.LittleEndian) - for cut := range rotated { - _ = jpegOrientation(bytes.NewReader(rotated[:cut])) - } -} - -// The boundaries below are HEY's, measured by running its variant command under -// hotcell/policy.xml and the cell's limits: 10800×10800 resizes and 11000×11000 does not; -// rotated, 7600×7600 does and 7700×7700 does not; CMYK, 9600×9600 does and 9800×9600 does -// not; rotated CMYK, 6000×6000 does and 7000×7000 does not. Each case sits on the line the -// budget draws between them. -func TestContactPhotoSetRefusesPhotosTooLargeForHEYToResize(t *testing.T) { +func TestContactPhotoSetRefusesPhotosOverTheSizeLimit(t *testing.T) { tests := []struct { name string content []byte - hint string }{ - {name: "too wide", content: pngHeader(contactPhotoMaxSide+1, 1), hint: "16000 pixels a side"}, - {name: "too tall", content: jpegHeader(1, contactPhotoMaxSide+1, 3, 0, nil), hint: "16000 pixels a side"}, - {name: "a 200 megapixel phone photo", content: jpegHeader(16_320, 12_240, 3, 0, nil), hint: "16000 pixels a side"}, - {name: "upright png", content: pngHeader(16_000, 7_341), hint: "a photo of at most 117.4 megapixels"}, - {name: "upright jpeg", content: jpegHeader(16_000, 7_341, 3, 1, binary.BigEndian), hint: "a photo of at most 117.4 megapixels"}, - {name: "grayscale jpeg", content: jpegHeader(16_000, 7_341, 1, 0, nil), hint: "a photo of at most 117.4 megapixels"}, - {name: "a 108 megapixel portrait", content: jpegHeader(12_000, 9_000, 3, 6, binary.LittleEndian), hint: "a photo that its EXIF orientation rotates or flips of at most 58.7 megapixels"}, - {name: "rotated jpeg", content: jpegHeader(8_000, 7_341, 3, 8, binary.BigEndian), hint: "rotates or flips of at most 58.7 megapixels"}, - {name: "mirrored jpeg", content: jpegHeader(8_000, 7_341, 3, 2, binary.BigEndian), hint: "rotates or flips of at most 58.7 megapixels"}, - {name: "cmyk jpeg", content: jpegHeader(16_000, 5_873, 4, 0, nil), hint: "a CMYK JPEG of at most 94.0 megapixels"}, - {name: "rotated cmyk jpeg", content: jpegHeader(8_000, 5_873, 4, 6, binary.BigEndian), hint: "a CMYK JPEG that its EXIF orientation rotates or flips of at most 47.0 megapixels"}, + {name: "one pixel row over 58 megapixels", content: pngHeader(10_000, 5_801)}, + {name: "a 64 megapixel phone photo", content: jpegHeader(9_248, 6_936)}, + {name: "a 108 megapixel phone photo", content: jpegHeader(12_000, 9_000)}, + {name: "a 200 megapixel phone photo", content: jpegHeader(16_320, 12_240)}, + {name: "too wide", content: pngHeader(contactPhotoMaxSide+1, 1)}, + {name: "too tall", content: jpegHeader(1, contactPhotoMaxSide+1)}, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { @@ -400,8 +311,8 @@ func TestContactPhotoSetRefusesPhotosTooLargeForHEYToResize(t *testing.T) { if !errors.As(err, &cliErr) || cliErr.Code != apierr.CodeUsage { t.Fatalf("error = %v, want usage error", err) } - if !strings.Contains(cliErr.Message, "too large for HEY to resize") || !strings.Contains(cliErr.Hint, tc.hint) { - t.Errorf("message = %q, hint = %q, want hint containing %q", cliErr.Message, cliErr.Hint, tc.hint) + if !strings.Contains(cliErr.Message, "larger than a contact photo can be") || !strings.Contains(cliErr.Hint, "58 megapixels and 16000 pixels a side") { + t.Errorf("message = %q, hint = %q", cliErr.Message, cliErr.Hint) } if requests := state.requestLines(); len(requests) != 0 { t.Fatalf("a refused photo sent requests: %v", requests) @@ -410,17 +321,14 @@ func TestContactPhotoSetRefusesPhotosTooLargeForHEYToResize(t *testing.T) { } } -func TestContactPhotoSetAcceptsPhotosAtTheResizeLimits(t *testing.T) { +func TestContactPhotoSetAcceptsPhotosAtTheSizeLimit(t *testing.T) { for name, content := range map[string][]byte{ - "widest png": pngHeader(contactPhotoMaxSide, 1), - "tallest jpeg": jpegHeader(1, contactPhotoMaxSide, 3, 0, nil), - "largest upright png": pngHeader(16_000, 7_340), - "largest upright jpeg": jpegHeader(16_000, 7_340, 3, 1, binary.LittleEndian), - "png whatever its exif": pngHeader(16_000, 7_340), - "largest rotated jpeg": jpegHeader(8_000, 7_340, 3, 6, binary.BigEndian), - "largest cmyk jpeg": jpegHeader(16_000, 5_872, 4, 0, nil), - "largest rotated cmyk": jpegHeader(8_000, 5_872, 4, 5, binary.LittleEndian), - "a 48 megapixel portrait": jpegHeader(8_064, 6_048, 3, 6, binary.LittleEndian), + "exactly 58 megapixels": pngHeader(10_000, 5_800), + "an iPhone's 48 megapixels": jpegHeader(8_064, 6_048), + "an iPhone photo, portrait": jpegHeader(6_048, 8_064), + "a 50 megapixel Android one": jpegHeader(8_160, 6_120), + "the widest panorama": jpegHeader(contactPhotoMaxSide, 3_000), + "the tallest strip": pngHeader(1, contactPhotoMaxSide), } { photo := writePhotoFixture(t, "jane-doe.jpg", content) prepared, err := openContactPhoto(photo) diff --git a/skills/hey/SKILL.md b/skills/hey/SKILL.md index 9351a56e2..6d54932cc 100644 --- a/skills/hey/SKILL.md +++ b/skills/hey/SKILL.md @@ -212,7 +212,7 @@ postings, not the box, label or contact around them. | Read private contact note | `hey contact note show --json` (`note_markdown` is the form `note set` takes when `note_markdown_lossless` is true) | | Replace private contact note | `hey contact note set "Prefers email"` (replaces the whole note) | | Delete private contact note | `hey contact note delete ` | -| Set a contact's photo | `hey contact photo set ` (PNG or JPEG, at most 40 MiB, 16000 px a side and ~117 megapixels; ~58 if EXIF-rotated) | +| Set a contact's photo | `hey contact photo set ` (PNG or JPEG, at most 40 MiB, 58 megapixels and 16000 px a side) | | Remove a contact's photo | `hey contact photo remove ` | | Read email thread | `hey thread read --json` | | Rename a thread | `hey thread update --name "Kitchen renovation quotes"` | From da5aa9b90155c15099fb078427e6ce2e1a2d2410 Mon Sep 17 00:00:00 2001 From: Rob Zolkos Date: Mon, 12 Oct 2026 16:49:14 -0400 Subject: [PATCH 5/6] Address review: a FIFO no longer hangs, and the smoke test fetches the photo - openContactPhoto stats the path before opening it, as prepareAttachments does: opening a named pipe blocks until something writes to it, so a FIFO hung instead of being refused. A Unix test proves it returns. - The smoke test fetches the avatar after setting it and requires a raster image. avatar_url changes with the blob whether or not HEY can resize it, and AvatarsController answers a photo it cannot resize with the contact's initials as SVG, so the URL alone proved nothing. Its cleanup also removes the photo before hiding the contact. - The upload-failure test checks that the failing request was reached. - A photo HEY cannot resize is never shown, and the contact keeps their initials; it is not a broken avatar. The comments and docs say so, the docs name the one case past the 58MP cap's cover (a rotated CMYK JPEG over 47MP), and the format hint says PNG or JPEG is this command's rule rather than claiming HEY can show nothing else. - photo remove mentions the 30-minute avatar cache, as set does. - API-COVERAGE.md lists the avatar endpoints and the new callers. --- API-COVERAGE.md | 8 ++++--- docs/cli.md | 2 +- internal/cmd/contact_photo.go | 22 +++++++++++++----- internal/cmd/contact_photo_test.go | 14 ++++++++++- internal/cmd/contact_photo_unix_test.go | 31 +++++++++++++++++++++++++ tests/smoke/contacts_test.go | 18 +++++++++++++- 6 files changed, 83 insertions(+), 12 deletions(-) create mode 100644 internal/cmd/contact_photo_unix_test.go diff --git a/API-COVERAGE.md b/API-COVERAGE.md index a5ea8ca02..166f3d4ad 100644 --- a/API-COVERAGE.md +++ b/API-COVERAGE.md @@ -44,7 +44,7 @@ forward answers. The drafts index (`/entries/drafts.json`) pages the same way, w | `/my/clearances.json` | GET | SDK `Clearances().Screened`, `ScreenedPage` | `hey screener history`, TUI Screener History | covered | | `/my/clearances/{id}` | PATCH | SDK `Clearances().Rescreen` | — | unused by the CLI | | `/contacts.json` | GET | SDK `Contacts().List` | `hey contact list`, Contacts TUI | covered | -| `/contacts/{id}.json` | GET | SDK `Contacts().Get` | `hey contact show`, `hey contact deliver --to screened-out` (external-contact check), Contacts TUI | covered | +| `/contacts/{id}.json` | GET | SDK `Contacts().Get` | `hey contact show`, `hey contact deliver --to screened-out` (external-contact check), `hey contact photo set` (existence check before uploading), Contacts TUI | covered | | `/contacts.json` | POST | SDK `Contacts().Create` | `hey contact add`, Contacts TUI | covered | | `/contacts/{id}.json` | PATCH | SDK `Contacts().Update` | `hey contact update`, Contacts TUI | covered | | `/contacts/{id}.json` | DELETE | SDK `Contacts().Hide` | `hey contact hide`, Contacts TUI | covered | @@ -56,6 +56,8 @@ forward answers. The drafts index (`/entries/drafts.json`) pages the same way, w | `/contacts/{id}/note.json` | GET | SDK `Contacts().Note` | `hey contact note show`, Contacts TUI | covered | | `/contacts/{id}/note.json` | PATCH | SDK `Contacts().SetNote` | `hey contact note set`, Contacts TUI | covered | | `/contacts/{id}/note.json` | DELETE | SDK `Contacts().DeleteNote` | `hey contact note delete`, Contacts TUI | covered | +| `/contacts/{id}/uploaded_avatar.json` | PUT | SDK `Contacts().UpdateAvatar` | `hey contact photo set` | covered | +| `/contacts/{id}/uploaded_avatar.json` | DELETE | SDK `Contacts().DeleteAvatar` | `hey contact photo remove` | covered | | `/autocompletable/contacts/addressable.json` | GET | SDK `Contacts().Addressable` | TUI composer's To, Cc and Bcc suggestions (new message, reply, forward) | covered | | `/calendars.json` | GET | SDK `Calendars().List` | `hey calendar list` | covered | | `/calendars/{id}/recordings.json` | GET | SDK `Calendars().GetRecordings` | `hey event list`, `hey event edit ` (reading the event back; with `--occurrence`, the occurrence's day and then the series' first day for its `Calendar::Countdown`), `hey event delete ` (checking the id is not a day of a series HEY has written out; with `--occurrence --apply-to future`, the occurrence's day), `hey todo list`, `hey journal list` | covered | @@ -69,8 +71,8 @@ forward answers. The drafts index (`/entries/drafts.json`) pages the same way, w | `/entries/{id}/replies/new.json` | GET | SDK `Entries().NewReply` | `hey reply ` and `hey compose --thread-id` (recipients, with the acting user excluded) | covered | | `/entries/drafts.json` | GET | SDK `Entries().ListDraftsPage` | `hey draft list` (`--all`/`--page` follow its cursor) | covered | | `/entries/drafts/{id}` | DELETE | SDK `Entries().DeleteDraft` | `hey draft delete ...` | covered | -| `/rails/active_storage/direct_uploads.json` | POST | SDK `Attachments().Upload` | `hey compose --attach`, `hey reply --attach`, `hey bulk-reply send --attach` | covered | -| signed Active Storage upload URL | PUT | SDK `Attachments().Upload` | `hey compose --attach`, `hey reply --attach`, `hey bulk-reply send --attach` | covered | +| `/rails/active_storage/direct_uploads.json` | POST | SDK `Attachments().Upload` | `hey compose --attach`, `hey reply --attach`, `hey bulk-reply send --attach`, `hey contact photo set` | covered | +| signed Active Storage upload URL | PUT | SDK `Attachments().Upload` | `hey compose --attach`, `hey reply --attach`, `hey bulk-reply send --attach`, `hey contact photo set` | covered | | signed Active Storage blob URL | GET | SDK `DownloadBlob` | `hey attachment save ` | covered | | `/messages.json` | POST | SDK `Messages().Create`, `Messages().CreateDraft` | `hey compose`, `hey compose --draft`, `hey forward ` | covered | | `/entries/{id}/replies` | POST | SDK `Entries().CreateReply`, `Entries().CreateReplyDraft` | `hey reply `, `hey reply --draft`, `hey compose --thread-id [--draft]` | covered | diff --git a/docs/cli.md b/docs/cli.md index 2404b2644..1fe355414 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -288,7 +288,7 @@ Drafts are the review-before-send lane: `hey compose --draft` (and `hey reply -- Search accepts free text plus `--required`, `--any`, `--none`, `--exact`, `--from`, `--to`, `--subject`, `--date`, `--in`, `--label`, and `--attachment`. `--in`, `--date`, `--label` and `--attachment` take one of the values `hey search filters` lists — the attachment kinds are `any`, `images`, `pdfs`, `calendar_invites`, `documents`, `spreadsheets`, `presentations`, `media` and `zip_files`, so it is `--attachment pdfs` rather than `pdf`, and an unrecognized `--in`, `--date` or `--attachment` is refused as a usage error, with the values it accepts, before any search is sent. `--in` also takes a box's kind or display name (`feedbox`, `"Paper Trail"`), the spellings `hey box view` takes, including a name you gave one of those boxes in HEY; a name it does not recognize costs one read of the box list to find out which box it is. Search can narrow only to the Imbox, The Feed, the Paper Trail and Trash: any other box, and a name no box has, is a usage error rather than `not_found`. Use `--page` for one page or `--all` to fetch up to 100 pages; capped searches report the next page for continuation. Search results include `topic_id` for reading the thread and the matching message summaries. Results also include `id` for organization actions, except when you have no box item for the thread. -Contact updates preserve omitted name, email, and alias fields. Supplying `--alias` replaces the complete alias list; `--alias=` clears it. Contact notes accept positional content, `--note`, stdin, or `$EDITOR`. `hey contact note set` replaces the whole note — HEY has no append — so adding a detail means reading the note, changing it and writing all of it back. `hey contact note show --json` (and `hey contact show --json`) answers the note three ways, each an empty string for a contact with no note: `note`, HEY's plain text, which drops formatting (bold is lost, list items become `•`); `note_html`, as HEY serves it; and `note_markdown`, the note as Markdown. `note_markdown_lossless` says whether that Markdown holds everything in the note. When it is `true`, `note set` writes the Markdown back as the same note — HEY's editor markup becomes plain paragraphs on the first write, and from then on a read and a write give the same Markdown and the same HTML. When it is `false`, the note holds an attachment, an image, a table or other markup Markdown has no syntax for, and setting Markdown would drop it: change `note_html` and write it back with `--note-html` instead. HTML passed to `--note-html` has the `
` HEY serves a note in taken off, so writing back `note_html` does not nest the note one level deeper each time. Nothing guards a read and a write against someone else saving in between: the last save wins, as it does in the web app. HEY hides contacts rather than permanently deleting them; hidden contacts leave lists, autocomplete, and search, and can be shown again by ID. `hey contact deliver --to imbox|feed|papertrail|screened-out` changes where a contact's future email arrives. It takes a contact ID — not an email address, clearance ID, box item ID, or box ID — and is write-only because HEY does not expose the current setting as structured data. Imbox removes a custom box preference; choosing a box does not screen a denied contact back in. Screened Out is the blocking choice and applies only to external email contacts. Selecting The Feed removes an existing bundle because HEY cannot keep Feed-designated contacts bundled; Imbox and Paper Trail preserve eligible bundles. Existing mail can move asynchronously, so success means HEY accepted the change, not that every affected thread has already moved. Bundling groups a contact's mail into one row without merging or deleting the underlying threads; unbundling lists those threads separately again. HEY bundles only a contact with no box preference or one sent to the Paper Trail; for any other, bundling answers success and changes nothing. `hey contact photo set ` uploads a photo for a contact, replacing any it had, and answers the contact with its new `avatar_url`; `hey contact photo remove` takes it off again, and removing from a contact with no photo succeeds. The file must be a PNG or JPEG — the formats HEY's own photo picker takes — of at most 40 MiB, the most HEY's web form can send, and at most 58 megapixels and 16000 pixels a side. That admits a full-resolution photo from a modern phone (an iPhone's 48 megapixels, or 50 on Android) and stays inside what HEY can resize, which for a JPEG it has to rotate upright is about 58.7 megapixels; HEY shows a contact photo at 300×300 at most anyway. It is judged by its bytes rather than its name, and its header must decode, because HEY stores any file it is handed and fails only later, when it resizes the photo to show it: a HEIC from a phone, a photo too large to resize, or a file that is not an image would leave the contact with a broken avatar. HEY crops the photo to a square, so a square image without transparency works best, and it caches avatars, so the old photo can still show in places for up to 30 minutes. Your own contact takes a photo too. A contact that is hidden, an alias, or another HEY user cannot be edited: updating it, setting or deleting its note, or setting or removing its photo answers `not_found`. `hey contact list` holds only non-HEY senders you have screened in — other HEY users, senders still in or denied by The Screener, aliases and hidden contacts are left out. It has no search: HEY ignores a query on that list, so finding a contact by email means reading every page (`--all`) and filtering, which matches primary addresses only. +Contact updates preserve omitted name, email, and alias fields. Supplying `--alias` replaces the complete alias list; `--alias=` clears it. Contact notes accept positional content, `--note`, stdin, or `$EDITOR`. `hey contact note set` replaces the whole note — HEY has no append — so adding a detail means reading the note, changing it and writing all of it back. `hey contact note show --json` (and `hey contact show --json`) answers the note three ways, each an empty string for a contact with no note: `note`, HEY's plain text, which drops formatting (bold is lost, list items become `•`); `note_html`, as HEY serves it; and `note_markdown`, the note as Markdown. `note_markdown_lossless` says whether that Markdown holds everything in the note. When it is `true`, `note set` writes the Markdown back as the same note — HEY's editor markup becomes plain paragraphs on the first write, and from then on a read and a write give the same Markdown and the same HTML. When it is `false`, the note holds an attachment, an image, a table or other markup Markdown has no syntax for, and setting Markdown would drop it: change `note_html` and write it back with `--note-html` instead. HTML passed to `--note-html` has the `
` HEY serves a note in taken off, so writing back `note_html` does not nest the note one level deeper each time. Nothing guards a read and a write against someone else saving in between: the last save wins, as it does in the web app. HEY hides contacts rather than permanently deleting them; hidden contacts leave lists, autocomplete, and search, and can be shown again by ID. `hey contact deliver --to imbox|feed|papertrail|screened-out` changes where a contact's future email arrives. It takes a contact ID — not an email address, clearance ID, box item ID, or box ID — and is write-only because HEY does not expose the current setting as structured data. Imbox removes a custom box preference; choosing a box does not screen a denied contact back in. Screened Out is the blocking choice and applies only to external email contacts. Selecting The Feed removes an existing bundle because HEY cannot keep Feed-designated contacts bundled; Imbox and Paper Trail preserve eligible bundles. Existing mail can move asynchronously, so success means HEY accepted the change, not that every affected thread has already moved. Bundling groups a contact's mail into one row without merging or deleting the underlying threads; unbundling lists those threads separately again. HEY bundles only a contact with no box preference or one sent to the Paper Trail; for any other, bundling answers success and changes nothing. `hey contact photo set ` uploads a photo for a contact, replacing any it had, and answers the contact with its new `avatar_url`; `hey contact photo remove` takes it off again, and removing from a contact with no photo succeeds. The file must be a PNG or JPEG — the formats HEY's own photo picker takes — of at most 40 MiB, the most HEY's web form can send, and at most 58 megapixels and 16000 pixels a side. That admits a full-resolution photo from a modern phone (an iPhone's 48 megapixels, or 50 on Android) and stays inside what HEY can resize, which for a JPEG it has to rotate upright is about 58.7 megapixels; the one exception is a CMYK JPEG that has to be rotated, which HEY can resize only up to about 47 megapixels, and which cameras do not produce. HEY shows a contact photo at 300×300 at most anyway. It is judged by its bytes rather than its name, and its header must decode, because HEY stores any file it is handed and fails only later, when it resizes the photo to show it: a HEIC from a phone, a photo too large to resize, or a file that is not an image would be saved and never shown, the contact keeping their initials. HEY crops the photo to a square, so a square image without transparency works best, and it caches avatars, so after a set or a remove the old photo can still show in places for up to 30 minutes. Your own contact takes a photo too. A contact that is hidden, an alias, or another HEY user cannot be edited: updating it, setting or deleting its note, or setting or removing its photo answers `not_found`. `hey contact list` holds only non-HEY senders you have screened in — other HEY users, senders still in or denied by The Screener, aliases and hidden contacts are left out. It has no search: HEY ignores a query on that list, so finding a contact by email means reading every page (`--all`) and filtering, which matches primary addresses only. The Screener is where first-time senders wait. `hey screener list` returns clearance IDs — not contact IDs — with the sender and the subject of what they sent, plus `topic_id` for reading the thread before deciding. `--count` asks for the number alone, which is a far cheaper request than the queue, and prints it as a bare number like every other command's `--count`, so `n=$(hey screener list --count)` reads it directly. Approving delivers everything the sender has waiting; denying hides it. Either is reversible with the opposite command, and `hey screener history` shows what was already decided. Both listings page the way `hey box view` does: `--all` follows HEY's cursor to the end of the queue (up to 100 pages), and a single-page or capped read reports `next_page` in its JSON meta, which `--page ` continues from — the cursor is opaque, so a page *number* does not name a position. `hey screener list` also reports `total_count`, the whole queue's size, next to what the read returned. `--box` and `--seen` approve one sender at a time; several IDs go through HEY's bulk endpoint, which takes neither. `--seen` reliably marks the mail read only for a sender with one waiting thread. `--spam` marks what they sent as spam and, for a single ID, trains HEY's filter, which is harder to undo than denying. `hey screener clear` moves everything waiting to Trash (a shared thread loses your access instead) without approving or denying anyone — those senders reappear on their next email. diff --git a/internal/cmd/contact_photo.go b/internal/cmd/contact_photo.go index 3b8c1416f..f731e0f2d 100644 --- a/internal/cmd/contact_photo.go +++ b/internal/cmd/contact_photo.go @@ -27,8 +27,9 @@ const contactPhotoMaxBytes = 40 << 20 // contactPhotoFormats are the formats HEY's photo picker accepts, keyed by the content // type sniffed from the bytes and naming the decoder that must agree. HEY stores whatever // blob it is handed and only fails later, when it resizes the photo to show it: a HEIC or -// an AVIF, which HEY cannot decode, or a file that is not an image at all, then breaks the -// contact's avatar everywhere it is drawn. So the bytes are judged, not the extension. +// an AVIF, which HEY cannot decode, or a file that is not an image at all, is then never +// shown, and the contact keeps wearing their initials. So the bytes are judged, not the +// extension. var contactPhotoFormats = map[string]string{ "image/png": "png", "image/jpeg": "jpeg", @@ -39,7 +40,8 @@ var contactPhotoFormats = map[string]string{ // full-resolution photo from a modern phone — an iPhone's 48MP is 8064×6048, and 50MP // Android cameras are common — and stays inside what HEY can resize. HEY stores whatever it // is handed and only then resizes it, in ImageMagick under hotcell/policy.xml and the -// cell's 896MiB memory limit; a photo past those is never drawn. Measured against HEY's own +// cell's 896MiB memory limit; a photo past those is never drawn, and AvatarsController +// serves the contact's initials instead. Measured against HEY's own // variant command, the tightest ceiling is a JPEG whose EXIF orientation has to be applied, // which holds a second copy: 7600×7600 resizes and 7700×7700 kills the worker, about 58.7MP. // An upright photo goes to about 117MP, and policy.xml refuses any side over 16000 pixels. @@ -83,7 +85,7 @@ func newContactPhotoSetCommand() *contactPhotoSetCommand { setCommand.cmd = &cobra.Command{ Use: "set ", Short: "Upload a photo for a contact", - Long: "Upload a local image as a contact's photo, replacing any photo they already have. " + + Long: "Upload a local image as a contact's photo, replacing any photo they already have, and answer the contact with its new avatar_url. " + "The file must be a PNG or JPEG of at most 40 MiB, 58 megapixels and 16000 pixels a side, judged by its contents rather than its name; a full-resolution photo from a modern phone fits. " + "HEY crops it to a square, so a square image without transparency works best. " + "Only a contact you can edit, or your own, takes a photo: a hidden contact, an alias or another HEY user answers not_found. " + @@ -186,7 +188,7 @@ func (c *contactPhotoRemoveCommand) run(cmd *cobra.Command, args []string) error return contactPhotoError(err, contactID) } return writeMutationLine(cmd, - fmt.Sprintf("Photo removed from contact %d.", contactID), + fmt.Sprintf("Photo removed from contact %d. The old photo may still show in places for up to 30 minutes.", contactID), "Contact photo removed", map[string]int64{"id": contactID}, output.WithBreadcrumbs(output.Breadcrumb{Action: "set", Command: fmt.Sprintf("hey contact photo set %d ", contactID), Description: "Upload a photo"}), @@ -207,6 +209,14 @@ func contactPhotoError(err error, contactID int64) error { // sent: a regular, non-empty file within contactPhotoMaxBytes whose bytes are a PNG or a // JPEG with a readable header and dimensions HEY can resize. The file is left open at its start. func openContactPhoto(path string) (preparedAttachment, error) { + // Stat first: opening a named pipe blocks until something writes to it. + pathInfo, err := os.Stat(path) + if err != nil { + return preparedAttachment{}, apierr.ErrUsage(fmt.Sprintf("could not open photo %q: %v", path, err)) + } + if !pathInfo.Mode().IsRegular() { + return preparedAttachment{}, apierr.ErrUsage(fmt.Sprintf("photo %q is not a regular file", path)) + } file, err := os.Open(path) // #nosec G304 -- the user explicitly selected this photo if err != nil { return preparedAttachment{}, apierr.ErrUsage(fmt.Sprintf("could not open photo %q: %v", path, err)) @@ -245,7 +255,7 @@ func checkContactPhoto(file *os.File, path string) (preparedAttachment, error) { if !ok { return preparedAttachment{}, apierr.ErrUsageHint( fmt.Sprintf("photo %q is %s, not a PNG or JPEG", path, contentType), - "Convert it to PNG or JPEG first; HEY cannot show other formats as a contact photo.") + "This command takes only PNG or JPEG, the formats HEY's own photo picker offers; convert the file first.") } if _, err = file.Seek(0, io.SeekStart); err != nil { diff --git a/internal/cmd/contact_photo_test.go b/internal/cmd/contact_photo_test.go index 8a34ca2e3..fc26f49b2 100644 --- a/internal/cmd/contact_photo_test.go +++ b/internal/cmd/contact_photo_test.go @@ -213,6 +213,13 @@ func TestContactPhotoSetRefusesBadFilesBeforeAnyRequest(t *testing.T) { {name: "pdf named png", path: func(t *testing.T) string { return writePhotoFixture(t, "jane-doe.png", []byte("%PDF-1.7\n1 0 obj\n<<>>\nendobj\n")) }, message: "is application/pdf, not a PNG or JPEG"}, + {name: "dangling symlink", path: func(t *testing.T) string { + link := filepath.Join(t.TempDir(), "jane-doe.png") + if err := os.Symlink(filepath.Join(t.TempDir(), "gone.png"), link); err != nil { + t.Fatal(err) + } + return link + }, message: "could not open photo"}, {name: "text named jpg", path: func(t *testing.T) string { return writePhotoFixture(t, "jane-doe.jpg", []byte("Jane Doe, Head of Partnerships")) }, message: "not a PNG or JPEG"}, @@ -429,11 +436,16 @@ func TestContactPhotoSetStopsWhenTheUploadFails(t *testing.T) { if _, err := runContacts(t, state.server, "photo", "set", "7", photo); err == nil { t.Fatalf("%s failing: expected an error", statusKey) } + reached := false for _, line := range state.requestLines() { + reached = reached || line == statusKey if line == "PUT /contacts/7/uploaded_avatar.json" { t.Errorf("%s failing: the avatar was still set", statusKey) } } + if !reached { + t.Errorf("the command stopped before %s, so its failure was never exercised: %v", statusKey, state.requestLines()) + } } } @@ -468,7 +480,7 @@ func TestContactPhotoStyledOutput(t *testing.T) { if err != nil { t.Fatal(err) } - if want := "Photo removed from contact 7.\n"; got != want { + if want := "Photo removed from contact 7. The old photo may still show in places for up to 30 minutes.\n"; got != want { t.Errorf("remove output = %q, want %q", got, want) } } diff --git a/internal/cmd/contact_photo_unix_test.go b/internal/cmd/contact_photo_unix_test.go new file mode 100644 index 000000000..71fab8302 --- /dev/null +++ b/internal/cmd/contact_photo_unix_test.go @@ -0,0 +1,31 @@ +//go:build aix || darwin || dragonfly || freebsd || linux || netbsd || openbsd || solaris + +package cmd + +import ( + "path/filepath" + "testing" + "time" + + "golang.org/x/sys/unix" +) + +func TestContactPhotoRejectsAFIFOWithoutOpeningIt(t *testing.T) { + fifo := filepath.Join(t.TempDir(), "jane-doe.png") + if err := unix.Mkfifo(fifo, 0o600); err != nil { + t.Fatal(err) + } + done := make(chan error, 1) + go func() { + _, err := openContactPhoto(fifo) + done <- err + }() + select { + case err := <-done: + if err == nil { + t.Fatal("a FIFO was accepted as a photo") + } + case <-time.After(5 * time.Second): + t.Fatal("opening a FIFO blocked instead of being refused") + } +} diff --git a/tests/smoke/contacts_test.go b/tests/smoke/contacts_test.go index 08c269d54..42de05af4 100644 --- a/tests/smoke/contacts_test.go +++ b/tests/smoke/contacts_test.go @@ -7,8 +7,10 @@ import ( "image" "image/color" "image/png" + "net/http" "os" "path/filepath" + "strings" "testing" "time" ) @@ -119,7 +121,10 @@ func TestContactPhoto(t *testing.T) { AvatarURL string `json:"avatar_url"` }](t, contactWriteJSON(t, "contact", "add", "--name", "Priya Shah", "--email", "priya.shah+"+unique+"@example.com")) id := intStr(created.ID) - t.Cleanup(func() { _, _, _ = hey(t, "contact", "hide", id) }) + t.Cleanup(func() { + _, _, _ = hey(t, "contact", "photo", "remove", id) + _, _, _ = hey(t, "contact", "hide", id) + }) portrait := image.NewRGBA(image.Rect(0, 0, 64, 64)) for x := 0; x < 64; x++ { @@ -147,6 +152,17 @@ func TestContactPhoto(t *testing.T) { if withPhoto.AvatarURL == "" || withPhoto.AvatarURL == created.AvatarURL { t.Errorf("avatar_url did not change after setting a photo: %q", withPhoto.AvatarURL) } + // The URL changes with the blob whether or not HEY can resize it; one it cannot is + // answered with the contact's initials as SVG. So fetch it: the photo is a raster. + // The avatar route needs no session. + resp, err := http.Get(withPhoto.AvatarURL) + if err != nil { + t.Fatalf("fetch avatar: %v", err) + } + _ = resp.Body.Close() + if contentType := resp.Header.Get("Content-Type"); resp.StatusCode != http.StatusOK || !strings.HasPrefix(contentType, "image/") || strings.Contains(contentType, "svg") { + t.Errorf("avatar answered %d %q, want the uploaded photo as a raster image", resp.StatusCode, contentType) + } contactWriteJSON(t, "contact", "photo", "remove", id) withoutPhoto := dataAs[struct { From 1cd46c47170b95a52a627e8385822c0fbedd2430 Mon Sep 17 00:00:00 2001 From: Rob Zolkos Date: Mon, 12 Oct 2026 16:52:20 -0400 Subject: [PATCH 6/6] Bound the smoke test's avatar fetch, and name remove's argument in the docs The avatar request used http.Get, so a stalled response held the smoke suite until its global timeout; it now uses a 10s client like the other direct requests. docs/cli.md wrote `hey contact photo remove` without the contact ID it requires. --- docs/cli.md | 2 +- tests/smoke/contacts_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/cli.md b/docs/cli.md index 1fe355414..2c06d19d0 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -288,7 +288,7 @@ Drafts are the review-before-send lane: `hey compose --draft` (and `hey reply -- Search accepts free text plus `--required`, `--any`, `--none`, `--exact`, `--from`, `--to`, `--subject`, `--date`, `--in`, `--label`, and `--attachment`. `--in`, `--date`, `--label` and `--attachment` take one of the values `hey search filters` lists — the attachment kinds are `any`, `images`, `pdfs`, `calendar_invites`, `documents`, `spreadsheets`, `presentations`, `media` and `zip_files`, so it is `--attachment pdfs` rather than `pdf`, and an unrecognized `--in`, `--date` or `--attachment` is refused as a usage error, with the values it accepts, before any search is sent. `--in` also takes a box's kind or display name (`feedbox`, `"Paper Trail"`), the spellings `hey box view` takes, including a name you gave one of those boxes in HEY; a name it does not recognize costs one read of the box list to find out which box it is. Search can narrow only to the Imbox, The Feed, the Paper Trail and Trash: any other box, and a name no box has, is a usage error rather than `not_found`. Use `--page` for one page or `--all` to fetch up to 100 pages; capped searches report the next page for continuation. Search results include `topic_id` for reading the thread and the matching message summaries. Results also include `id` for organization actions, except when you have no box item for the thread. -Contact updates preserve omitted name, email, and alias fields. Supplying `--alias` replaces the complete alias list; `--alias=` clears it. Contact notes accept positional content, `--note`, stdin, or `$EDITOR`. `hey contact note set` replaces the whole note — HEY has no append — so adding a detail means reading the note, changing it and writing all of it back. `hey contact note show --json` (and `hey contact show --json`) answers the note three ways, each an empty string for a contact with no note: `note`, HEY's plain text, which drops formatting (bold is lost, list items become `•`); `note_html`, as HEY serves it; and `note_markdown`, the note as Markdown. `note_markdown_lossless` says whether that Markdown holds everything in the note. When it is `true`, `note set` writes the Markdown back as the same note — HEY's editor markup becomes plain paragraphs on the first write, and from then on a read and a write give the same Markdown and the same HTML. When it is `false`, the note holds an attachment, an image, a table or other markup Markdown has no syntax for, and setting Markdown would drop it: change `note_html` and write it back with `--note-html` instead. HTML passed to `--note-html` has the `
` HEY serves a note in taken off, so writing back `note_html` does not nest the note one level deeper each time. Nothing guards a read and a write against someone else saving in between: the last save wins, as it does in the web app. HEY hides contacts rather than permanently deleting them; hidden contacts leave lists, autocomplete, and search, and can be shown again by ID. `hey contact deliver --to imbox|feed|papertrail|screened-out` changes where a contact's future email arrives. It takes a contact ID — not an email address, clearance ID, box item ID, or box ID — and is write-only because HEY does not expose the current setting as structured data. Imbox removes a custom box preference; choosing a box does not screen a denied contact back in. Screened Out is the blocking choice and applies only to external email contacts. Selecting The Feed removes an existing bundle because HEY cannot keep Feed-designated contacts bundled; Imbox and Paper Trail preserve eligible bundles. Existing mail can move asynchronously, so success means HEY accepted the change, not that every affected thread has already moved. Bundling groups a contact's mail into one row without merging or deleting the underlying threads; unbundling lists those threads separately again. HEY bundles only a contact with no box preference or one sent to the Paper Trail; for any other, bundling answers success and changes nothing. `hey contact photo set ` uploads a photo for a contact, replacing any it had, and answers the contact with its new `avatar_url`; `hey contact photo remove` takes it off again, and removing from a contact with no photo succeeds. The file must be a PNG or JPEG — the formats HEY's own photo picker takes — of at most 40 MiB, the most HEY's web form can send, and at most 58 megapixels and 16000 pixels a side. That admits a full-resolution photo from a modern phone (an iPhone's 48 megapixels, or 50 on Android) and stays inside what HEY can resize, which for a JPEG it has to rotate upright is about 58.7 megapixels; the one exception is a CMYK JPEG that has to be rotated, which HEY can resize only up to about 47 megapixels, and which cameras do not produce. HEY shows a contact photo at 300×300 at most anyway. It is judged by its bytes rather than its name, and its header must decode, because HEY stores any file it is handed and fails only later, when it resizes the photo to show it: a HEIC from a phone, a photo too large to resize, or a file that is not an image would be saved and never shown, the contact keeping their initials. HEY crops the photo to a square, so a square image without transparency works best, and it caches avatars, so after a set or a remove the old photo can still show in places for up to 30 minutes. Your own contact takes a photo too. A contact that is hidden, an alias, or another HEY user cannot be edited: updating it, setting or deleting its note, or setting or removing its photo answers `not_found`. `hey contact list` holds only non-HEY senders you have screened in — other HEY users, senders still in or denied by The Screener, aliases and hidden contacts are left out. It has no search: HEY ignores a query on that list, so finding a contact by email means reading every page (`--all`) and filtering, which matches primary addresses only. +Contact updates preserve omitted name, email, and alias fields. Supplying `--alias` replaces the complete alias list; `--alias=` clears it. Contact notes accept positional content, `--note`, stdin, or `$EDITOR`. `hey contact note set` replaces the whole note — HEY has no append — so adding a detail means reading the note, changing it and writing all of it back. `hey contact note show --json` (and `hey contact show --json`) answers the note three ways, each an empty string for a contact with no note: `note`, HEY's plain text, which drops formatting (bold is lost, list items become `•`); `note_html`, as HEY serves it; and `note_markdown`, the note as Markdown. `note_markdown_lossless` says whether that Markdown holds everything in the note. When it is `true`, `note set` writes the Markdown back as the same note — HEY's editor markup becomes plain paragraphs on the first write, and from then on a read and a write give the same Markdown and the same HTML. When it is `false`, the note holds an attachment, an image, a table or other markup Markdown has no syntax for, and setting Markdown would drop it: change `note_html` and write it back with `--note-html` instead. HTML passed to `--note-html` has the `
` HEY serves a note in taken off, so writing back `note_html` does not nest the note one level deeper each time. Nothing guards a read and a write against someone else saving in between: the last save wins, as it does in the web app. HEY hides contacts rather than permanently deleting them; hidden contacts leave lists, autocomplete, and search, and can be shown again by ID. `hey contact deliver --to imbox|feed|papertrail|screened-out` changes where a contact's future email arrives. It takes a contact ID — not an email address, clearance ID, box item ID, or box ID — and is write-only because HEY does not expose the current setting as structured data. Imbox removes a custom box preference; choosing a box does not screen a denied contact back in. Screened Out is the blocking choice and applies only to external email contacts. Selecting The Feed removes an existing bundle because HEY cannot keep Feed-designated contacts bundled; Imbox and Paper Trail preserve eligible bundles. Existing mail can move asynchronously, so success means HEY accepted the change, not that every affected thread has already moved. Bundling groups a contact's mail into one row without merging or deleting the underlying threads; unbundling lists those threads separately again. HEY bundles only a contact with no box preference or one sent to the Paper Trail; for any other, bundling answers success and changes nothing. `hey contact photo set ` uploads a photo for a contact, replacing any it had, and answers the contact with its new `avatar_url`; `hey contact photo remove ` takes it off again, and removing from a contact with no photo succeeds. The file must be a PNG or JPEG — the formats HEY's own photo picker takes — of at most 40 MiB, the most HEY's web form can send, and at most 58 megapixels and 16000 pixels a side. That admits a full-resolution photo from a modern phone (an iPhone's 48 megapixels, or 50 on Android) and stays inside what HEY can resize, which for a JPEG it has to rotate upright is about 58.7 megapixels; the one exception is a CMYK JPEG that has to be rotated, which HEY can resize only up to about 47 megapixels, and which cameras do not produce. HEY shows a contact photo at 300×300 at most anyway. It is judged by its bytes rather than its name, and its header must decode, because HEY stores any file it is handed and fails only later, when it resizes the photo to show it: a HEIC from a phone, a photo too large to resize, or a file that is not an image would be saved and never shown, the contact keeping their initials. HEY crops the photo to a square, so a square image without transparency works best, and it caches avatars, so after a set or a remove the old photo can still show in places for up to 30 minutes. Your own contact takes a photo too. A contact that is hidden, an alias, or another HEY user cannot be edited: updating it, setting or deleting its note, or setting or removing its photo answers `not_found`. `hey contact list` holds only non-HEY senders you have screened in — other HEY users, senders still in or denied by The Screener, aliases and hidden contacts are left out. It has no search: HEY ignores a query on that list, so finding a contact by email means reading every page (`--all`) and filtering, which matches primary addresses only. The Screener is where first-time senders wait. `hey screener list` returns clearance IDs — not contact IDs — with the sender and the subject of what they sent, plus `topic_id` for reading the thread before deciding. `--count` asks for the number alone, which is a far cheaper request than the queue, and prints it as a bare number like every other command's `--count`, so `n=$(hey screener list --count)` reads it directly. Approving delivers everything the sender has waiting; denying hides it. Either is reversible with the opposite command, and `hey screener history` shows what was already decided. Both listings page the way `hey box view` does: `--all` follows HEY's cursor to the end of the queue (up to 100 pages), and a single-page or capped read reports `next_page` in its JSON meta, which `--page ` continues from — the cursor is opaque, so a page *number* does not name a position. `hey screener list` also reports `total_count`, the whole queue's size, next to what the read returned. `--box` and `--seen` approve one sender at a time; several IDs go through HEY's bulk endpoint, which takes neither. `--seen` reliably marks the mail read only for a sender with one waiting thread. `--spam` marks what they sent as spam and, for a single ID, trains HEY's filter, which is harder to undo than denying. `hey screener clear` moves everything waiting to Trash (a shared thread loses your access instead) without approving or denying anyone — those senders reappear on their next email. diff --git a/tests/smoke/contacts_test.go b/tests/smoke/contacts_test.go index 42de05af4..ae42fb960 100644 --- a/tests/smoke/contacts_test.go +++ b/tests/smoke/contacts_test.go @@ -155,7 +155,7 @@ func TestContactPhoto(t *testing.T) { // The URL changes with the blob whether or not HEY can resize it; one it cannot is // answered with the contact's initials as SVG. So fetch it: the photo is a raster. // The avatar route needs no session. - resp, err := http.Get(withPhoto.AvatarURL) + resp, err := (&http.Client{Timeout: 10 * time.Second}).Get(withPhoto.AvatarURL) if err != nil { t.Fatalf("fetch avatar: %v", err) }