feat: add api command for arbitrary authenticated requests#189
feat: add api command for arbitrary authenticated requests#189LoganBarnett wants to merge 1 commit into
Conversation
Adds a `confluence api <endpoint>` command modeled after `gh api`, allowing users to make authenticated requests to any Confluence REST endpoint without falling back to curl. Includes README documentation and tests. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
pchuri
left a comment
pchuri
left a comment
There was a problem hiding this comment.
Thanks a lot for the contribution! 🙌 A gh api-style escape hatch is genuinely useful for endpoints we haven't wrapped yet, and the test coverage (12 unit + 14 integration) is impressive — that's the right amount of rigor for something this generic.
I left a few inline comments. The two blocking ones (from my perspective) are (1) the file layout and (2) how absolute paths interact with apiPath on Cloud. The rest are nice-to-haves that could be follow-ups if you'd rather keep this PR focused. Either way, very nice work.
| }); | ||
|
|
||
| // API command (arbitrary authenticated requests, modeled after gh api) | ||
| program |
There was a problem hiding this comment.
Architecture nit. PR #186 split this file into per-domain modules under bin/commands/. To stay consistent with that direction, would you mind moving these ~135 lines into a new bin/commands/api.js and wiring it up the same way the other domain commands are? Right now this is the only new command added directly to bin/confluence.js since the refactor.
| if (/^https?:\/\//i.test(endpoint)) { | ||
| url = endpoint; | ||
| } else if (endpoint.startsWith('/')) { | ||
| url = `${this.protocol}://${this.domain}${endpoint}`; |
There was a problem hiding this comment.
Cloud-vs-Server path mismatch — probably the most important comment.
On Confluence Cloud, apiPath is typically /wiki/rest/api. By treating any /-prefixed endpoint as bypassing apiPath, the README's marquee example
confluence api /rest/api/content/123456789/labelresolves to https://{domain}/rest/api/content/... (404 on Cloud) instead of https://{domain}/wiki/rest/api/content/.... So the documented examples only work on Server/DC, while Cloud users have to know to write /wiki/rest/api/....
A couple of options:
- Document explicitly that absolute paths bypass
apiPath, and update the examples to use/wiki/rest/api/...for Cloud. - Introduce a third tier: treat
rest/api/...(no leading slash) as "relative to apiPath" (which axios already does via baseURL),/...as "relative to host but auto-prependapiPathwhen it looks like a REST endpoint", andhttps://...as absolute.
Option 1 is cheaper and matches the gh api mental model. Either is fine — but the current behavior + current README will silently break for Cloud users.
| if (options.jq) { | ||
| const { execSync } = require('child_process'); | ||
| try { | ||
| const filtered = execSync(`jq ${JSON.stringify(options.jq)}`, { |
There was a problem hiding this comment.
Shell quoting / minor injection surface.
JSON.stringify wraps the expression in double quotes, but the shell still interprets $, backticks, and \ inside double quotes. For example --jq '.[] | select(.name == "$HOME")' will have $HOME expanded by the shell before jq ever sees it. The blast radius is the user's own machine, but it's also trivial to sidestep:
const { spawnSync } = require('child_process');
const r = spawnSync('jq', [options.jq], {
input: typeof result.data === 'string' ? result.data : JSON.stringify(result.data),
encoding: 'utf-8',
});Args go straight to the binary, no shell involved.
| }); | ||
| output += filtered; | ||
| } catch { | ||
| process.exit(2); |
There was a problem hiding this comment.
When jq isn't installed (ENOENT) or the expression is invalid, this swallows the cause and exits with code 2 with no message. A one-line console.error('jq failed:', err.message) (or specifically detecting ENOENT and saying "jq is not installed") would save users a debugging round-trip.
| const fs = require('fs'); | ||
| let raw; | ||
| if (options.input === '-') { | ||
| raw = fs.readFileSync(process.stdin.fd, 'utf-8'); |
There was a problem hiding this comment.
PR #184 (fix(convert): read stdin as a stream and guard against TTY-only invocations) hit this exact pattern in the convert command — fs.readFileSync(process.stdin.fd, ...) can hang on a TTY and has issues with large inputs. Could you reuse the stream-based stdin helper from that fix to stay consistent?
| .option('--input <file>', 'Read body from file (use - for stdin)') | ||
| .option('--jq <expression>', 'Filter response with jq') | ||
| .option('-i, --include', 'Include response status and headers') | ||
| .option('--silent', 'Suppress all output') |
There was a problem hiding this comment.
Minor: the description says "Suppress all output", but the read-only block and the catch handler still write to stderr. Either tighten the implementation (--silent ⇒ also swallow stderr on errors) or relax the doc to "Suppress success output." Most CLIs go with the latter — script consumers usually still want to see errors.
| if (config.readOnly && WRITE_METHODS.includes(method)) { | ||
| console.error(chalk.red('Error: This profile is in read-only mode. Write operations are not allowed.')); | ||
| console.error(chalk.yellow('Tip: Use "confluence profile add <name>" without --read-only, or set readOnly to false in config.')); | ||
| process.exit(1); |
There was a problem hiding this comment.
analytics.track('api', false) is missing here, and on the field/header parse error paths above (lines 1978, 1989). The bottom catch tracks failures, but these early process.exit(1) branches don't, so the metric will under-count failures.
2 participants
Summary
confluence api <endpoint>command modeled aftergh api, allowing users to make authenticated requests to any Confluence REST endpoint (labels, restrictions, groups, audit, v2 API, etc.) without falling back tocurl. It's kind of a release valve for API calls thatconfluence-apihasn't had a chance to support yet.rawRequestmethod toConfluenceClientsupporting relative, absolute, and full URL endpoints.-X,-f,-H,--input,--jq,-i,--silentoptions with read-only profile enforcement.Test plan
rawRequest(relative/absolute/full URLs, methods, headers, auth, errors).apicommand (field/header parsing, read-only enforcement, method auto-detection,--input,--silent, help output).