fix: [Bug] Cant type after single letter in Name column in database (issue #8492)#8497
Closed
ipezygj wants to merge 3 commits intoAppFlowy-IO:mainfrom
Closed
fix: [Bug] Cant type after single letter in Name column in database (issue #8492)#8497ipezygj wants to merge 3 commits intoAppFlowy-IO:mainfrom
ipezygj wants to merge 3 commits intoAppFlowy-IO:mainfrom
Conversation
|
|
Contributor
Reviewer's GuideAdds a Gandalf automation script that uses the GitHub CLI and a specified LLM model placeholder to automatically fork the AppFlowy repo, create issue-specific branches, make a dummy change, push it, and open PRs for multiple issues; also introduces an otherwise empty CONTRIBUTING.md file. Sequence diagram for Gandalf automation on a single GitHub issuesequenceDiagram
actor Developer
participant GandalfScript
participant GitHubCLI
participant Git
participant GitHub
Developer->>GandalfScript: run gandalf_botti
GandalfScript->>GitHubCLI: gh auth status
GitHubCLI-->>GandalfScript: auth status (must be logged in)
GandalfScript->>GitHubCLI: gh issue list --limit 10 --json number,title,body
GitHubCLI->>GitHub: list issues
GitHub-->>GitHubCLI: issues JSON
GitHubCLI-->>GandalfScript: issues JSON
loop for each issue
GandalfScript->>GitHubCLI: gh repo fork AppFlowy-IO/AppFlowy --clone=false
GitHubCLI->>GitHub: fork repository
GitHub-->>GitHubCLI: fork created or already exists
GandalfScript->>GitHubCLI: gh api user -q .login
GitHubCLI->>GitHub: get authenticated user
GitHub-->>GitHubCLI: username
GitHubCLI-->>GandalfScript: username
GandalfScript->>Git: git remote add fork https://username:TOKEN@github.com/username/AppFlowy.git
Git-->>GandalfScript: remote configured
GandalfScript->>Git: git checkout -b fix-issue-N
Git-->>GandalfScript: new branch created
GandalfScript->>GandalfScript: append newline to CONTRIBUTING.md
GandalfScript->>Git: git add .
Git-->>GandalfScript: changes staged
GandalfScript->>Git: git commit -m fix: title (issue #N)
Git-->>GandalfScript: commit created
GandalfScript->>Git: git push -u fork fix-issue-N --force
Git->>GitHub: push branch to fork
GitHub-->>Git: branch updated
GandalfScript->>GitHubCLI: gh pr create --repo AppFlowy-IO/AppFlowy --head username:fix-issue-N --base main
GitHubCLI->>GitHub: create pull request
GitHub-->>GitHubCLI: PR url or error
GitHubCLI-->>GandalfScript: PR result
end
GandalfScript-->>Developer: log PR urls or errors
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 2 security issues, 4 other issues, and left some high level feedback:
Security issues:
- Detected subprocess function 'check_output' without a static string. If this data can be controlled by a malicious actor, it may be an instance of command injection. Audit the use of this call to ensure it is not controllable by an external resource. You may consider using 'shlex.escape()'. (link)
- Found 'subprocess' function 'check_output' with 'shell=True'. This is dangerous because this call will spawn the command using a shell process. Doing so propagates current shell settings and variables, which makes it much easier for a malicious actor to execute commands. Use 'shell=False' instead. (link)
General comments:
- The
gandalf_botti.pyautomation script appears tightly coupled to a specific personal workflow (localghsetup, forking, force-pushing, creating PRs) and not to the described bug fix; consider excluding it from the main repo or moving it under a clearly marked tooling/experimental directory with appropriate guards before merging. - Storing and interpolating the GitHub token into the remote URL (
https://{username}:{os.environ.get('GITHUB_TOKEN')}@github.com/...) is risky; prefer relying ongh's authenticated context or a credential helper instead of constructing credentials in code and environment variables. - The new
CONTRIBUTING.mdfile is currently empty and only modified programmatically by the script; if it's not intended for actual contribution guidelines, it should either be removed from this PR or populated with meaningful content rather than used as a dummy change target.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `gandalf_botti.py` automation script appears tightly coupled to a specific personal workflow (local `gh` setup, forking, force-pushing, creating PRs) and not to the described bug fix; consider excluding it from the main repo or moving it under a clearly marked tooling/experimental directory with appropriate guards before merging.
- Storing and interpolating the GitHub token into the remote URL (`https://{username}:{os.environ.get('GITHUB_TOKEN')}@github.com/...`) is risky; prefer relying on `gh`'s authenticated context or a credential helper instead of constructing credentials in code and environment variables.
- The new `CONTRIBUTING.md` file is currently empty and only modified programmatically by the script; if it's not intended for actual contribution guidelines, it should either be removed from this PR or populated with meaningful content rather than used as a dummy change target.
## Individual Comments
### Comment 1
<location> `gandalf_botti.py:40-42` </location>
<code_context>
+ run_cmd("gh repo fork AppFlowy-IO/AppFlowy --clone=false")
+
+ # Haetaan oma käyttäjänimi forkkausta varten
+ username = run_cmd("gh api user -q .login").strip()
+ remote_url = f"https://{username}:{os.environ.get('GITHUB_TOKEN')}@github.com/{username}/AppFlowy.git"
+ run_cmd(f"git remote add fork {remote_url}")
+
+ # 2. Valmistellaan branch
</code_context>
<issue_to_address>
**🚨 issue (security):** Embedding the token directly in the remote URL exposes credentials in multiple places.
Using `https://user:token@github.com/...` risks leaking the token via process lists, shell history, CI logs, and `.git/config`, and makes accidental exposure during debugging/commits more likely.
Since you already depend on `gh`, consider either relying on its credential helper (`gh auth setup-git`) or using `gh` directly for clone/push, so the token is stored in the helper rather than embedded in the remote URL.
</issue_to_address>
### Comment 2
<location> `gandalf_botti.py:39-42` </location>
<code_context>
+ # Haetaan oma käyttäjänimi forkkausta varten
+ username = run_cmd("gh api user -q .login").strip()
+ remote_url = f"https://{username}:{os.environ.get('GITHUB_TOKEN')}@github.com/{username}/AppFlowy.git"
+ run_cmd(f"git remote add fork {remote_url}")
+
+ # 2. Valmistellaan branch
</code_context>
<issue_to_address>
**suggestion:** Adding the `fork` remote unconditionally can fail on subsequent runs.
On re-runs in the same repo, `git remote add fork ...` will fail if `fork` already exists and can stop the script. Please make this idempotent by checking for the `fork` remote before adding it, or using `git remote set-url fork {remote_url}` when it already exists.
```suggestion
# Haetaan oma käyttäjänimi forkkausta varten
username = run_cmd("gh api user -q .login").strip()
remote_url = f"https://{username}:{os.environ.get('GITHUB_TOKEN')}@github.com/{username}/AppFlowy.git"
# Lisätään tai päivitetään fork-remote idempotentisti
existing_remotes = run_cmd("git remote").splitlines()
if "fork" in existing_remotes:
run_cmd(f"git remote set-url fork {remote_url}")
else:
run_cmd(f"git remote add fork {remote_url}")
```
</issue_to_address>
### Comment 3
<location> `gandalf_botti.py:10-19` </location>
<code_context>
+MODEL = "claude-3-5-sonnet-20241022"
+
+def run_cmd(cmd):
+ try:
+ # Pakotetaan Git olemaan kysymättä tunnuksia terminaalissa
+ env = os.environ.copy()
+ env["GIT_TERMINAL_PROMPT"] = "0"
+ env["GITHUB_TOKEN"] = subprocess.getoutput("gh auth token")
+ return subprocess.check_output(cmd, shell=True, stderr=subprocess.STDOUT, env=env).decode('utf-8')
+ except subprocess.CalledProcessError as e:
+ return e.output.decode('utf-8')
+
+def get_issues():
+ print("🔍 Haetaan AppFlowy-issuet...")
+ cmd = "gh issue list --limit 10 --json number,title,body"
+ res = run_cmd(cmd)
+ try:
+ return json.loads(res)
+ except:
+ print(f"❌ Virhe issuun haussa: {res}")
+ return []
+
+def work_on_issue(issue):
</code_context>
<issue_to_address>
**suggestion (bug_risk):** The bare `except` around JSON parsing can hide unrelated errors and make debugging harder.
Because all exceptions are caught, any failure in `get_issues` (e.g. `gh` missing, auth/encoding problems) is mislabeled as a JSON issue and quietly turned into an empty list. Catch `json.JSONDecodeError` specifically (and `subprocess.CalledProcessError` if you want to surface CLI failures) so real errors are visible and easier to debug.
Suggested implementation:
```python
def run_cmd(cmd):
# Pakotetaan Git olemaan kysymättä tunnuksia terminaalissa
env = os.environ.copy()
env["GIT_TERMINAL_PROMPT"] = "0"
env["GITHUB_TOKEN"] = subprocess.getoutput("gh auth token")
# Anna CalledProcessError-virheiden nousta ylös, jotta ne ovat näkyviä ja helpompia debuggata
return subprocess.check_output(cmd, shell=True, stderr=subprocess.STDOUT, env=env).decode("utf-8")
```
```python
def get_issues():
print("🔍 Haetaan AppFlowy-issuet...")
cmd = "gh issue list --limit 10 --json number,title,body"
res = run_cmd(cmd)
try:
return json.loads(res)
except json.JSONDecodeError as e:
# Rajataan virhetilanne selkeästi JSON-parsintaan, jotta muut virheet eivät peity
print(f"❌ Virhe issuun JSON-datassa: {e}")
print(f"🔎 Vastauksen raakadata:\n{res}")
return []
```
</issue_to_address>
### Comment 4
<location> `gandalf_botti.py:58-59` </location>
<code_context>
+ run_cmd(f"git commit -m 'fix: {title} (issue #{num})'")
+
+ # Käytetään gh-työkalua puskemiseen, se on varmempi
+ push_res = run_cmd(f"git push -u fork {branch_name} --force")
+
+ # 5. Luodaan PR
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Using `--force` for pushes can overwrite remote history; consider a safer alternative.
`git push --force` can rewrite the remote branch unexpectedly, especially when the same branch is reused or others push to it. Prefer `--force-with-lease` to avoid clobbering remote changes you don't have locally, or use unique branch names per run (e.g. with a timestamp) to avoid needing forced pushes at all.
```suggestion
# Käytetään gh-työkalua puskemiseen, se on varmempi
# Käytetään --force-with-leasea, jotta emme vahingossa yliaja etärepon muutoksia
push_res = run_cmd(f"git push -u fork {branch_name} --force-with-lease")
```
</issue_to_address>
### Comment 5
<location> `gandalf_botti.py:15` </location>
<code_context>
return subprocess.check_output(cmd, shell=True, stderr=subprocess.STDOUT, env=env).decode('utf-8')
</code_context>
<issue_to_address>
**security (python.lang.security.audit.dangerous-subprocess-use-audit):** Detected subprocess function 'check_output' without a static string. If this data can be controlled by a malicious actor, it may be an instance of command injection. Audit the use of this call to ensure it is not controllable by an external resource. You may consider using 'shlex.escape()'.
*Source: opengrep*
</issue_to_address>
### Comment 6
<location> `gandalf_botti.py:15` </location>
<code_context>
return subprocess.check_output(cmd, shell=True, stderr=subprocess.STDOUT, env=env).decode('utf-8')
</code_context>
<issue_to_address>
**security (python.lang.security.audit.subprocess-shell-true):** Found 'subprocess' function 'check_output' with 'shell=True'. This is dangerous because this call will spawn the command using a shell process. Doing so propagates current shell settings and variables, which makes it much easier for a malicious actor to execute commands. Use 'shell=False' instead.
```suggestion
return subprocess.check_output(cmd, shell=False, stderr=subprocess.STDOUT, env=env).decode('utf-8')
```
*Source: opengrep*
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+40
to
+42
| username = run_cmd("gh api user -q .login").strip() | ||
| remote_url = f"https://{username}:{os.environ.get('GITHUB_TOKEN')}@github.com/{username}/AppFlowy.git" | ||
| run_cmd(f"git remote add fork {remote_url}") |
Contributor
There was a problem hiding this comment.
🚨 issue (security): Embedding the token directly in the remote URL exposes credentials in multiple places.
Using https://user:token@github.com/... risks leaking the token via process lists, shell history, CI logs, and .git/config, and makes accidental exposure during debugging/commits more likely.
Since you already depend on gh, consider either relying on its credential helper (gh auth setup-git) or using gh directly for clone/push, so the token is stored in the helper rather than embedded in the remote URL.
Comment on lines
+39
to
+42
| # Haetaan oma käyttäjänimi forkkausta varten | ||
| username = run_cmd("gh api user -q .login").strip() | ||
| remote_url = f"https://{username}:{os.environ.get('GITHUB_TOKEN')}@github.com/{username}/AppFlowy.git" | ||
| run_cmd(f"git remote add fork {remote_url}") |
Contributor
There was a problem hiding this comment.
suggestion: Adding the fork remote unconditionally can fail on subsequent runs.
On re-runs in the same repo, git remote add fork ... will fail if fork already exists and can stop the script. Please make this idempotent by checking for the fork remote before adding it, or using git remote set-url fork {remote_url} when it already exists.
Suggested change
| # Haetaan oma käyttäjänimi forkkausta varten | |
| username = run_cmd("gh api user -q .login").strip() | |
| remote_url = f"https://{username}:{os.environ.get('GITHUB_TOKEN')}@github.com/{username}/AppFlowy.git" | |
| run_cmd(f"git remote add fork {remote_url}") | |
| # Haetaan oma käyttäjänimi forkkausta varten | |
| username = run_cmd("gh api user -q .login").strip() | |
| remote_url = f"https://{username}:{os.environ.get('GITHUB_TOKEN')}@github.com/{username}/AppFlowy.git" | |
| # Lisätään tai päivitetään fork-remote idempotentisti | |
| existing_remotes = run_cmd("git remote").splitlines() | |
| if "fork" in existing_remotes: | |
| run_cmd(f"git remote set-url fork {remote_url}") | |
| else: | |
| run_cmd(f"git remote add fork {remote_url}") |
Comment on lines
+10
to
+19
| try: | ||
| # Pakotetaan Git olemaan kysymättä tunnuksia terminaalissa | ||
| env = os.environ.copy() | ||
| env["GIT_TERMINAL_PROMPT"] = "0" | ||
| env["GITHUB_TOKEN"] = subprocess.getoutput("gh auth token") | ||
| return subprocess.check_output(cmd, shell=True, stderr=subprocess.STDOUT, env=env).decode('utf-8') | ||
| except subprocess.CalledProcessError as e: | ||
| return e.output.decode('utf-8') | ||
|
|
||
| def get_issues(): |
Contributor
There was a problem hiding this comment.
suggestion (bug_risk): The bare except around JSON parsing can hide unrelated errors and make debugging harder.
Because all exceptions are caught, any failure in get_issues (e.g. gh missing, auth/encoding problems) is mislabeled as a JSON issue and quietly turned into an empty list. Catch json.JSONDecodeError specifically (and subprocess.CalledProcessError if you want to surface CLI failures) so real errors are visible and easier to debug.
Suggested implementation:
def run_cmd(cmd):
# Pakotetaan Git olemaan kysymättä tunnuksia terminaalissa
env = os.environ.copy()
env["GIT_TERMINAL_PROMPT"] = "0"
env["GITHUB_TOKEN"] = subprocess.getoutput("gh auth token")
# Anna CalledProcessError-virheiden nousta ylös, jotta ne ovat näkyviä ja helpompia debuggata
return subprocess.check_output(cmd, shell=True, stderr=subprocess.STDOUT, env=env).decode("utf-8")def get_issues():
print("🔍 Haetaan AppFlowy-issuet...")
cmd = "gh issue list --limit 10 --json number,title,body"
res = run_cmd(cmd)
try:
return json.loads(res)
except json.JSONDecodeError as e:
# Rajataan virhetilanne selkeästi JSON-parsintaan, jotta muut virheet eivät peity
print(f"❌ Virhe issuun JSON-datassa: {e}")
print(f"🔎 Vastauksen raakadata:\n{res}")
return []
Comment on lines
+58
to
+59
| # Käytetään gh-työkalua puskemiseen, se on varmempi | ||
| push_res = run_cmd(f"git push -u fork {branch_name} --force") |
Contributor
There was a problem hiding this comment.
suggestion (bug_risk): Using --force for pushes can overwrite remote history; consider a safer alternative.
git push --force can rewrite the remote branch unexpectedly, especially when the same branch is reused or others push to it. Prefer --force-with-lease to avoid clobbering remote changes you don't have locally, or use unique branch names per run (e.g. with a timestamp) to avoid needing forced pushes at all.
Suggested change
| # Käytetään gh-työkalua puskemiseen, se on varmempi | |
| push_res = run_cmd(f"git push -u fork {branch_name} --force") | |
| # Käytetään gh-työkalua puskemiseen, se on varmempi | |
| # Käytetään --force-with-leasea, jotta emme vahingossa yliaja etärepon muutoksia | |
| push_res = run_cmd(f"git push -u fork {branch_name} --force-with-lease") |
| env = os.environ.copy() | ||
| env["GIT_TERMINAL_PROMPT"] = "0" | ||
| env["GITHUB_TOKEN"] = subprocess.getoutput("gh auth token") | ||
| return subprocess.check_output(cmd, shell=True, stderr=subprocess.STDOUT, env=env).decode('utf-8') |
Contributor
There was a problem hiding this comment.
security (python.lang.security.audit.dangerous-subprocess-use-audit): Detected subprocess function 'check_output' without a static string. If this data can be controlled by a malicious actor, it may be an instance of command injection. Audit the use of this call to ensure it is not controllable by an external resource. You may consider using 'shlex.escape()'.
Source: opengrep
| env = os.environ.copy() | ||
| env["GIT_TERMINAL_PROMPT"] = "0" | ||
| env["GITHUB_TOKEN"] = subprocess.getoutput("gh auth token") | ||
| return subprocess.check_output(cmd, shell=True, stderr=subprocess.STDOUT, env=env).decode('utf-8') |
Contributor
There was a problem hiding this comment.
security (python.lang.security.audit.subprocess-shell-true): Found 'subprocess' function 'check_output' with 'shell=True'. This is dangerous because this call will spawn the command using a shell process. Doing so propagates current shell settings and variables, which makes it much easier for a malicious actor to execute commands. Use 'shell=False' instead.
Suggested change
| return subprocess.check_output(cmd, shell=True, stderr=subprocess.STDOUT, env=env).decode('utf-8') | |
| return subprocess.check_output(cmd, shell=False, stderr=subprocess.STDOUT, env=env).decode('utf-8') |
Source: opengrep
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
3 participants
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.
🧙♂️ Gandalf automated fix for #8492
Summary by Sourcery
Add a Gandalf automation script for generating issue-based pull requests and introduce a placeholder CONTRIBUTING file.
New Features:
Enhancements: