TW-4638: Air fixes#70
Open
qasim-nylas wants to merge 5 commits intomainfrom
Open
Conversation
Hardening pass on the Air web UI's i003 work plus a calendar-invite
RSVP feature pulled in along the way. make ci-full passes
end-to-end (180 unit packages, 57 integration packages, 352
Playwright e2e tests).
== Calendar invite RSVP (new) ==
- New handler internal/air/handlers_email_rsvp.go + tests/fixtures.
- Server-side resolution: re-fetches the message and parses the
iCal UID; never trusts a client-supplied event ID.
- domain.EventQueryParams.ICalUID lets the adapter resolve a
Nylas event ID from a VEVENT UID.
- adapter SendRSVP adds input validation: required IDs, status
allow-list (yes/no/maybe, case-insensitive), 1024-byte comment
cap, all wrapped in domain.ErrInvalidInput.
- Frontend invite card built via DOM construction
(buildCalendarInviteCard, buildInviteAttendees,
buildInviteActions); no innerHTML on user data.
- Adapter regression suite in calendars_events_rsvp_extra_test.go.
- E2E coverage: tests/air/e2e/archive-and-rsvp.spec.js (16 tests).
== Archive correctness ==
Gmail-archive silent no-op + Codex P1 IMAP/EWS regression.
- domain.UpdateMessageRequest.Folders JSON tag drops `omitempty`.
Empty []string{} now wires as `"folders":[]` (Gmail archive,
drop INBOX label) instead of being elided. nil still wires as
`"folders":null`; Nylas treats null and absent equivalently
(leave alone). domain.UpdateThreadRequest mirrors this.
- adapters/nylas/{messages,threads}.go: cherry-pick when
`req.Folders != nil` (was `len > 0`, which collapsed the
archive intent silently).
- computeArchiveFolders reordered: typed Archive folder lookup
(system_folder='archive', strict type match, no name fallback)
runs before the INBOX-removal filter. Fixes the IMAP/EWS
regression where folders:['INBOX'] + a typed Archive folder
produced folders:[] upstream instead of [archiveFolder.id].
== Privacy sweep (backend) ==
Don't echo upstream errors, decoder errors, or query-param values
back to the client.
- writeUpstreamError(w, status, msg, err, attrs...) helper logs
raw error via slog and returns a generic client message.
- writeBadParamError logs the parser error and returns
"invalid <key>".
- parseJSONBody slog-logs the json.Decoder error; the raw error
often quotes input bytes (PII / secrets).
- Sweep sites: handlers_email.go, handlers_email_invite.go,
handlers_availability.go (8 redirected sites).
- Test lock-down: 6x *_DoesNotLeakUpstream tests across
list / invite / availability / freebusy / conflicts / get-email.
== Token redaction ==
domain.Grant.AccessToken and Grant.RefreshToken now use json:"-"
so list/get JSON output (`nylas admin grants list --format json`,
shell history, CI logs) cannot leak OAuth secrets even if a
future upstream API change starts returning them. Tokens are
still populated programmatically by ExchangeCode.
== XSS hardening (frontend) ==
- email-selection.js: invite card builders ported from HTML
strings to DOM nodes; aria-hidden="true" on all 6 SVG icon
literals; iframe error state replaced with replaceChildren +
DOM construction.
- mobile.js: refresh icon uses createElementNS instead of static
innerHTML.
- Critical fix: infinite recursion in email-messages.js debug
logger.
== Demo-mode "All Mail" semantics ==
normalizeDemoFolder("all"/"all mail") returns "all" (was
"archive"). Pairs with the existing target == "all" branch in
demoEmailIsInFolder. Aliasing All Mail to archive surfaced only
the single demo email tagged archive instead of every demo
email.
== Observability ==
Fail-first tests captured silent drops; production logs make
them visible.
- handlers_email.go: handleDeleteEmail offline-enqueue failure
now logs via slog (parity with handleUpdateEmail). Transient-
API-error -> queue-write-fails branch co-logs apiErr +
queueErr at slog.Error (was dropping queueErr context).
- server_offline.go: 3-strike permanent drop (grant unresolvable
AND processOfflineAction failure) now slog.Error with
resource_id and last error.
- handlers_email_invite.go: download / read / parse failures in
tryParseAttachmentInvite log at slog.Debug (was silent).
- handlers_email.go: get-email cache-fill failure slog.Warn
(parity with list-emails).
- httputil.WriteJSON encoder errors now slog.Error instead of
swallowed.
== File-size split ==
handlers_email.go was approaching the 600-line max. Split
unexported helpers into handlers_email_demo.go (demo seed) and
handlers_email_offline.go (offline queue logic). Same package,
no caller changes.
== Breaking changes (in-repo only) ==
- domain.UpdateMessageRequest.Folders / UpdateThreadRequest.Folders
JSON tag drops `omitempty`. Wire shape changes: nil -> null
(was: omitted), []string{} -> [] (was: omitted). Every in-repo
caller updated.
- HTTP error response bodies are now generic instead of leaking
%v of upstream errors. Status codes unchanged.
- domain.Grant.AccessToken / RefreshToken no longer in JSON. Any
external scraper of `nylas admin grants list --format json`
will see the fields absent (security fix).
- JS rename renderCalendarInviteCard -> buildCalendarInviteCard
and renderInviteAttendees -> buildInviteAttendees (return type
changed from HTML string to DOM Node). 0 external callers.
== Tests added (~30) ==
- domain/basic_test.go: TestGrant tokens-never-serialised lock-down.
- domain/email_request_test.go: TestUpdateMessageRequest_
FoldersWireFormat (3-state wire-format pin).
- adapters/nylas: Gmail-archive empty-folders wire-shape regression
on messages and threads, ical_uid query-param forwarding, RSVP
validation + normalisation + comment cap.
- internal/air:
* handlers_email_offline_test.go (460 lines, 11 tests): cache
fallback, offline queue, transient-network -> queue,
queue-fails fallthrough, shouldQueueEmailAction predicate,
normalizeDemoFolder aliases, demoEmailIsInFolder branches.
* handlers_email_offline_replay_test.go: PreservesEmptyFolders
Intent / NilFoldersIntent (queue-side replay shape).
* handlers_email_silent_failure_test.go: HandleDeleteEmail
OfflineQueueFails / QueueWriteAfterTransientError /
HandleGetEmail_CacheFillFailure.
* handlers_email_invite_silent_failure_test.go (3 tests).
* server_offline_silent_failure_test.go (3-strike +
grant-resolve permanent failure).
* handlers_privacy_sweep_test.go: 6x DoesNotLeakUpstream pins.
* handlers_helpers_test.go: TestRedactEmail (8 subtests).
* handlers_availability_test.go: bad duration_minutes /
interval_minutes echo lock-downs + integer-param sanity.
- tests/air/e2e/archive-and-rsvp.spec.js (813 lines, 16 tests):
RSVP success/failure, archive Strategy 1/2/null, Codex P1
regression, vanity-Gmail-label safety, archive Undo
restore-unavailable / restore-failed, archive success
detail-pane empty state, navigated-away guards.
Verification:
- go build ./...: clean
- go vet ./...: clean
- make ci-full: 0 FAIL across 41,978 + 19,879 lines of output
- 0 lint issues on diff (golangci-lint --new-from-rev=main)
- All touched files under 600-line cap
Cleanup follow-up to eba27dc — reduces over-engineered comments and removes the "_extra_test.go" smell. No behavior change; make ci-full clean (180 packages ok, 0 FAIL). - Merge calendars_events_rsvp_extra_test.go into the canonical calendars_events_rsvp_test.go (combined 474 lines, under cap). - Rename handlers_email_rsvp_extra_test.go -> handlers_email_rsvp_edge_cases_test.go (the canonical 542-line file is too close to the 600-line cap to merge). - Trim verbose privacy/security docstrings in handlers_helpers.go (parseJSONBody 9->3, writeBadParamError 5->2, writeUpstreamError 11->3 lines). - Trim handlers_email_rsvp.go: errEventNotImported / errNoWritable Calendar 5-6 -> 2 lines each; handleEmailRSVP pipeline doc 13 -> 5 lines (kept the security-invariant paragraph); 6 inline comment blocks in the body trimmed; findInviteEventAcrossCalendars 17 -> 4 lines; writableCalendars 3 -> 1 line; findEventByICalUID 6 -> 2 lines. - Drop the writeError-vs-http.Error preamble in handlers_availability.go and trim parseInt64Param / parseIntParam docstrings.
- Add agent, audit, and dashboard command pages with full sub-command coverage (mirroring the existing per-group page + JS module pattern) - Backfill missing sub-commands: email signatures, email templates, admin callback-uris - Move theme toggle outside .header-controls so it stays visible in the setup view (where header-controls has display: none) - Give .header position: relative + z-index: 50 and wrap controls + theme button in .header-right so the open ACCOUNT dropdown overlays the glass-card stacking contexts below - Reorder sidebar nav alphabetically; rename "Nylas Dashboard" -> "Dashboard" - Update Documentation URL (developer.nylas.com root) and rename API v3 Reference link to "CLI Command Reference" (cli.nylas.com/docs/commands) - Extend /api/exec allowlist for the new commands; remove three bare 1-word entries (agent/audit/dashboard) flagged by review as unnecessary surface widening via the prefix-matching fallback - Update overview-page and keyboard Playwright specs for the new link copy, nav layout, and password-toggle tab order
Address remaining items from the i003 review pass. == Privacy sweep == - handlers_bundles.go: validateBundleRule failure no longer echoes the raw regexp.Compile error to the client; logs via slog.Warn with the rule index and returns a generic "Invalid regex in rule N". - server_template.go: ExecuteTemplate failure no longer echoes the template error (which can include data field paths and snippets); logs via slog.Error and returns "Failed to render page". - ui/server_exec.go: command exec error no longer echoed to clients; logs via slog.Error with args and returns generic "Command failed". Aligns the /ui surface with the air doctrine of "no upstream errors echoed to clients". == Defense-in-depth == - adapters/nylas/calendars_events.go: GetEventsWithCursor now validateRequired's grant ID and calendar ID at entry, matching its GetEvents sibling. Not exploitable here (RSVP handler passes server-derived IDs) but closes a small gap for future callers. == JS doctrine alignment == - ui/static/js/output.js: parseTable / parseAnsi / formatOutput refactored to return DOM nodes (table or DocumentFragment) instead of HTML strings. Every cell, header, and text run set via textContent so the call site never has to reason about escaping. ANSI parser walks once with a style stack; per-reset pop-one-level semantics preserved. - ui/static/js/commands.js: setOutputSuccess uses output.replaceChildren(formatted) instead of output.innerHTML = formatted. Trust boundary moves into output.js, bringing the /ui surface in line with the air diff's "every interpolation goes through textContent" rule. Verification: - go build / vet / test ./...: clean - golangci-lint run --new-from-rev=main: 0 issues - node --check: clean - All file sizes under cap
`make ci-full` was failing on golangci-lint's govet inline check: "Constant reflect.Ptr should be inlined". reflect.Ptr is the legacy alias; reflect.Pointer is the canonical name since Go 1.18 — they are the same constant. Mechanical rename across 5 files (15 sites). Pure naming change, no behavior difference. - internal/adapters/output/quiet.go (2) - internal/adapters/output/table.go (4) - internal/cli/common/format.go (5) - internal/cli/config/get.go (2) - internal/cli/config/set.go (2) Verification: - golangci-lint run --timeout=5m: 0 issues (was 15) - make ci-full: green end-to-end (180 packages PASS, 0 FAIL)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Hardening pass on the Air web UI's i003 work plus a calendar-invite RSVP feature.
handleEmailRSVPhandler with server-side event resolution from VEVENT UID (never trusts client event IDs); status allow-list (yes/no/maybe) and 1024-byte comment cap enforced at handler and adapter.domain.UpdateMessageRequest.FoldersJSON tag dropsomitemptyso Gmail archive (empty folders) wires as[]instead of being elided. ReorderedcomputeArchiveFoldersto do typed-Archive lookup before INBOX-removal, fixing an IMAP/EWS regression.err.Error()in HTTP responses with sanitized client messages +slog-logged raw errors. New helperswriteUpstreamError,writeBadParamError,redactEmail. Same treatment applied to bundles, server template, andui/server_exec.go.domain.Grant.AccessToken/RefreshTokenusejson:"-"so list/get JSON output cannot leak OAuth secrets. Tokens still populated programmatically byExchangeCode./uicommand output renderer all switched to DOM construction (createElement+textContent+replaceChildren). NoinnerHTMLon user data anywhere in the diff.normalizeDemoFolder("all"/"all mail")returns"all"(was"archive"), so demo "All Mail" surfaces every demo email instead of just the archived one.slognow captures previously silent failures: offline-enqueue drops, 3-strike permanent drops, invite-parse failures, get-email cache fills, encoder errors.handlers_email.gosplit intohandlers_email_demo.goandhandlers_email_offline.goto stay under the 600-line cap.allowedCommands. Removed three bare 1-word entries that widened surface via prefix-matching.reflect.Ptrwithreflect.Pointer(Go 1.18+ canonical name) across 5 unrelated files. Unblocksmake ci-full.Verification