ros/automaker
1
0
Fork 0
mirror of https://github.com/AutoMaker-Org/automaker.git synced 2026-01-30 06:12:03 +00:00
Code Issues Packages Projects Releases Wiki Activity

chore: Remove obsolete files related to graph layout and security audit

Browse Source 
- Deleted `graph-layout-bug.md`, `SECURITY_TODO.md`, `TODO.md`, and `phase-model-selector.tsx` as they are no longer relevant to the project.
- This cleanup helps streamline the codebase and remove outdated documentation and components.
This commit is contained in:
Shirone
2026-01-25 17:41:17 +01:00
parent 5eda2c9b2b
commit b65037d995
4 changed files with 0 additions and 2110 deletions
Download Patch File Download Diff File Expand all files Collapse all files

300
SECURITY_TODO.md
View File

@@ -1,300 +0,0 @@
# Security Audit Findings - v0.13.0rc Branch
**Date:** $(date)
**Audit Type:** Git diff security review against v0.13.0rc branch
**Status:** ⚠️ Security vulnerabilities found - requires fixes before release
## Executive Summary
No intentionally malicious code was detected in the changes. However, several **critical security vulnerabilities** were identified that could allow command injection attacks. These must be fixed before release.
---
## 🔴 Critical Security Issues
### 1. Command Injection in Merge Handler
**File:** `apps/server/src/routes/worktree/routes/merge.ts`
**Lines:** 43, 54, 65-66, 93
**Severity:** CRITICAL
**Issue:**
User-controlled inputs (`branchName`, `mergeTo`, `options?.message`) are directly interpolated into shell commands without validation, allowing command injection attacks.
**Vulnerable Code:**
```typescript
// Line 43 - branchName not validated
await execAsync(`git rev-parse --verify ${branchName}`, { cwd: projectPath });
// Line 54 - mergeTo not validated
await execAsync(`git rev-parse --verify ${mergeTo}`, { cwd: projectPath });
// Lines 65-66 - branchName and message not validated
const mergeCmd = options?.squash
? `git merge --squash ${branchName}`
: `git merge ${branchName} -m "${options?.message || `Merge ${branchName} into ${mergeTo}`}"`;
// Line 93 - message not sanitized
await execAsync(`git commit -m "${options?.message || `Merge ${branchName} (squash)`}"`, {
cwd: projectPath,
});
```
**Attack Vector:**
An attacker could inject shell commands via branch names or commit messages:
- Branch name: `main; rm -rf /`
- Commit message: `"; malicious_command; "`
**Fix Required:**
1. Validate `branchName` and `mergeTo` using `isValidBranchName()` before use
2. Sanitize commit messages or use `execGitCommand` with proper escaping
3. Replace `execAsync` template literals with `execGitCommand` array-based calls
**Note:** `isValidBranchName` is imported but only used AFTER deletion (line 119), not before execAsync calls.
---
### 2. Command Injection in Push Handler
**File:** `apps/server/src/routes/worktree/routes/push.ts`
**Lines:** 44, 49
**Severity:** CRITICAL
**Issue:**
User-controlled `remote` parameter and `branchName` are directly interpolated into shell commands without validation.
**Vulnerable Code:**
```typescript
// Line 38 - remote defaults to 'origin' but not validated
const targetRemote = remote || 'origin';
// Lines 44, 49 - targetRemote and branchName not validated
await execAsync(`git push -u ${targetRemote} ${branchName} ${forceFlag}`, {
cwd: worktreePath,
});
await execAsync(`git push --set-upstream ${targetRemote} ${branchName} ${forceFlag}`, {
cwd: worktreePath,
});
```
**Attack Vector:**
An attacker could inject commands via the remote name:
- Remote: `origin; malicious_command; #`
**Fix Required:**
1. Validate `targetRemote` parameter (alphanumeric + `-`, `_` only)
2. Validate `branchName` before use (even though it comes from git output)
3. Use `execGitCommand` with array arguments instead of template literals
---
### 3. Unsafe Environment Variable Export in Shell Script
**File:** `start-automaker.sh`
**Lines:** 5068, 5085
**Severity:** CRITICAL
**Issue:**
Unsafe parsing and export of `.env` file contents using `xargs` without proper handling of special characters.
**Vulnerable Code:**
```bash
export $(grep -v '^#' .env | xargs)
```
**Attack Vector:**
If `.env` file contains malicious content with spaces, special characters, or code, it could be executed:
- `.env` entry: `VAR="value; malicious_command"`
- Could lead to code execution during startup
**Fix Required:**
Replace with safer parsing method:
```bash
# Safer approach
set -a
source <(grep -v '^#' .env | sed 's/^/export /')
set +a
# Or even safer - validate each line
while IFS= read -r line; do
[[ "$line" =~ ^[[:space:]]*# ]] && continue
[[ -z "$line" ]] && continue
if [[ "$line" =~ ^([A-Za-z_][A-Za-z0-9_]*)=(.*)$ ]]; then
export "${BASH_REMATCH[1]}"="${BASH_REMATCH[2]}"
fi
done < .env
```
---
## 🟡 Moderate Security Concerns
### 4. Inconsistent Use of Secure Command Execution
**Issue:**
The codebase has `execGitCommand()` function available (which uses array arguments and is safer), but it's not consistently used. Some places still use `execAsync` with template literals.
**Files Affected:**
- `apps/server/src/routes/worktree/routes/merge.ts`
- `apps/server/src/routes/worktree/routes/push.ts`
**Recommendation:**
- Audit all `execAsync` calls with template literals
- Replace with `execGitCommand` where possible
- Document when `execAsync` is acceptable (only with fully validated inputs)
---
### 5. Missing Input Validation
**Issues:**
1. `targetRemote` in `push.ts` defaults to 'origin' but isn't validated
2. Commit messages in `merge.ts` aren't sanitized before use in shell commands
3. `worktreePath` validation relies on middleware but should be double-checked
**Recommendation:**
- Add validation functions for remote names
- Sanitize commit messages (remove shell metacharacters)
- Add defensive validation even when middleware exists
---
## ✅ Positive Security Findings
1. **No Hardcoded Credentials:** No API keys, passwords, or tokens found in the diff
2. **No Data Exfiltration:** No suspicious network requests or data transmission patterns
3. **No Backdoors:** No hidden functionality or unauthorized access patterns detected
4. **Safe Command Execution:** `execGitCommand` function properly uses array arguments in some places
5. **Environment Variable Handling:** `init-script-service.ts` properly sanitizes environment variables (lines 194-220)
---
## 📋 Action Items
### Immediate (Before Release)
- [ ] **Fix command injection in `merge.ts`**
- [ ] Validate `branchName` with `isValidBranchName()` before line 43
- [ ] Validate `mergeTo` with `isValidBranchName()` before line 54
- [ ] Sanitize commit messages or use `execGitCommand` for merge commands
- [ ] Replace `execAsync` template literals with `execGitCommand` array calls
- [ ] **Fix command injection in `push.ts`**
- [ ] Add validation function for remote names
- [ ] Validate `targetRemote` before use
- [ ] Validate `branchName` before use (defensive programming)
- [ ] Replace `execAsync` template literals with `execGitCommand`
- [ ] **Fix shell script security issue**
- [ ] Replace unsafe `export $(grep ... | xargs)` with safer parsing
- [ ] Add validation for `.env` file contents
- [ ] Test with edge cases (spaces, special chars, quotes)
### Short-term (Next Sprint)
- [ ] **Audit all `execAsync` calls**
- [ ] Create inventory of all `execAsync` calls with template literals
- [ ] Replace with `execGitCommand` where possible
- [ ] Document exceptions and why they're safe
- [ ] **Add input validation utilities**
- [ ] Create `isValidRemoteName()` function
- [ ] Create `sanitizeCommitMessage()` function
- [ ] Add validation for all user-controlled inputs
- [ ] **Security testing**
- [ ] Add unit tests for command injection prevention
- [ ] Add integration tests with malicious inputs
- [ ] Test shell script with malicious `.env` files
### Long-term (Security Hardening)
- [ ] **Code review process**
- [ ] Add security checklist for PR reviews
- [ ] Require security review for shell command execution changes
- [ ] Add automated security scanning
- [ ] **Documentation**
- [ ] Document secure coding practices for shell commands
- [ ] Create security guidelines for contributors
- [ ] Add security section to CONTRIBUTING.md
---
## 🔍 Testing Recommendations
### Command Injection Tests
```typescript
// Test cases for merge.ts
describe('merge handler security', () => {
it('should reject branch names with shell metacharacters', () => {
// Test: branchName = "main; rm -rf /"
// Expected: Validation error, command not executed
});
it('should sanitize commit messages', () => {
// Test: message = '"; malicious_command; "'
// Expected: Sanitized or rejected
});
});
// Test cases for push.ts
describe('push handler security', () => {
it('should reject remote names with shell metacharacters', () => {
// Test: remote = "origin; malicious_command; #"
// Expected: Validation error, command not executed
});
});
```
### Shell Script Tests
```bash
# Test with malicious .env content
echo 'VAR="value; echo PWNED"' > test.env
# Expected: Should not execute the command
# Test with spaces in values
echo 'VAR="value with spaces"' > test.env
# Expected: Should handle correctly
# Test with special characters
echo 'VAR="value\$with\$dollars"' > test.env
# Expected: Should handle correctly
```
---
## 📚 References
- [OWASP Command Injection](https://owasp.org/www-community/attacks/Command_Injection)
- [Node.js Child Process Security](https://nodejs.org/api/child_process.html#child_process_security_concerns)
- [Shell Script Security Best Practices](https://mywiki.wooledge.org/BashGuide/Practices)
---
## Notes
- All findings are based on code diff analysis
- No runtime testing was performed
- Assumes attacker has access to API endpoints (authenticated or unauthenticated)
- Fixes should be tested thoroughly before deployment
---
**Last Updated:** $(date)
**Next Review:** After fixes are implemented

25
TODO.md
View File

@@ -1,25 +0,0 @@
# Bugs
- Setting the default model does not seem like it works.
# Performance (completed)
- [x] Graph performance mode for large graphs (compact nodes/edges + visible-only rendering)
- [x] Render containment on heavy scroll regions (kanban columns, chat history)
- [x] Reduce blur/shadow effects when lists get large
- [x] React Query tuning for heavy datasets (less refetch on focus/reconnect)
- [x] DnD/list rendering optimizations (virtualized kanban + memoized card sections)
# UX
- Consolidate all models to a single place in the settings instead of having AI profiles and all this other stuff
- Simplify the create feature modal. It should just be one page. I don't need nessa tabs and all these nested buttons. It's too complex.
- added to do's list checkbox directly into the card so as it's going through if there's any to do items we can see those update live
- When the feature is done, I want to see a summary of the LLM. That's the first thing I should see when I double click the card.
- I went away to mass edit all my features. For example, when I created a new project, it added auto testing on every single feature card. Now I have to manually go through one by one and change those. Have a way to mass edit those, the configuration of all them.
- Double check and debug if there's memory leaks. It seems like the memory of automaker grows like 3 gigabytes. It's 5gb right now and I'm running three different cursor cli features implementing at the same time.
- Typing in the text area of the plan mode was super laggy.
- When I have a bunch of features running at the same time, it seems like I cannot edit the features in the backlog. Like they don't persist their file changes and I think this is because of the secure FS file has an internal queue to prevent hitting that file open write limit. We may have to reconsider refactoring away from file system and do Postgres or SQLite or something.
- modals are not scrollable if height of the screen is small enough
- and the Agent Runner add an archival button for the new sessions.
- investigate a potential issue with the feature cards not refreshing. I see a lock icon on the feature card But it doesn't go away until I open the card and edit it and I turn the testing mode off. I think there's like a refresh sync issue.

203
graph-layout-bug.md
View File

@@ -1,203 +0,0 @@
# Graph View Layout Bug
## Problem
When navigating directly to the graph view route (e.g., refreshing on `/graph` or opening the app on that route), all feature cards appear in a single vertical column instead of being properly arranged in a hierarchical dependency graph.
**Works correctly when:** User navigates to Kanban view first, then to Graph view.
**Broken when:** User loads the graph route directly (refresh, direct URL, app opens on that route).
## Expected Behavior
Nodes should be positioned by the dagre layout algorithm in a hierarchical DAG based on their dependency relationships (edges).
## Actual Behavior
All nodes appear stacked in a single column/row, as if dagre computed the layout with no edges.
## Technology Stack
- React 19
- @xyflow/react (React Flow) for graph rendering
- dagre for layout algorithm
- Zustand for state management
## Architecture
### Data Flow
1. `GraphViewPage` loads features via `useBoardFeatures` hook
2. Shows loading spinner while `isLoading === true`
3. When loaded, renders `GraphView` → `GraphCanvas`
4. `GraphCanvas` uses three hooks:
- `useGraphNodes`: Transforms features → React Flow nodes and edges (edges from `feature.dependencies`)
- `useGraphLayout`: Applies dagre layout to position nodes based on edges
- `useNodesState`/`useEdgesState`: React Flow's state management
### Key Files
- `apps/ui/src/components/views/graph-view-page.tsx` - Page component with loading state
- `apps/ui/src/components/views/graph-view/graph-canvas.tsx` - React Flow integration
- `apps/ui/src/components/views/graph-view/hooks/use-graph-layout.ts` - Dagre layout logic
- `apps/ui/src/components/views/graph-view/hooks/use-graph-nodes.ts` - Feature → node/edge transformation
- `apps/ui/src/components/views/board-view/hooks/use-board-features.ts` - Data fetching
## Relevant Code
### use-graph-layout.ts (layout computation)
```typescript
export function useGraphLayout({ nodes, edges }: UseGraphLayoutProps) {
const positionCache = useRef<Map<string, { x: number; y: number }>>(new Map());
const lastStructureKey = useRef<string>('');
const layoutVersion = useRef<number>(0);
const getLayoutedElements = useCallback((inputNodes, inputEdges, direction = 'LR') => {
const dagreGraph = new dagre.graphlib.Graph();
dagreGraph.setGraph({ rankdir: direction, nodesep: 50, ranksep: 100 });
inputNodes.forEach((node) => {
dagreGraph.setNode(node.id, { width: 280, height: 120 });
});
inputEdges.forEach((edge) => {
dagreGraph.setEdge(edge.source, edge.target); // THIS IS WHERE EDGES MATTER
});
dagre.layout(dagreGraph);
// ... returns positioned nodes
}, []);
// Structure key includes both nodes AND edges
const structureKey = useMemo(() => {
const nodeIds = nodes
.map((n) => n.id)
.sort()
.join(',');
const edgeConnections = edges
.map((e) => `${e.source}->${e.target}`)
.sort()
.join(',');
return `${nodeIds}|${edgeConnections}`;
}, [nodes, edges]);
const layoutedElements = useMemo(() => {
if (nodes.length === 0) return { nodes: [], edges: [] };
const structureChanged = structureKey !== lastStructureKey.current;
if (structureChanged) {
lastStructureKey.current = structureKey;
layoutVersion.current += 1;
return getLayoutedElements(nodes, edges, 'LR'); // Full layout with edges
} else {
// Use cached positions
}
}, [nodes, edges, structureKey, getLayoutedElements]);
return { layoutedNodes, layoutedEdges, layoutVersion: layoutVersion.current, runLayout };
}
```
### graph-canvas.tsx (React Flow integration)
```typescript
function GraphCanvasInner({ features, ... }) {
// Transform features to nodes/edges
const { nodes: initialNodes, edges: initialEdges } = useGraphNodes({ features, ... });
// Apply layout
const { layoutedNodes, layoutedEdges, layoutVersion, runLayout } = useGraphLayout({
nodes: initialNodes,
edges: initialEdges,
});
// React Flow state - INITIALIZES with layoutedNodes
const [nodes, setNodes, onNodesChange] = useNodesState(layoutedNodes);
const [edges, setEdges, onEdgesChange] = useEdgesState(layoutedEdges);
// Effect to update nodes when layout changes
useEffect(() => {
// ... updates nodes/edges state when layoutedNodes/layoutedEdges change
}, [layoutedNodes, layoutedEdges, layoutVersion, ...]);
// Attempted fix: Force layout after mount when edges are available
useEffect(() => {
if (!hasLayoutWithEdges.current && layoutedNodes.length > 0 && layoutedEdges.length > 0) {
hasLayoutWithEdges.current = true;
setTimeout(() => runLayout('LR'), 100);
}
}, [layoutedNodes.length, layoutedEdges.length, runLayout]);
return <ReactFlow nodes={nodes} edges={edges} fitView ... />;
}
```
### use-board-features.ts (data loading)
```typescript
export function useBoardFeatures({ currentProject }) {
const { features, setFeatures } = useAppStore(); // From Zustand store
const [isLoading, setIsLoading] = useState(true);
const loadFeatures = useCallback(async () => {
setIsLoading(true);
const result = await api.features.getAll(currentProject.path);
if (result.success) {
const featuresWithIds = result.features.map((f) => ({
...f, // dependencies come from here via spread
id: f.id || `...`,
status: f.status || 'backlog',
}));
setFeatures(featuresWithIds); // Updates Zustand store
}
setIsLoading(false);
}, [currentProject, setFeatures]);
useEffect(() => { loadFeatures(); }, [loadFeatures]);
return { features, isLoading, ... }; // features is from useAppStore()
}
```
### graph-view-page.tsx (loading gate)
```typescript
export function GraphViewPage() {
const { features: hookFeatures, isLoading } = useBoardFeatures({ currentProject });
if (isLoading) {
return <Spinner />; // Graph doesn't render until loading is done
}
return <GraphView features={hookFeatures} ... />;
}
```
## What I've Tried
1. **Added edges to structureKey** - So layout recalculates when dependencies change, not just when nodes change
2. **Added layoutVersion tracking** - To signal when a fresh layout was computed vs cached positions used
3. **Track layoutVersion in GraphCanvas** - To detect when to apply fresh positions instead of preserving old ones
4. **Force runLayout after mount** - Added useEffect that calls `runLayout('LR')` after 100ms when nodes and edges are available
5. **Reset all refs on project change** - Clear layout state when switching projects
## Hypothesis
The issue appears to be a timing/race condition where:
- When going Kanban → Graph: Features are already in Zustand store, so graph mounts with complete data
- When loading Graph directly: Something causes the initial layout to compute before edges are properly available, or the layout result isn't being applied to React Flow's state correctly
The fact that clicking Kanban then Graph works suggests the data IS correct, just something about the initial render timing when loading the route directly.
## Questions to Investigate
1. Is `useNodesState(layoutedNodes)` capturing stale initial positions?
2. Is there a React 19 / StrictMode double-render issue with the refs?
3. Is React Flow's `fitView` prop interfering with initial positions?
4. Is there a race between Zustand store updates and React renders?
5. Should the graph component not render until layout is definitively computed with edges?

1582
worktrees/automode-api/apps/ui/src/components/views/settings-view/model-defaults/phase-model-selector.tsx
View File

File diff suppressed because it is too large Load Diff