-
-
Notifications
You must be signed in to change notification settings - Fork 227
[codex] Support stored preregistered OAuth secrets #1993
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 5 commits
fc4cd1d
181d642
0b83db0
a2a195d
47fcf7b
93f64be
04a4d6f
452712f
5bf9e09
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -231,6 +231,48 @@ function clearHostedCallbackRetryState() { | |
| } | ||
| } | ||
|
|
||
| const OAUTH_DEBUGGER_SECRET_PATTERNS = [ | ||
| /\b(access_token|refresh_token|id_token|client_secret|clientSecret|code_verifier|code|state)\b(\s*[:=]\s*)("[^"]*"|'[^']*'|[^\s&,;]+)/gi, | ||
| /\bBearer\s+[A-Za-z0-9._~+/=-]+\b/gi, | ||
| /\bBasic\s+[A-Za-z0-9+/=._~-]+\b/gi, | ||
| ]; | ||
|
|
||
| function sanitizeOAuthDebuggerText(value: string | null | undefined): string { | ||
| if (!value) { | ||
| return ""; | ||
| } | ||
|
|
||
| return OAUTH_DEBUGGER_SECRET_PATTERNS.reduce( | ||
| (sanitized, pattern) => | ||
| sanitized.replace(pattern, (...args) => { | ||
| const key = typeof args[1] === "string" ? args[1] : undefined; | ||
| const separator = typeof args[2] === "string" ? args[2] : undefined; | ||
| return key && separator ? `${key}${separator}[redacted]` : "[redacted]"; | ||
| }), | ||
| value, | ||
| ); | ||
| } | ||
|
|
||
| function sanitizeOAuthDebuggerError(error: Error | null) { | ||
| return { | ||
| name: sanitizeOAuthDebuggerText(error?.name ?? "Error"), | ||
| message: sanitizeOAuthDebuggerText(error?.message ?? "Unknown error"), | ||
| stack: sanitizeOAuthDebuggerText(error?.stack), | ||
| }; | ||
| } | ||
|
|
||
| function formatOAuthDebuggerErrorDetails(error: Error | null): string { | ||
| const sanitized = sanitizeOAuthDebuggerError(error); | ||
| return [ | ||
| "OAuth Debugger error", | ||
| `Name: ${sanitized.name}`, | ||
| `Message: ${sanitized.message}`, | ||
| sanitized.stack ? `Stack:\n${sanitized.stack}` : "", | ||
| ] | ||
| .filter(Boolean) | ||
| .join("\n\n"); | ||
| } | ||
|
|
||
| function BillingHandoffLoading({ overlay = false }: { overlay?: boolean }) { | ||
| return ( | ||
| <div | ||
|
|
@@ -2139,12 +2181,52 @@ export default function App() { | |
|
|
||
| {activeTab === "oauth-flow" && ( | ||
| <ErrorBoundary | ||
| fallback={ | ||
| <div className="flex items-center justify-center h-full text-muted-foreground"> | ||
| Something went wrong in the OAuth Debugger. Try refreshing the | ||
| page. | ||
| </div> | ||
| } | ||
| fallback={({ error, reset }) => { | ||
| const copyDetails = () => { | ||
| const details = formatOAuthDebuggerErrorDetails(error); | ||
| void navigator.clipboard | ||
| ?.writeText(details) | ||
| .then(() => toast.success("Copied OAuth debugger error")) | ||
| .catch(() => | ||
| toast.error("Could not copy OAuth debugger error"), | ||
| ); | ||
|
Comment on lines
+2187
to
+2192
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: # First, check if the file exists and get its basic info
git ls-files | grep -E "mcpjam-inspector/client.*App\.tsx"Repository: MCPJam/inspector Length of output: 96 🏁 Script executed: # Get the specific lines mentioned in the review (2187-2192)
if [ -f "mcpjam-inspector/client/src/App.tsx" ]; then
echo "=== Lines 2187-2192 ==="
sed -n '2185,2195p' "mcpjam-inspector/client/src/App.tsx"
fiRepository: MCPJam/inspector Length of output: 539 🏁 Script executed: # Get broader context around copyDetails function
if [ -f "mcpjam-inspector/client/src/App.tsx" ]; then
echo "=== Searching for copyDetails function ==="
grep -n "copyDetails" "mcpjam-inspector/client/src/App.tsx" | head -20
fiRepository: MCPJam/inspector Length of output: 230 Prevent unhandled errors when Clipboard API is unavailable.
🛠️ Safe async handling- const copyDetails = () => {
+ const copyDetails = async () => {
const details = formatOAuthDebuggerErrorDetails(error);
- void navigator.clipboard
- ?.writeText(details)
- .then(() => toast.success("Copied OAuth debugger error"))
- .catch(() =>
- toast.error("Could not copy OAuth debugger error"),
- );
+ try {
+ if (!navigator.clipboard?.writeText) {
+ throw new Error("Clipboard API unavailable");
+ }
+ await navigator.clipboard.writeText(details);
+ toast.success("Copied OAuth debugger error");
+ } catch {
+ toast.error("Could not copy OAuth debugger error");
+ }
};🤖 Prompt for AI Agents |
||
| }; | ||
|
|
||
| return ( | ||
| <div className="flex h-full items-center justify-center p-6"> | ||
| <div className="max-w-md space-y-4 text-center"> | ||
| <AlertTriangle className="mx-auto size-10 text-destructive" /> | ||
| <div className="space-y-2"> | ||
| <h2 className="text-lg font-semibold"> | ||
| OAuth Debugger crashed | ||
| </h2> | ||
| <p className="text-sm text-muted-foreground"> | ||
| {sanitizeOAuthDebuggerError(error).message} | ||
| </p> | ||
| </div> | ||
| <div className="flex justify-center gap-2"> | ||
| <Button variant="outline" onClick={reset}> | ||
| Try again | ||
| </Button> | ||
| <Button variant="outline" onClick={copyDetails}> | ||
| Copy details | ||
| </Button> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| ); | ||
| }} | ||
| onError={(error, errorInfo) => { | ||
| const sanitizedError = sanitizeOAuthDebuggerError(error); | ||
| posthog.capture("oauth_debugger_error_boundary", { | ||
| name: sanitizedError.name, | ||
| message: sanitizedError.message, | ||
| stack: sanitizedError.stack, | ||
| componentStack: sanitizeOAuthDebuggerText( | ||
| errorInfo.componentStack, | ||
| ), | ||
| }); | ||
| }} | ||
| > | ||
| <OAuthFlowTab | ||
| serverConfigs={appState.servers} | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When
navigator.clipboardis unavailable (e.g., insecure context or restricted browser),navigator.clipboard?.writeText(details)evaluates toundefined, and the following.then(...)access throws aTypeError. This makes the “Copy details” action crash instead of gracefully reporting that copy is unavailable.Useful? React with 👍 / 👎.