Skip to content

Conversation

@gricha
Copy link
Owner

@gricha gricha commented Feb 3, 2026

Summary

  • Switch from anthropic-api-key input to CLAUDE_CODE_OAUTH_TOKEN env var (from WARDEN_ANTHROPIC_API_KEY secret)
  • Add fetch-depth: 0 so warden can diff against base branch
  • Strip newlines from OAuth token to avoid auth failures

Comment on lines 93 to 100
useEffect(() => {
const handleResize = () => {
if (window.innerWidth >= 1024 && isOpen) {
onToggle();
}
};
window.addEventListener('resize', handleResize);
}, [isOpen]);

This comment was marked as outdated.

Comment on lines 376 to 378
{filteredWorkspaces.map((ws: WorkspaceInfo, index: number) => (
<WorkspaceRow
key={ws.name}
key={index}

This comment was marked as outdated.

Comment on lines 201 to 205
useEffect(() => {
if (workspaces && workspaces.length === 1 && !showCreate) {
navigate(`/workspaces/${workspaces[0].name}/sessions`)
}
}, [workspaces])

This comment was marked as outdated.


- name: Test claude auth with haiku
env:
CLAUDE_CODE_OAUTH_TOKEN: ${{ secrets.WARDEN_ANTHROPIC_API_KEY }}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ OAuth token prefix logged to workflow output (high confidence)

Line 33 logs the first 10 characters of the OAuth token to workflow logs. Workflow logs are often publicly accessible and can aid attackers in token compromise.

Suggested fix: Remove the token prefix logging. Only log non-sensitive metadata.

Identified by Warden via security-review · high, high confidence

echo '{"hasCompletedOnboarding": true}' > ~/.claude.json

- name: Test claude auth with haiku
env:
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 OAuth token length logged to workflow output (high confidence)

Line 32 logs the token length, which provides metadata that could assist in token compromise attempts.

Suggested fix: Remove token length logging to avoid exposing token metadata.

Identified by Warden via security-review · medium, high confidence

run: echo "CLAUDE_CODE_OAUTH_TOKEN=$(printf '%s' "$CLAUDE_CODE_OAUTH_TOKEN" | tr -d '\n\r\t ')" >> "$GITHUB_ENV"
- name: Fix warden CLAUDE_CODE_PATH
run: sudo mkdir -p /.local/bin && sudo ln -sf /home/runner/.local/bin/claude /.local/bin/claude
- uses: getsentry/warden@fix/propagate-sdk-errors
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 GitHub Action not pinned to commit hash (high confidence)

Line 54 uses an unpinned action reference (@fix/propagate-sdk-errors) which could change maliciously. Actions should be pinned to specific commit hashes.

Identified by Warden via security-review · medium, high confidence

Comment on lines 70 to 72
useEffect(() => {
setWorkspaceCount(workspaces?.length || 0);
}, [workspaces]);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 Unnecessary derived state - compute workspaceCount inline (high confidence)

workspaceCount is derived state synced via useEffect. Compute it directly as const workspaceCount = workspaces?.length || 0 instead.

Suggested fix: Remove useState and useEffect, compute workspaceCount directly from workspaces

Suggested change
useEffect(() => {
setWorkspaceCount(workspaces?.length || 0);
}, [workspaces]);
const workspaceCount = workspaces?.length || 0;

Identified by Warden via code-simplifier · medium, high confidence

Comment on lines 195 to 199
useEffect(() => {
setTotalCount(workspaces?.length || 0)
setRunningCount(workspaces?.filter(ws => ws.status === 'running').length || 0)
setFilteredWorkspaces(workspaces || [])
}, [workspaces])
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 Derived state synced via useEffect instead of computed inline (high confidence)

totalCount, runningCount, and filteredWorkspaces should be computed directly from workspaces, not synced to state via useEffect.

Suggested fix: Remove state variables and compute values inline

Suggested change
useEffect(() => {
setTotalCount(workspaces?.length || 0)
setRunningCount(workspaces?.filter(ws => ws.status === 'running').length || 0)
setFilteredWorkspaces(workspaces || [])
}, [workspaces])
const filteredWorkspaces = workspaces || []
const totalCount = filteredWorkspaces.length
const runningCount = filteredWorkspaces.filter(ws => ws.status === 'running').length

Identified by Warden via code-simplifier · medium, high confidence

Comment on lines 235 to 245
function WorkspaceStats({ workspaces }: { workspaces: WorkspaceInfo[] }) {
const running = workspaces.filter(ws => ws.status === 'running').length
return (
<div className="grid grid-cols-2 sm:grid-cols-4 gap-3">
<StatCard value={workspaces.length} label="Workspaces" />
<StatCard value={running} label="Running" accent={running > 0} />
<StatCard value={workspaces.length - running} label="Stopped" />
<StatCard value="—" label="Sessions" />
</div>
)
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 Component defined inside another component (high confidence)

WorkspaceStats is defined inside WorkspaceList, causing it to be recreated on every render. Move it outside the component.

Suggested fix: Move WorkspaceStats outside of WorkspaceList component

Suggested change
function WorkspaceStats({ workspaces }: { workspaces: WorkspaceInfo[] }) {
const running = workspaces.filter(ws => ws.status === 'running').length
return (
<div className="grid grid-cols-2 sm:grid-cols-4 gap-3">
<StatCard value={workspaces.length} label="Workspaces" />
<StatCard value={running} label="Running" accent={running > 0} />
<StatCard value={workspaces.length - running} label="Stopped" />
<StatCard value="—" label="Sessions" />
</div>
)
}
function WorkspaceStats({ workspaces }: { workspaces: WorkspaceInfo[] }) {
const running = workspaces.filter(ws => ws.status === 'running').length
return (
<div className="grid grid-cols-2 sm:grid-cols-4 gap-3">
<StatCard value={workspaces.length} label="Workspaces" />
<StatCard value={running} label="Running" accent={running > 0} />
<StatCard value={workspaces.length - running} label="Stopped" />
<StatCard value="—" label="Sessions" />
</div>
)
}

Identified by Warden via code-simplifier · medium, high confidence

Comment on lines 207 to 213
useEffect(() => {
const header = document.querySelector('.workspace-header')
if (header) {
header.classList.add('loaded')
}
document.title = `Perry - ${totalCount} Workspaces`
}, [totalCount])
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 Direct DOM manipulation instead of React patterns (high confidence)

Using document.querySelector and classList instead of React refs or state. This bypasses React's rendering model.

Identified by Warden via code-simplifier · medium, high confidence

{filteredWorkspaces.map((ws: WorkspaceInfo, index: number) => (
<WorkspaceRow
key={ws.name}
key={index}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 Array index used as key in list rendering (high confidence)

Using array index as key can cause issues with component state and performance. Use workspace.name as the unique identifier.

Suggested fix: Use workspace.name instead of index as key

Suggested change
key={index}
{filteredWorkspaces.map((ws: WorkspaceInfo) => (
key={ws.name}

Identified by Warden via code-simplifier · medium, high confidence

Comment on lines 195 to 199
useEffect(() => {
setTotalCount(workspaces?.length || 0)
setRunningCount(workspaces?.filter(ws => ws.status === 'running').length || 0)
setFilteredWorkspaces(workspaces || [])
}, [workspaces])
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 Unnecessary derived state synchronized via useEffect (high confidence)

totalCount, runningCount, and filteredWorkspaces are derived from workspaces and should be computed inline instead of stored in state and synced via useEffect.

Suggested fix: Remove state variables and compute values inline

Suggested change
useEffect(() => {
setTotalCount(workspaces?.length || 0)
setRunningCount(workspaces?.filter(ws => ws.status === 'running').length || 0)
setFilteredWorkspaces(workspaces || [])
}, [workspaces])
const totalCount = workspaces?.length || 0
const runningCount = workspaces?.filter(ws => ws.status === 'running').length || 0
const filteredWorkspaces = workspaces || []

Identified by Warden via code-simplifier · medium, high confidence

Comment on lines 201 to 205
useEffect(() => {
if (workspaces && workspaces.length === 1 && !showCreate) {
navigate(`/workspaces/${workspaces[0].name}/sessions`)
}
}, [workspaces])
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 useEffect with missing dependencies (high confidence)

useEffect depends on navigate and showCreate but only lists workspaces in dependencies array.

Suggested fix: Add missing dependencies to useEffect

Suggested change
useEffect(() => {
if (workspaces && workspaces.length === 1 && !showCreate) {
navigate(`/workspaces/${workspaces[0].name}/sessions`)
}
}, [workspaces])
}, [workspaces, navigate, showCreate])

Identified by Warden via code-simplifier · medium, high confidence

Comment on lines 376 to 382
{filteredWorkspaces.map((ws: WorkspaceInfo, index: number) => (
<WorkspaceRow
key={ws.name}
key={index}
workspace={ws}
onClick={() => handleRowClick(ws)}
onClick={() => navigate(`/workspaces/${ws.name}/sessions`)}
/>
))}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 Array index used as key in list rendering (high confidence)

Using index as key can cause rendering issues. Use workspace.name which is unique.

Suggested fix: Use workspace.name as key instead of index

Suggested change
{filteredWorkspaces.map((ws: WorkspaceInfo, index: number) => (
<WorkspaceRow
key={ws.name}
key={index}
workspace={ws}
onClick={() => handleRowClick(ws)}
onClick={() => navigate(`/workspaces/${ws.name}/sessions`)}
/>
))}
{filteredWorkspaces.map((ws: WorkspaceInfo) => (
key={ws.name}

Identified by Warden via code-simplifier · medium, high confidence

@gricha gricha force-pushed the test/warden-react-violations branch from 5a85282 to db18be5 Compare February 8, 2026 19:08
@gricha gricha changed the title test: react anti-patterns for warden validation fix: update warden CI auth to use CLAUDE_CODE_OAUTH_TOKEN Feb 8, 2026
@gricha gricha merged commit 80d7671 into main Feb 8, 2026
9 checks passed
@gricha gricha deleted the test/warden-react-violations branch February 8, 2026 19:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant