Add support ticket and organization commands

[lukaskroepfl]

What

Adds two things the CLI could not do before: listing organizations (including the sub-org hierarchy) and working with support tickets.

bitmovin account organizations list [--parent <org-id>] [--type root|sub]

bitmovin support tickets list    [--organization <org-id>] [--status …] [--category …]
                                 [--priority …] [--severity …] [--search …] [--sort …]
bitmovin support tickets get     <ID> [--organization <org-id>]
bitmovin support tickets create  --category … --body … [--organization <org-id>] [-y]
bitmovin support tickets comment <ID> --body … [--html] [-y]

Sub-org targeting is via --organization, which sets the X-Tenant-Org-Id header, so tickets can be listed and filed for a sub-organization rather than only the credential's own org.

Creating a ticket asks first, and refuses rather than guessing

create and comment print the exact payload with a warning that the ticket is real and cannot be withdrawn via the API, then prompt with the default set to no.

With no TTY, or in --json mode, and without --yes, the command errors out (exit 2) and sends nothing:

$ bitmovin support tickets create --category other --subject "…" --body "…" --organization 25d46556-…
This files a REAL support ticket with Bitmovin support.
Support engineers will see it, and it cannot be withdrawn via the API.

Organization: 25d46556-817e-48f3-b762-91d2b061989d
Payload: { "body": …, "category": "other", "subject": …,
           "organizationId": "25d46556-817e-48f3-b762-91d2b061989d" }
 ›   Error: Creating a support ticket requires confirmation.

This is deliberate and load-bearing: while verifying these endpoints by hand I filed three junk tickets into production Zendesk because I sent throwaway payloads to a write endpoint I expected to reject me. A CLI that makes that easy is a CLI that will do it to someone else.

organizationId is never a separate flag — it is always pinned to the X-Tenant-Org-Id value, so the "body organizationId must match the authorized org" rejection is unreachable from the CLI.

Validation happens before the request

Each of these fails locally with an actionable message instead of a confusing API error:

Input	Why it is rejected client-side
--offset not a multiple of --limit	the API pages by offset/limit and silently returns an earlier page
--search with punctuation, or > 100 chars	the API rejects it
bad --status / --category / --priority / --severity / --sort	validated against the real enums
--encoding-id without --category encoding; --license/--page-url outside player/analytics	the API gates these by category
comment without a stamp	comment reads the ticket first and sends its modifiedAt as the required updatedStamp, so nobody hits the misleading 1004 … Check your JSON syntax that a missing stamp actually produces

A 403 now names the organization the request was scoped to and points at account organizations list, since the common cause is a tenant org the credential has no ACL grant for.

Notes on the endpoints

• Uses the non-deprecated /v1/support/tickets. bitmovin-open-api marks /v1/account/zendesk/tickets as deprecated. support-service serves both from one controller (@RequestMapping("/tickets", "/public/tickets")) behind the two gateway routes, so they are the same handlers. Verified equivalent against production before switching: both return total=9 with the same first ticket for the same tenant org, and the detail endpoint returns the same caseId/modifiedAt.
• The org hierarchy is derived from the flat listing. GET /v1/account/organizations/{id}/sub-organizations returns 1001 An organization with the given id does not exist for a valid, visible org id, so it is not used; parentId on GET /v1/account/organizations is. config list organizations was moved onto the same shared derivation (its output is unchanged).
• These endpoints are not in the generated @bitmovin/api-sdk, so they go through a small REST helper (src/lib/rest.ts) that reuses the CLI's existing credential resolution — API key / OAuth precedence, silent token refresh, and X-Api-Client identification stay identical to SDK calls. No hand-rolled config reads; the API key is never printed.

Testing

npm run build, npm run lint, npm test all clean — 34 files, 281 tests (+26 new; was 29 files).

No POST was made from the test suite — HTTP is mocked throughout (vi.mock on lib/rest.js for commands, stubbed fetch for the REST client). Live checks were read-only GETs plus the create-refusal path above.

Out of scope

Attachments (POST /tickets/uploads and a comment's uploads) — documented as "use the dashboard". The rarer create fields (collaborators, businessImpact, streamId, the player/analytics config blobs). account organizations get/create.

🤖 Generated with Claude Code

[lukaskroepfl]

Review context for @SInCE

This went through a five-dimension review (correctness, architecture, documentation, maintainability, security) before landing here. Everything raised is now addressed; three commits, cleanest read is commit-by-commit:

1. f4e3fc7 — the commands
2. dfa6d30 — correctness/security/docs fixes
3. 9245a95 — the architecture refactor (shared tenant scope, one field table, one confirm policy)

Things worth your eye specifically

• getClient() now takes a tenant-organization override and --organization moved to BaseCommand.tenantOrgFlag. This touches shared plumbing every topic uses. The motivation: the flag previously lived in lib/organizations.ts and only REST-backed commands could honour it, so adding it to an SDK-backed command would have parsed fine and been silently ignored. If you'd rather it were opt-in per command rather than available from BaseCommand, say so — it's a one-line move.
• lib/tenant.ts exists for a mundane reason: every command test mocks client.js wholesale, so putting resolveTenantOrgId there broke 16 unrelated test files. Its own module also keeps organizations.ts to pure derivation.
• confirmDestructive() returns a three-way outcome (proceed / declined / unconfirmable) rather than a boolean, so a declined prompt and an unaskable one keep different exit codes. encoding jobs delete and stop currently confirm nothing — this is ready for them to adopt.

Two API behaviours that forced local checks — both verified against support-service, not guessed:

• A category-mismatched field (--allow-file-access outside --category encoding) is accepted and silently dropped by the API, not rejected. So the ticket looks filed while the data never arrives, and the check has to be client-side. There's a comment saying so; please don't let anyone relax it expecting a loud error.
• --offset must be 0 or a multiple of --limit: the server computes page = (offset / limit) + 1 with integer division, so a non-multiple offset silently serves an earlier page.

Known follow-ups, deliberately not in this PR

• getAuthHeaders/requestScope still read config directly. When feature/config-profiles lands, requestScope() is the one place that needs the profile threaded through — I left the shape to match that branch rather than guessing.
• Attachments (POST /tickets/uploads) unimplemented; the rarer create fields (collaborators, businessImpact, streamId, player/analytics config blobs) have no flags; TICKET_CATEGORIES omits the API's UNKNOWN, so tickets in that state can't be filtered.
• Ctrl+D at the confirmation prompt neither confirms nor aborts — the promise never settles and the process exits 0. Not a bypass (nothing is sent), but a wrapper could read it as success.

Testing: 315 tests. The suite is mutation-verified rather than just green — eleven planted mutations each fail, including "canPrompt always true", "confirm defaults to yes", "getAuthHeaders sends no credential", and "getClient ignores the org override". Worth knowing why: the confirmation gate and getAuthHeaders were originally completely unpinned, because every command test mocks them. No POST was made from the test suite; live checks were read-only GETs plus the create-refusal path.

[lukaskroepfl]

Update: maintainability review landed, and it found a live bug — plus a correction to my earlier comment

The fifth review dimension finally completed (it hit API errors twice before) and reviewed the code after the architecture refactor. It found one real bug and two regressions that refactor introduced. All fixed in 18ec9fb; CI green.

Correction to my previous comment. I wrote that "dropping a mapped field value fails the suite". That was only true for the values translations (REQUEST_TYPES etc.) and organizationId. 11 of the 20 payload keys could be renamed or deleted with the suite green — so the single-source-of-truth table removed the duplication and added no coverage, turning "compiles but drifts" into "silently wrong and unverified". There is now an exhaustive payload assertion.

The live bug: abbreviate(text, 300, 0) printed the entire text while labelling it truncated, because slice(-0) is slice(0). It was in the comment confirmation, so a long support reply emitted head-300, [4700 characters omitted], then all 5000 characters — pushing the PUBLIC comment warning off screen, which is precisely what the preview exists to prevent.

The typing regression: Object.fromEntries widened createTicketFlags() to {[k: string]: Flag}, erasing the 18 flag names from oclif's parsed-flags type. flags['sdk-version'] no longer compiled, and the only thing keeping create.ts building was the as CreateTicketFlags cast — so the drift the table was meant to remove had just moved somewhere less visible. The generator now returns exact per-flag types and the cast is gone entirely, so the flag→payload chain is type-checked again.

Also fixed from that review: requestScope() wasn't adopted by the two organization commands (the drift it exists to prevent already existed in this PR); the paging loop had no iteration bound; sanitization was missing on the comment confirmation's subject and on tickets get's requester/organization; config list organizations nested an orphan sub-org under an unrelated root; 409/network/timeout had no CLI handling; --sort createdAt:desc was sent lowercase and silently ignored; and MAX_BODY_LENGTH bounded only --body-file, not --body.

323 tests, 17 mutations verified across the three fix rounds. Two notes on the new ones: the paging-bound test carries an explicit 10s timeout so removing the bound fails fast instead of hanging CI, and the CREATE_TICKET_FIELDS payload assertion is exhaustive on purpose — adding a field means updating that test, which is the point.

Still open, unchanged from my earlier comment: the feature/config-profiles coupling, attachments, the rarer create fields, UNKNOWN category filtering, and Ctrl+D at the prompt. The review also left a set of low-severity items I did not take: test files that no longer sit beside the modules they test, constants re-typed as literals in the docs, and the four-place edit needed to add a list filter. Happy to do any of those if you'd rather they land here.

[SInCE]

Code review (high effort)

Reviewed the 5-commit diff and read the two author comments first — everything documented there as known/accepted (Ctrl+D prompt hang, config-direct reads pending feature/config-profiles, attachments, rarer create fields, UNKNOWN category filtering, confirmDestructive not yet adopted by encoding jobs delete/stop) is excluded from the findings below.

The PR is unusually well-defended (mutation-tested, careful comments). Two items are worth blocking on; the rest are polish. Requesting changes for the two high-severity ones — happy to approve once #1 and #2 are addressed (or consciously deferred).

Must fix

1. get --json leaks attachment download URLs, bypassing the --show-secrets masking (data exposure).
2. account organizations list declares --organization but ignores it, and throws on --organization "" for a flag that has no effect — contradicting the flag's own "can never be silently ignored" invariant, and its printed usage hint.

Worth fixing

3. sanitizeForTerminal keeps \r, leaving a terminal line-overwrite vector inside its own threat model.
4. describeTransportFailure misclassifies any TypeError mentioning "network"/"socket" as a transport failure, for every command.
5. validateSort vs normalizeSort trim mismatch → spaced sort spuriously rejected.
6. 403 handler names the config org for SDK errors (latent until an SDK command adopts the flag).

Cleanup

7. resolveBody duplicated between create.ts and comment.ts (already drifting).
8. Misplaced doc comment above normalizeSort.
9. Dead defensive full-list re-sort in toOrganizationRows.
10. limit/offset flag pair re-declared (9th copy) — a shared paginationFlags would fit.

Checked and cleared (no change needed)

• normalizeEnumFilter not lowercasing is fine — per the code's own note the API uppercases filter values server-side (unlike sort direction, which is why normalizeSort exists).
• getTicket before the confirm gate in comment.ts is intentional (supplies the collision stamp + latest-comment preview).
• config list organizations failing hard on a short page is deliberate.
• buildCreateTicketPayload category gate is correct (oclif boolean flags default to undefined, not false, when absent).

[lukaskroepfl]

All 10 items addressed in 97d7a41 — both must-fixes, the four worth-fixing, and the four cleanups. Nothing consciously deferred; each thread has a reply with the specifics.

Two decisions worth calling out:

• Bump vite from 8.0.3 to 8.0.10 #2: dropped --organization from account organizations list rather than honouring it — /account/organizations is scoped by the credential, not by X-Tenant-Org-Id, so honouring it would mean inventing a scope the endpoint does not have. --parent already covers sub-org filtering.
• Mask secrets in account info by default #6: fixed now rather than left latent — requestScope() memoizes the resolved scope and the 403 branch falls back to it, so the seam is safe before an SDK command adopts the flag.

Also folded in, same threat model as #3: attachment file names are now sanitized before printing (they are chosen by whoever uploaded the file).

337 tests pass, lint and tsc clean. Each fix was mutation-checked — reverting it individually fails the new test.

[SInCE]

Security review (high effort, second pass)

Re-reviewed at head 97d7a41 with a security focus. The 10 issues from the previous round are all fixed, and I confirmed those fixes introduced no new weaknesses. The REST layer is clean on injection: encodeURIComponent(caseId) neutralizes path traversal, undici rejects CRLF header injection before send, no prototype-pollution sink, no ReDoS, redirect:'error' prevents credential-forwarding, the confirm gate fails closed, and the collision stamp is unbypassable.

Requesting changes on one item — #1, a real --show-secrets bypass (capability URLs leak via comment body/htmlBody). The rest are worth-fixing hardening and defense-in-depth; the standout is #6, which is the root cause of several others: sanitize at the output boundary and #2/#3/#8/#10 collapse into one fix.

Must fix

1. Inline capability URLs in comment body/htmlBody are never redacted → --show-secrets bypass in get --json.

Worth fixing

2. API error text (developerMessage/message) printed raw → ANSI/CR injection on the error path.
3. Org name column rendered unsanitized in the org-list commands (get.ts sanitizes the same field).
4. Untrusted response body echoed into developerMessage (to stdout under --json).
5. Empty/whitespace config org bypasses the empty-string scope guard (flag-only).
6. Root cause: terminal-safety is per-print-site, not at the outputData/formatTable boundary.

Defense-in-depth / consistency

7. Unbounded response.text() — memory-exhaustion DoS from a hostile/compromised endpoint.
8. Attachment url printed unsanitized under --show-secrets.
9. tickets get --show-secrets prints no exposure warning (account info does).
10. Create/comment confirmation preview prints the body unsanitized above the y/N prompt.

Checked and cleared

Path traversal, CRLF header injection, prototype pollution, ReDoS, redirect/credential-forwarding, confirm-gate fail-closed, collision stamp, create header/body org match (non-empty case), API key / OAuth token never printed.

[SInCE]

Defense-in-depth — unbounded response.text() (memory-exhaustion DoS).

The whole response body is buffered into a string with no size cap; AbortSignal.timeout(30s) bounds time, not bytes. A malicious/compromised endpoint (or MITM — X-Api-Key is a custom header, and only redirects, not body size, are refused) returning a multi-GB body OOM-kills the CLI, in contrast to the carefully capped 65 KB outbound side. Consider a Content-Length pre-check or a bounded read.

[lukaskroepfl]

Not taken, deliberately. The endpoint is api.bitmovin.com over TLS with redirects refused, so this needs a compromised API or a MITM holding a valid cert, and the payoff is OOM-killing a short-lived CLI process on the user’s own machine — no data crosses a trust boundary. I did shrink what we keep from an unexpected body to 200 characters and stopped trimming the whole string to produce it (#4), which removes the only place the full body was being processed beyond the parse. Happy to add a Content-Length pre-check if you feel strongly, but I would rather not add a code path we cannot test against a real adversary.

[lukaskroepfl]

Second round addressed in a0e5251 — 7 taken as written, 2 taken in part, 1 declined with a reason. Per-thread replies have the detail.

Taken: #2 (error text sanitized), #3 (via the boundary fix), #5 (blank configured org — sharpest finding of the round; config set <key> "" now refused too), #6 (sanitizeForTerminal moved to lib/sanitize.ts, output.ts sanitizes every rendered cell), #8, #9, #10.

Taken in part:

• Bump lodash from 4.17.23 to 4.18.1 #1 — the inline-image premise is right, but I am not regex-scrubbing URLs out of comment text: that text is what get exists to show, and the match is unreliable both ways. Fixed the real defect instead — my docstring overclaimed. Scope is now stated explicitly in the code and in the --show-secrets docs row.
• Fix encoding templates validate to support JSON Schema 2020-12 #4 — kept the excerpt (it is what makes "a gateway answered with HTML" diagnosable) but capped it at 200 chars, sanitized it on print, and sliced-before-trim as you suggested.

Declined: #7 — TLS to a pinned host with redirects refused, so it needs a compromised API or a valid-cert MITM, and the payoff is OOM-killing a short-lived CLI on the user's own machine. Say the word and I will add a Content-Length pre-check anyway.

One correction on #6: the boundary fix does not cover #2, #8 and #10 — those write straight to stderr/stdout and never pass through output.ts, so each got its own fix. It closes #3 and makes future rendered fields safe by default, which is the real win.

348 tests pass, lint and tsc clean. Every fix in this round was mutation-checked individually.

[SInCE]

Approving ✅

Both review rounds are fully addressed, verified against the code at a0e5251 (not just the replies):

• Correctness round (10) — fixed in 97d7a41.
• Security round (10) — fixed in a0e5251: 7 taken as written, 2 taken in part (#1 docstring scoped, #4 excerpt capped + slice-before-trim), 1 declined (#7) with sound reasoning.

The REST layer is clean on injection/SSRF/prototype-pollution/ReDoS/credential-forwarding, the confirm gate fails closed, the collision stamp is unbypassable, and terminal-safety now lives at the output boundary (lib/sanitize.ts + output.ts) so future rendered fields are safe by default. CI green, 348 tests, lint + tsc clean, each fix mutation-checked. This supersedes my earlier changes-requested reviews.

Two optional, non-blocking follow-ups (either can be a later PR):

• #7 — a Content-Length pre-check on response.text() as defense-in-depth against a malicious/MITM endpoint (you reasonably declined it; noting for the record).
• #1 — closing the inline-image capability-URL case (a link inside comment text) server-side or via an opt-in --redact-links flag; the docstring overclaim itself is fixed.

Nice work — thorough and well-defended throughout.