feat(webscraper): connect Apify with an API token on self-hosted backends - #217
Conversation
Adds `insforge webscraper apify connect --token <token>` so agents without dashboard access can wire up Apify on a self-hosted InsForge backend, using the PUT /api/webscraper/apify/config endpoint instead of OAuth. Also fixes the ossFetch 404 message under /api/webscraper, which used to claim the feature is cloud-only and unsupported when self-hosted; a 404 now just means the backend predates the feature.
WalkthroughThe webscraper API now stores and validates self-hosted Apify tokens. The connect command accepts ChangesApify connection workflow
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryAdds a self-hosted Apify token connection flow.
Confidence Score: 5/5The PR appears safe to merge. The previously reported false-success path is closed because
|
| Filename | Overview |
|---|---|
| src/lib/api/webscraper.ts | Adds token storage through the OSS API and now rejects missing, malformed, or unconfigured token-status responses. |
| src/commands/webscraper/apify/connect.ts | Adds the self-hosted token branch while retaining the existing OAuth behavior. |
| src/lib/api/oss.ts | Replaces obsolete cloud-only webscraper 404 guidance with self-hosted upgrade instructions. |
| src/lib/api/webscraper.test.ts | Covers successful token storage, backend errors, malformed responses, and the previously reported unconfigured-token response. |
| src/commands/webscraper/apify/connect.test.ts | Verifies token-mode output and isolation from OAuth while preserving the original OAuth paths. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["webscraper apify connect"] --> B{"--token supplied?"}
B -->|Yes| C["Validate non-empty token"]
C --> D["PUT /api/webscraper/apify/config via ossFetch"]
D --> E{"configured === true?"}
E -->|Yes| F["Return masked token status"]
E -->|No| G["Raise CLIError"]
B -->|No| H["Run existing OAuth flow"]
H --> I["Run local Apify auth bridge"]
Reviews (2): Last reviewed commit: "fix: address review findings on the self..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
jwfing
left a comment
jwfing
left a comment
There was a problem hiding this comment.
Review — feat(webscraper): connect Apify with an API token on self-hosted backends
Summary: A well-scoped, TDD'd addition of a --token self-hosted path to webscraper apify connect that short-circuits OAuth and stores the token server-side; clean rename of api/apify.ts → api/webscraper.ts, no dangling importers, strong tests, no blocking issues.
Requirements context
No matching spec/plan found under docs/specs/ (the only specs present are diagnose and db-migrations; there is no docs/superpowers/). Assessed against the PR description and its stated pairing with backend PR InsForge/InsForge#1824, which ships the PUT /api/webscraper/apify/config endpoint this calls.
Findings
Critical
(none)
Suggestion
- Security (
src/commands/webscraper/apify/connect.ts:47,src/lib/api/webscraper.ts:154-158) — The token is supplied as a positional flag value (--token <token>), so it lands in shell history and the process argument list (ps,/proc). The CLI itself never logs/prints the raw token (good — only the masked key is echoed, and analytics carry no secret), but the invocation surface still leaks it. Consider also accepting it from an env var or stdin (cf.docker login --password-stdin). Non-blocking. - Functionality (
src/commands/webscraper/apify/connect.ts:98) —if (opts.token)treats--token ""as absent and falls through to the OAuth path, which then fails withNot logged in. Run insforge login first.— a confusing error for what is really an empty-token input. Atypeof opts.token === 'string'guard (or explicit empty-string rejection) would give a clearer message. Low blast radius. - Project convention (DEVELOPMENT.md §3) — Adding a user-facing flag to an existing command triggers the agent-skills sync checklist ("Did flags/defaults/output shape change? Update the skill examples."). If the
insforge-cliskill documentswebscraper apify connect, it's now stale re:--token. Consider opening the companion PR inInsForge/agent-skills. Cross-repo, non-blocking.
Information
- Functionality (
src/lib/api/oss.ts:212-214) — The new/api/webscraper404 message drops the dual self-hosted/cloud guidance the sibling messages (/api/compute,/api/payments,/api/memory,/api/advisor) all keep. A genuinely outdated cloud backend that route-404s now reads "Upgrade your InsForge instance," which doesn't apply to a cloud project. In practice cloud returns a 400 ("managed by InsForge Cloud"), not a 404, so this is edge-only — but the wording is slightly less precise than its neighbours. - Software engineering (
src/lib/api/webscraper.ts) —storeApifyToken(the config PUT) now lives inwebscraper.tswhile the siblingfetchApifyAccessToken(the token GET) stays inapify-token.ts; the PR body also references asrc/lib/api/apify-config.tsthat doesn't exist in the diff (the code landed inwebscraper.ts). Placement is reasonable, just worth noting the description drifted from the final layout. - Functionality (
src/commands/webscraper/apify/connect.ts:104-108) — The token path returns a hardcodedconnection: { apifyUsername: null, plan: null, status: 'active' }because the config endpoint returns only{ configured, maskedKey }, not account metadata. Fine given the backend validates before storing; just an assumption to be aware of ifconfiguredcan ever befalseon a 2xx.
Dimension coverage
- Software engineering — Strong. 9
connect.test.tscases pin both the new--tokenpath and that the OAuth path is unchanged; 4storeApifyTokencases cover success, backend-rejection propagation, cloud-managed propagation, and malformed-response-throws; 1oss.test.tscase guards the new 404 wording. Fail-before/pass-after claimed. Rename left no broken imports (api/apify.jshas no remaining referrers). Follows repo test/analytics conventions. - Functionality — Implements what the PR claims. Correctly requires a linked project but not a login JWT on the token path; correctly omits
opts.apiUrlfromossFetch(config lives on the OSS host, not the platform API); relies onossFetchthrowingCLIErrorfor non-2xx so bad-token / cloud-managed messages propagate. Depends on InsForge/InsForge#1824 landing first — degrades gracefully (404 → upgrade message) if it doesn't. - Security — Token validated + stored server-side via the existing admin-authed
ik_key; only the masked key is surfaced;trackGroupCommandsends no secret. One non-blocking exposure noted above (argv/shell history). - Performance — No concerns. Single PUT, no loops, no new hot-path work.
Verdict
approved (informational — no Critical findings; the human still gives the explicit GitHub approval). Suggestions above are optional polish.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/commands/webscraper/apify/connect.ts (1)
104-108: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueHardcoded
status: 'active'doesn't reflectstatus.configured.The returned
connection.statusandconnectionStateare fixed literals, independent of the actualApifyTokenStatus.configuredvalue from the backend. If the backend can ever respond 200 withconfigured: false(e.g., a future "clear token" semantics on the same PUT endpoint), JSON-mode consumers of this CLI would see a misleadingactive/newly-connectedresult. Worth confirming the backend's/api/webscraper/apify/configcontract guaranteesconfigured: trueon every 200 response before relying on the hardcoded value.♻️ Suggested defensive fix
return { connectionState: 'newly-connected', - connection: { apifyUsername: null, plan: null, status: 'active' }, + connection: { apifyUsername: null, plan: null, status: status.configured ? 'active' : 'not-configured' }, token: status, };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/commands/webscraper/apify/connect.ts` around lines 104 - 108, Update the returned object in the Apify connection flow to derive connection.status and connectionState from the backend status.configured value instead of hardcoded 'active' and 'newly-connected' literals. Preserve the existing token assignment and ensure configured=false produces the appropriate disconnected/unconfigured result.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/commands/webscraper/apify/connect.ts`:
- Around line 93-98: Update the token-path condition in the command flow around
opts.token to detect when the --token option was supplied, including an empty
string, rather than relying on truthiness. Forward the supplied value to
storeApifyToken so backend validation rejects an empty token with its clear
error; only enter the OAuth flow when --token is absent.
---
Nitpick comments:
In `@src/commands/webscraper/apify/connect.ts`:
- Around line 104-108: Update the returned object in the Apify connection flow
to derive connection.status and connectionState from the backend
status.configured value instead of hardcoded 'active' and 'newly-connected'
literals. Preserve the existing token assignment and ensure configured=false
produces the appropriate disconnected/unconfigured result.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: c94841e4-f0ad-442e-b32d-33c5dacefb10
📒 Files selected for processing (6)
src/commands/webscraper/apify/connect.test.tssrc/commands/webscraper/apify/connect.tssrc/lib/api/oss.test.tssrc/lib/api/oss.tssrc/lib/api/webscraper.test.tssrc/lib/api/webscraper.ts
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 6 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
jwfing
left a comment
jwfing
left a comment
There was a problem hiding this comment.
Review: feat(webscraper): connect Apify with an API token on self-hosted backends
Summary: A clean, well-tested addition of a --token short-circuit path to webscraper apify connect that stores an Apify token server-side via PUT /api/webscraper/apify/config, leaving the OAuth flow untouched — no blocking issues found.
Requirements context
No matching spec/plan found — this repo keeps specs under docs/specs/ (only diagnose and db-migrations designs exist), not docs/superpowers/. Assessed against the PR description, the linked backend PR (InsForge/InsForge#1824), and repo conventions in DEVELOPMENT.md.
Findings
Critical
(none) — no correctness bugs, security holes, data-loss risks, or broken existing behavior. The apify.ts → webscraper.ts rename is clean (the only importer, connect.ts, is updated; apify-token.ts is a separate file and untouched), and the OAuth path is genuinely unchanged (the --token block short-circuits before the login-token check when the option is supplied, and falls through untouched when opts.token === undefined).
Suggestion
- Security — token supplied via argv (
src/commands/webscraper/apify/connect.ts:47,104-109): the raw token is passed on the command line, so it lands in shell history and is visible in the process list (ps aux) for the lifetime of the call. The server-side handling is exemplary (validated against Apify, only a masked value returned, never logged/printed — confirmed byconnect.test.ts:135-145), so the residual exposure is purely the input channel. This is a standard CLI tradeoff and reasonable for the agent use case, but consider also accepting the token from an env var or stdin so agents/CI aren't forced to put the secret in argv. Non-blocking. - Convention — agent-skills doc (
DEVELOPMENT.md§3): adding a user-facing--tokenflag is exactly the kind of command-surface change §3 says "almost certainly needs" a correspondinginsforge-cliupdate in theagent-skillsrepo, otherwise agents won't know the self-hosted path exists. Out of this repo's scope, but worth a follow-up so the feature is discoverable.
Information
- Functionality — merge ordering: the PR correctly notes it depends on InsForge/InsForge#1824 shipping the endpoint first. If this lands first,
connect --tokenhits the route-level 404 and surfaces the new upgrade message — no crash, just an unusable flag until the backend catches up. Already called out by the author; flagging for the merge queue. - Functionality — token path status is assumed (
connect.ts:113-117): the--tokenresult always reportsconnectionState: 'newly-connected'andconnection.status: 'active', even on an idempotent re-run where the token was already configured. Cosmetic; the JSONtokenobject carries the authoritativeconfigured/maskedKeyfrom the backend. - Functionality — slightly circular 404 guidance (
oss.ts:212-214): when the failing call isconnect --token, the new message ("…then runinsforge webscraper apify connect --token <token>") points the user back at the command they just ran. The actionable "upgrade your InsForge instance" part is correct, and the message reads well for the other caller (the bridge's/api/webscraper/apify/tokenfetch), so this is fine as-is. - SE — unchecked response cast (
webscraper.ts:183):data.tokenis cast toApifyTokenStatuswithout validatingmaskedKey. Impact is nil —configuredis explicitly checked (!== truethrowsAPIFY_TOKEN_NOT_STORED) andmaskedKey ?? '(hidden)'(connect.ts:111) covers a missing/null mask. Noting only for completeness.
Test coverage
Excellent. connect.test.ts (9 cases) pins both the --token success/error/empty/whitespace behaviors and that the OAuth path is unchanged (fast path, slow path, still-requires-login, no token field in JSON). webscraper.test.ts (8 new storeApifyToken cases) covers masked-status success, propagated backend messages (rejected token, cloud-managed), and every malformed-2xx branch (APIFY_CONFIG_MALFORMED for non-JSON/empty/null bodies, APIFY_TOKEN_NOT_STORED for configured: false). oss.test.ts pins the new 404 wording against regression. Error codes and CLIError(message, exitCode, code) arg order match src/lib/errors.ts.
Performance
No security- or performance-relevant concerns: the token path is a single PUT, no loops, no added dependencies, nothing on a hot path.
Verdict
approved (informational — no Critical findings; the human still gives the explicit GitHub approval via the approve flow). The two Suggestions and the Information notes are non-blocking.
2 participants
What
Adds
insforge webscraper apify connect --token <token>, so an Apify account can be connected on a self-hosted backend without the OAuth flow.Pairs with InsForge/InsForge#1824, which adds self-hosting support for the Web Scraper and ships the
PUT /api/webscraper/apify/configendpoint this calls. That PR should land first.Why
connecttoday only does OAuth, which is cloud-only. Agents frequently have no dashboard access — CLI is their whole surface — so without a token path a self-hosted agent cannot connect Apify on its own.Once connected, nothing else changes:
src/lib/api/apify-token.tsalready fetches from the OSS host's/api/webscraper/apify/token, so scraping, the auth bridge and theapify-clientpath all work as-is.Changes
src/lib/api/apify-config.ts(new) —storeApifyToken()PUTs to/api/webscraper/apify/config. It does not checkres.ok:ossFetchthrows aCLIErrorcarrying the backend's message on any non-2xx, so a rejected token and a cloud project ("The Apify connection is managed by InsForge Cloud.") both propagate with a useful message. Same shape as the neighbouringapify-token.ts.src/commands/webscraper/apify/connect.ts—--tokenshort-circuits after the project-linked check and before the login-token check, since the token path needs a linked project but not a login JWT. Without--tokenthe OAuth flow runs completely unchanged.src/lib/api/oss.ts— a route-level 404 under/api/webscraperused to be rewritten to "The Apify web scraper is cloud-only. Self-hosted: this feature is not supported." That is wrong once #1824 ships. It now says the backend predates the feature and points at--token.Testing
npm test→ 671 passing. New:apify-config.test.ts(4),connect.test.ts(9 — pins both the--tokenpath and that the OAuth path is unchanged), plus one case for the new 404 message. Fail-before/pass-after confirmed for each.Note
The backend endpoint is admin-authenticated through the existing
ossFetchik_key, validates the token against Apify before storing it, and returns only a masked value — the raw token is never printed or logged by the CLI.🤖 Generated with Claude Code
Summary by cubic
Adds a self-hosted connection path for the web scraper:
insforge webscraper apify connect --token <token>stores an Apify API token via the backend instead of OAuth. Also updates the/api/webscraper404 message to guide upgrades and the new token flow.New Features
--tokentoinsforge webscraper apify connectto connect Apify on self-hosted backends without OAuth.PUT /api/webscraper/apify/config; prints a masked key in text mode; JSON includestokenstatus; no login token required; the OAuth path is unchanged.storeApifyToken()andApifyTokenStatusinsrc/lib/api/webscraper.ts; moves Apify connection APIs into thewebscraperdomain module.Bug Fixes
/api/webscraper404 now suggests upgrading and using--token, not that the feature is cloud-only.--tokenlocally and improves errors for malformed responses or when the token is not stored.Written for commit dd56f97. Summary will update on new commits.
Note
Add
--tokenflag towebscraper apify connectfor self-hosted backends--token <token>option to theinsforge webscraper apify connectcommand, enabling API token-based connection without OAuth or browser login.--tokenis provided,runConnectcalls the newstoreApifyTokenfunction, which sends a PUT to/api/webscraper/apify/configand returns a masked token status./api/webscraper/*routes now surfaces an upgrade-and-token guidance message instead of a cloud-only warning.Changes since #217 opened
storeApifyTokenfunction to reject malformed API responses and unconfigured tokens [dd56f97]runConnectfunction to reject empty or whitespace-only--tokenflag values [dd56f97]apify connectcommand [dd56f97]storeApifyTokenfunction [dd56f97]Macroscope summarized 0da73eb.
Summary by CodeRabbit
New Features
Bug Fixes