Implement OAuth login #40
Conversation
zmrlft
force-pushed
the
main
branch
2 times, most recently
from
e549a5a to
e7a793e
Compare
|
@cubic-dev-ai review this PR |
@zmrlft I've started the AI code review. It'll take a few minutes to complete. |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
8 issues found across 25 files
Prompt for AI agents (all 8 issues)
Understand the root cause of the following 8 issues and fix them.
<file name="README.md">
<violation number="1" location="README.md:28">
Instructing users to run `spctl --master-disable` disables macOS Gatekeeper globally, leaving the system unprotected after your app starts. This step should be removed or replaced with a per-app workaround so users don’t permanently weaken their security.</violation>
</file>
<file name="README_zh.md">
<violation number="1" location="README_zh.md:32">
Avoid recommending `sudo spctl --master-disable`; it disables macOS Gatekeeper system-wide and leaves users exposed. Please remove or replace this step with a safer alternative.</violation>
</file>
<file name="frontend/src/utils/auth.ts">
<violation number="1" location="frontend/src/utils/auth.ts:21">
Persisting the OAuth access token in localStorage leaves it exposed to any XSS payload that can read from storage; please move the token to a secure channel (e.g., httpOnly cookie or in-memory/session storage guarded by the backend).</violation>
</file>
<file name="frontend/src/components/ContributionCalendar.tsx">
<violation number="1" location="frontend/src/components/ContributionCalendar.tsx:124">
Switching to a GitHub account without a public email leaves the previous account's email in state, so we end up generating repos with stale user data. Please default githubEmail to an empty string whenever the new githubUser lacks an email (e.g. assign `githubUser.email ?? ''` without guarding on truthiness).</violation>
</file>
<file name="config.go">
<violation number="1" location="config.go:137">
GetConfig drops the Load error, so unreadable or invalid config files surface as nil configs without the original failure details. Propagate the Load error instead of discarding it.</violation>
</file>
<file name="docs/CI_CD_SETUP.md">
<violation number="1" location="docs/CI_CD_SETUP.md:57">
The new link points to SECURITY_CONFIG.md, but that file does not exist in the repo, so the documentation link is broken. Please update the link target (and the English section below) to a valid path or add the missing document.</violation>
<violation number="2" location="docs/CI_CD_SETUP.md:58">
The Quick Start link references QUICK_START_SECURITY.md, but that file is not present in the repository, leaving the link broken. Please correct the target (and mirror the fix in the English section) or add the intended document.</violation>
</file>
<file name="frontend/src/i18n.tsx">
<violation number="1" location="frontend/src/i18n.tsx:230">
Removing the characterSelector.previewTooltip entry breaks the existing tooltip lookup in ContributionCalendar, so users will now see the raw key instead of localized text. Please keep the previewTooltip translation (or update the consumer accordingly).</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Ask questions if you need clarification on any suggestion
React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.
| ```bash | ||
| cd the-directory-where-green-wall.app-is-located | ||
| sudo xattr -cr ./green-wall.app | ||
| sudo spctl --master-disable |
There was a problem hiding this comment.
Instructing users to run spctl --master-disable disables macOS Gatekeeper globally, leaving the system unprotected after your app starts. This step should be removed or replaced with a per-app workaround so users don’t permanently weaken their security.
Prompt for AI agents
Address the following comment on README.md at line 28:
<comment>Instructing users to run `spctl --master-disable` disables macOS Gatekeeper globally, leaving the system unprotected after your app starts. This step should be removed or replaced with a per-app workaround so users don’t permanently weaken their security.</comment>
<file context>
@@ -6,10 +6,35 @@
+```bash
+cd the-directory-where-green-wall.app-is-located
+sudo xattr -cr ./green-wall.app
+sudo spctl --master-disable
+sudo xattr -r -d com.apple.quarantine ./green-wall.app
+```
</file context>
| ```bash | ||
| cd 你的green-wall.app存在的目录 | ||
| sudo xattr -cr ./green-wall.app | ||
| sudo spctl --master-disable |
There was a problem hiding this comment.
Avoid recommending sudo spctl --master-disable; it disables macOS Gatekeeper system-wide and leaves users exposed. Please remove or replace this step with a safer alternative.
Prompt for AI agents
Address the following comment on README_zh.md at line 32:
<comment>Avoid recommending `sudo spctl --master-disable`; it disables macOS Gatekeeper system-wide and leaves users exposed. Please remove or replace this step with a safer alternative.</comment>
<file context>
@@ -6,10 +6,39 @@
+```bash
+cd 你的green-wall.app存在的目录
+sudo xattr -cr ./green-wall.app
+sudo spctl --master-disable
+sudo xattr -r -d com.apple.quarantine ./green-wall.app
+```
</file context>
| */ | ||
| export function saveAuthState(state: AuthState): void { | ||
| try { | ||
| localStorage.setItem(STORAGE_KEYS.accessToken, state.accessToken); |
There was a problem hiding this comment.
Persisting the OAuth access token in localStorage leaves it exposed to any XSS payload that can read from storage; please move the token to a secure channel (e.g., httpOnly cookie or in-memory/session storage guarded by the backend).
Prompt for AI agents
Address the following comment on frontend/src/utils/auth.ts at line 21:
<comment>Persisting the OAuth access token in localStorage leaves it exposed to any XSS payload that can read from storage; please move the token to a secure channel (e.g., httpOnly cookie or in-memory/session storage guarded by the backend).</comment>
<file context>
@@ -0,0 +1,65 @@
+ */
+export function saveAuthState(state: AuthState): void {
+ try {
+ localStorage.setItem(STORAGE_KEYS.accessToken, state.accessToken);
+ localStorage.setItem(STORAGE_KEYS.user, JSON.stringify(state.user));
+ } catch (error) {
</file context>
| if (githubUser) { | ||
| setGithubUsername(githubUser.login); | ||
| if (githubUser.email) { | ||
| setGithubEmail(githubUser.email); |
There was a problem hiding this comment.
Switching to a GitHub account without a public email leaves the previous account's email in state, so we end up generating repos with stale user data. Please default githubEmail to an empty string whenever the new githubUser lacks an email (e.g. assign githubUser.email ?? '' without guarding on truthiness).
Prompt for AI agents
Address the following comment on frontend/src/components/ContributionCalendar.tsx at line 124:
<comment>Switching to a GitHub account without a public email leaves the previous account's email in state, so we end up generating repos with stale user data. Please default githubEmail to an empty string whenever the new githubUser lacks an email (e.g. assign `githubUser.email ?? ''` without guarding on truthiness).</comment>
<file context>
@@ -113,6 +116,21 @@ function ContributionCalendar({ contributions: originalContributions, className,
+ if (githubUser) {
+ setGithubUsername(githubUser.login);
+ if (githubUser.email) {
+ setGithubEmail(githubUser.email);
+ }
+ // 保存到 localStorage
</file context>
| // GetConfig 获取当前配置 | ||
| func (cm *ConfigManager) GetConfig() *Config { | ||
| if cm.config == nil { | ||
| config, _ := cm.Load() |
There was a problem hiding this comment.
GetConfig drops the Load error, so unreadable or invalid config files surface as nil configs without the original failure details. Propagate the Load error instead of discarding it.
Prompt for AI agents
Address the following comment on config.go at line 137:
<comment>GetConfig drops the Load error, so unreadable or invalid config files surface as nil configs without the original failure details. Propagate the Load error instead of discarding it.</comment>
<file context>
@@ -0,0 +1,190 @@
+// GetConfig 获取当前配置
+func (cm *ConfigManager) GetConfig() *Config {
+ if cm.config == nil {
+ config, _ := cm.Load()
+ return config
+ }
</file context>
| ## 参考文档 | ||
|
|
||
| - [安全配置指南](SECURITY_CONFIG.md) | ||
| - [快速开始](QUICK_START_SECURITY.md) |
There was a problem hiding this comment.
The Quick Start link references QUICK_START_SECURITY.md, but that file is not present in the repository, leaving the link broken. Please correct the target (and mirror the fix in the English section) or add the intended document.
Prompt for AI agents
Address the following comment on docs/CI_CD_SETUP.md at line 58:
<comment>The Quick Start link references QUICK_START_SECURITY.md, but that file is not present in the repository, leaving the link broken. Please correct the target (and mirror the fix in the English section) or add the intended document.</comment>
<file context>
@@ -0,0 +1,119 @@
+## 参考文档
+
+- [安全配置指南](SECURITY_CONFIG.md)
+- [快速开始](QUICK_START_SECURITY.md)
+
+---
</file context>
|
|
||
| ## 参考文档 | ||
|
|
||
| - [安全配置指南](SECURITY_CONFIG.md) |
There was a problem hiding this comment.
The new link points to SECURITY_CONFIG.md, but that file does not exist in the repo, so the documentation link is broken. Please update the link target (and the English section below) to a valid path or add the missing document.
Prompt for AI agents
Address the following comment on docs/CI_CD_SETUP.md at line 57:
<comment>The new link points to SECURITY_CONFIG.md, but that file does not exist in the repo, so the documentation link is broken. Please update the link target (and the English section below) to a valid path or add the missing document.</comment>
<file context>
@@ -0,0 +1,119 @@
+
+## 参考文档
+
+- [安全配置指南](SECURITY_CONFIG.md)
+- [快速开始](QUICK_START_SECURITY.md)
+
</file context>
| tabSymbols: '🎨 Symbols', | ||
| previewTooltip: 'Preview character: {{char}}', | ||
| characterTool: 'Character Tool', | ||
| preview: 'Preview', |
There was a problem hiding this comment.
Removing the characterSelector.previewTooltip entry breaks the existing tooltip lookup in ContributionCalendar, so users will now see the raw key instead of localized text. Please keep the previewTooltip translation (or update the consumer accordingly).
Prompt for AI agents
Address the following comment on frontend/src/i18n.tsx at line 230:
<comment>Removing the characterSelector.previewTooltip entry breaks the existing tooltip lookup in ContributionCalendar, so users will now see the raw key instead of localized text. Please keep the previewTooltip translation (or update the consumer accordingly).</comment>
<file context>
@@ -224,15 +227,18 @@ const translations: Record<Language, TranslationDict> = {
- tabSymbols: '🎨 Symbols',
- previewTooltip: 'Preview character: {{char}}',
- characterTool: 'Character Tool',
+ preview: 'Preview',
cancelPreview: 'Cancel Preview',
character: 'Character',
</file context>
3 participants
No description provided.