feat(oauth): multi-provider device-code flow + NocoDB read preset template#2
Open
jestyr27 wants to merge 5 commits into
Open
feat(oauth): multi-provider device-code flow + NocoDB read preset template#2jestyr27 wants to merge 5 commits into
jestyr27 wants to merge 5 commits into
Conversation
Document the plan to give OpenClaw's web_fetch a Firecrawl fallback for JS-rendered/bot-protected pages without a Nous subscription, while preserving deny-by-default egress. Uses the plugins.entries.firecrawl.config.webFetch shape (avoids the openclaw#20442 strict-schema rejection, mirroring the existing Brave web-search fix), routes through host.openshell.internal, and keeps Firecrawl's own egress allow-listed.
Commit messages, PR text, and committed artifacts attribute to the human author only — no Claude/Anthropic mentions or Co-Authored-By trailers.
Re-scoped, security-gated implementation plan for the "Forge" project/workflow assistant, synthesized from a five-discipline panel (architecture, AI engineering, NemoClaw dev, security, networking) plus an adversarial red-team. Key corrections vs the original 10-feature draft, all verified against the tree: registerCommand handlers cannot make HTTP calls and run in the gateway, not the proxy-governed sandbox (so network work must use the agent's native tools in a turn); no event/reaction hook, cron execution engine, or destructive-action gate exists; the policy schema only supports rest/websocket (SSH/IMAP/Podman-socket are inexpressible); redact() is a secret redactor, not an injection filter. Captures 6 gating decisions, feasibility tiers (A/B/C/D with cuts), a layer map, inline security controls, and a phased sequence with an empirical "start here" verification step.
Parameterize the OAuth device-flow helper with an OAuthProvider descriptor (endpoint paths, refresh style, refresh-token-required invariant, label) and add NOUS_PROVIDER (preserves current behavior) and GITHUB_PROVIDER. The same requestDeviceCode/pollForToken/refresh/runDeviceCodeFlow logic now drives a GitHub App device flow: GitHub endpoints, body-form refresh with client_secret, and access tokens without a refresh token (expiring user tokens off). Strictly additive — every resolution defaults to NOUS_PROVIDER and still honors the legacy portalBaseUrl/clientId/scope overrides, so Hermes callers are unchanged. Signed-off-by: Joseph A. Wisneski IV <stray@uhstray.io>
Add nocodb.yaml.example documenting the safe shape for the P2.1 NocoDB read feature: GET-only on the data path, no allowed_ips (public host, G-5), and no write surface. Kept as a .example so the real FQDN stays out of the fork (R2); nemoclaw-deploy owns the concrete nocodb.yaml. The template steers the deploy away from copying the wide /** multi-verb shape that jira/discord use. Adds a policies test asserting GET-only, scoped paths, and no allowed_ips. Signed-off-by: Joseph A. Wisneski IV <stray@uhstray.io>
📝 WalkthroughWalkthroughThis PR extends OAuth device-code flow to support multiple providers (GitHub and Nous), adds a NocoDB read-only policy preset template with validation, and includes three design proposal documents: fork commit-hygiene guidelines, a Firecrawl web-fetch fallback plan, and a comprehensive Forge integration strategy with gated execution phases. ChangesMulti-Provider OAuth Device Code Flow
NocoDB Read-Only Policy Preset
Design Proposals and Fork Guidelines
🎯 3 (Moderate) | ⏱️ ~25 minutes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/lib/oauth-device-code.ts`:
- Around line 455-460: The log prints provider.baseUrl even though
requestDeviceCode() may use opts.portalBaseUrl; update the log to show the
actual resolved base URL used for the request by computing the effective base
(e.g., use opts.portalBaseUrl ?? provider.baseUrl) and log that value instead of
provider.baseUrl so the prompt reflects custom hosts; modify the block around
provider, label, and the requestDeviceCode call to derive and use this
resolvedBaseUrl when calling log and leave requestDeviceCode(opts) unchanged.
- Around line 308-313: Make TokenResponse.refresh_token optional to match
device-code flows where refresh tokens aren't required: update the TokenResponse
type so refresh_token?: string and adjust any internal usage in pollForToken to
accept missing refresh_token when provider.refreshTokenRequired === false;
ensure callers that need a refresh token (e.g., Hermes/NouS) enforce a runtime
guard and throw or handle absence at their boundary rather than relying on the
type. Also update runDeviceCodeFlow logging to log the resolved portal base URL
(opts.portalBaseUrl ?? provider.baseUrl) instead of provider.baseUrl so the
logged value matches the actual request target.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 64b5e924-4dca-4b52-943f-f60751c1e08f
📒 Files selected for processing (7)
CLAUDE.mdnemoclaw-blueprint/policies/presets/nocodb.yaml.exampleproposals/firecrawl-web-fetch-provider.mdproposals/forge-integration-plan.mdsrc/lib/oauth-device-code.test.tssrc/lib/oauth-device-code.tstest/policies.test.ts
Comment on lines
+308
to
313
| const refreshRequired = provider.refreshTokenRequired !== false; | ||
| if (!payload.access_token || (refreshRequired && !payload.refresh_token)) { | ||
| throw new OAuthError( | ||
| "token_response_missing_tokens", | ||
| "portal returned no access_token or refresh_token; cannot complete host-side authorization", | ||
| "token response missing access_token (or required refresh_token); cannot complete host-side authorization", | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the file and relevant symbols
ls -la
rg -n "interface TokenResponse|type TokenResponse|TokenResponse" src/lib/oauth-device-code.ts || true
rg -n "refreshTokenRequired|refresh_token" src/lib/oauth-device-code.ts || true
rg -n "portalBaseUrl|baseUrl" src/lib/oauth-device-code.ts || true
rg -n "pollForToken\\(|runDeviceCodeFlow\\(" src/lib/oauth-device-code.ts || true
# Print the key regions with line numbers (kept small)
echo "---- TokenResponse type region ----"
nl -ba src/lib/oauth-device-code.ts | sed -n '1,220p'
echo "---- pollForToken region around 308-330 ----"
nl -ba src/lib/oauth-device-code.ts | sed -n '260,360p'
echo "---- runDeviceCodeFlow region around 430-520 ----"
nl -ba src/lib/oauth-device-code.ts | sed -n '420,520p'
# Check hermes-provider-auth mention
rg -n "NOUS_PROVIDER|refresh_token|hermes-provider-auth" src/lib || true
ls -la src/lib | head
test -f src/lib/hermes-provider-auth.ts && nl -ba src/lib/hermes-provider-auth.ts | sed -n '80,180p' || trueRepository: uhstray-io/NemoClaw
Length of output: 6078
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Basic sanity
ls -la src/lib/oauth-device-code.ts src/lib/hermes-provider-auth.ts
echo "---- TokenResponse definition (src/lib/oauth-device-code.ts) ----"
sed -n '80,130p' src/lib/oauth-device-code.ts
echo "---- refreshTokenRequired and related options (src/lib/oauth-device-code.ts) ----"
sed -n '40,110p' src/lib/oauth-device-code.ts
echo "---- pollForToken logic around 300-420 (src/lib/oauth-device-code.ts) ----"
sed -n '250,430p' src/lib/oauth-device-code.ts
echo "---- runDeviceCodeFlow logic around 420-520 (src/lib/oauth-device-code.ts) ----"
sed -n '430,520p' src/lib/oauth-device-code.ts
echo "---- hermes-provider-auth usage around 112-160 (src/lib/hermes-provider-auth.ts) ----"
sed -n '90,180p' src/lib/hermes-provider-auth.ts
echo "---- Search for refresh_token usage across src/lib (limit output) ----"
rg -n "refresh_token" src/lib --glob "*.ts" | head -n 50Repository: uhstray-io/NemoClaw
Length of output: 16850
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect tests around “accepts token response without refresh_token”
rg -n "accepts a token response without a refresh_token|expiring tokens off|refreshTokenRequired" src/lib/oauth-device-code.test.ts
sed -n '130,260p' src/lib/oauth-device-code.test.ts
# Inspect Hermes provider auth tests around tokens.refresh_token usage
rg -n "tokens\\.refresh_token|registerHermesToolGatewayRefreshProvider" src/lib/hermes-provider-auth.test.ts
sed -n '80,180p' src/lib/hermes-provider-auth.test.ts
sed -n '180,260p' src/lib/hermes-provider-auth.test.ts
# Confirm requestDeviceCode uses portalBaseUrl override like pollForToken
rg -n "function requestDeviceCode|portalBaseUrl|opts\\.portalBaseUrl" src/lib/oauth-device-code.ts
sed -n '180,260p' src/lib/oauth-device-code.tsRepository: uhstray-io/NemoClaw
Length of output: 13686
Make TokenResponse.refresh_token optional for device-code responses where refresh tokens aren’t required.
When using GITHUB_PROVIDER (refreshTokenRequired: false), pollForToken accepts 200 responses without refresh_token and returns success, while TokenResponse still types refresh_token as a required string, making the exported API unsound and inconsistent with existing tests (token.refresh_token is asserted undefined).
Minimal fix
export interface TokenResponse {
access_token: string;
- refresh_token: string;
+ refresh_token?: string;
expires_in: number;
token_type: string;
scope?: string;
iat?: number;
exp?: number;
}Then add an explicit guard/requirement at the consumer boundary where a refresh token is mandatory (e.g., Hermes/NouS), instead of relying on the type.
- Minor:
runDeviceCodeFlowlogsprovider.baseUrl, but requests useopts.portalBaseUrl ?? provider.baseUrl; log the resolved value for clarity.
🤖 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/lib/oauth-device-code.ts` around lines 308 - 313, Make
TokenResponse.refresh_token optional to match device-code flows where refresh
tokens aren't required: update the TokenResponse type so refresh_token?: string
and adjust any internal usage in pollForToken to accept missing refresh_token
when provider.refreshTokenRequired === false; ensure callers that need a refresh
token (e.g., Hermes/NouS) enforce a runtime guard and throw or handle absence at
their boundary rather than relying on the type. Also update runDeviceCodeFlow
logging to log the resolved portal base URL (opts.portalBaseUrl ??
provider.baseUrl) instead of provider.baseUrl so the logged value matches the
actual request target.
Comment on lines
+455
to
460
| const provider = opts.provider ?? NOUS_PROVIDER; | ||
| const label = provider.label ?? "OAuth"; | ||
|
|
||
| log(""); | ||
| log(" Requesting device code from portal.nousresearch.com..."); | ||
| log(` Requesting device code from ${provider.baseUrl}...`); | ||
| const deviceCode = await requestDeviceCode(opts); |
There was a problem hiding this comment.
Use the resolved base URL in the log message.
requestDeviceCode() honors opts.portalBaseUrl, but this log still prints provider.baseUrl. Custom hosts will show the wrong endpoint in the prompt.
Suggested fix
export async function runDeviceCodeFlow(
opts: DeviceCodeFlowOptions = {},
): Promise<TokenResponse> {
const log = opts.log ?? ((line: string) => console.error(line));
const provider = opts.provider ?? NOUS_PROVIDER;
const label = provider.label ?? "OAuth";
+ const baseUrl = opts.portalBaseUrl ?? provider.baseUrl;
log("");
- log(` Requesting device code from ${provider.baseUrl}...`);
+ log(` Requesting device code from ${baseUrl}...`);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const provider = opts.provider ?? NOUS_PROVIDER; | |
| const label = provider.label ?? "OAuth"; | |
| log(""); | |
| log(" Requesting device code from portal.nousresearch.com..."); | |
| log(` Requesting device code from ${provider.baseUrl}...`); | |
| const deviceCode = await requestDeviceCode(opts); | |
| const provider = opts.provider ?? NOUS_PROVIDER; | |
| const label = provider.label ?? "OAuth"; | |
| const baseUrl = opts.portalBaseUrl ?? provider.baseUrl; | |
| log(""); | |
| log(` Requesting device code from ${baseUrl}...`); | |
| const deviceCode = await requestDeviceCode(opts); |
🤖 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/lib/oauth-device-code.ts` around lines 455 - 460, The log prints
provider.baseUrl even though requestDeviceCode() may use opts.portalBaseUrl;
update the log to show the actual resolved base URL used for the request by
computing the effective base (e.g., use opts.portalBaseUrl ?? provider.baseUrl)
and log that value instead of provider.baseUrl so the prompt reflects custom
hosts; modify the block around provider, label, and the requestDeviceCode call
to derive and use this resolvedBaseUrl when calling log and leave
requestDeviceCode(opts) unchanged.
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
Generalize the OAuth device-flow helper so the same flow drives both the Nous portal and a GitHub App sign-on, and add a read-only NocoDB preset template. Groundwork for the Forge assistant's GitHub
/status(interactive GitHub sign-on) and NocoDB read features.Changes
src/lib/oauth-device-code.ts— introduce anOAuthProviderdescriptor (endpoint paths, refresh style, refresh-token-required invariant, label) withNOUS_PROVIDER(defaults preserve current behavior) andGITHUB_PROVIDER.requestDeviceCode/pollForToken/refreshAccessTokenWithRefreshToken/runDeviceCodeFloware now provider-parameterized. Strictly additive — every resolution defaults toNOUS_PROVIDERand still honors the legacyportalBaseUrl/clientId/scopeoverrides, so Hermes callers are unchanged.src/lib/oauth-device-code.test.ts— add GitHub-provider tests (device-code endpoint, refresh-token-optional poll, body-form refresh withclient_secret); existing Nous tests unchanged.nemoclaw-blueprint/policies/presets/nocodb.yaml.example— read-only NocoDB preset template: GET-only on the data path, noallowed_ips(public host), no write surface. Kept as.exampleso the real FQDN stays out of the fork; the deploy repo owns the concrete preset. Includes apolicies.test.tsassertion of the read-only shape.Type of Change
Verification
npm run typecheck:cliclean (no consumer breakage); Biome clean on changed filesnpx prek run --all-files—test-cliruns the full CLI project including env-gated e2e/provisioning scenarios that fail off-Linux; relevant suites run directly and pass. CI runs the full suite authoritatively.Signed-off-by: Joseph A. Wisneski IV stray@uhstray.io
Summary by CodeRabbit
New Features
Documentation
Tests