Skip to content

110#41

Merged
alvagante merged 19 commits intomainfrom
110
Apr 21, 2026
Merged

110#41
alvagante merged 19 commits intomainfrom
110

Conversation

@alvagante
Copy link
Copy Markdown
Member

No description provided.

… integrations

- Add cumulative login attempt counter table for permanent account lockout tracking
- Validate hostnames in Ansible inventory generation to prevent INI injection attacks
- Implement path traversal prevention in fact file lookups with nodeId validation
- Add path traversal guards in Hiera hierarchy interpolation for malicious fact values
- Add warning log when SSH host key verification is disabled in production
- Fix CORS configuration to validate URL format and default to empty array
- Fix expert mode header handling to use middleware-gated property instead of raw header
- Restrict cross-origin resource policy from "cross-origin" to "same-site"
- Remove NODE_ENV test environment bypass in rate limiting middleware
- Update authentication and error handling tests to reflect security changes
- Add brute force attack test coverage for cumulative attempt tracking
- Enhance migration integration tests for new login attempt counter schema
…and best practices

- Reorganize development-standards.md with new sections: Code Quality Standards, Backend-Specific Patterns, Async Performance, Pre-Work Before Refactors
- Add concrete limits: max 300 lines per file, max 50 lines per function
- Establish quality gates that must pass before any commit: tests, TypeScript compilation, linting, pre-commit hooks
- Clarify logging standards: use LoggerService with structured metadata, never console.log
- Clarify configuration access: always use ConfigService backed by Zod validation
- Add async performance guidance: use Promise.all for independent operations, never await sequentially
- Refactor kirograph.md to clarify KiroGraph is Kiro-native and already configured in mcp.json
- Add note about Claude Code's CodeGraph as separate but analogous tool
- Streamline mcp-best-practices.md: remove redundant installation links, clarify uvx behavior
- Consolidate dependency management section with focus on justification and maintenance
- Simplify documentation approach: working docs go in .kiro/, never create summary files unless asked
- Align all steering docs with established project patterns and quality standards
- 1.1 Add mapProxmoxTaskType and collectProxmoxTaskEntries
- 2.1 Add mapEC2StateToEventType and collectAWSStateEntry
- ProxmoxClientLike and AWSServiceLike minimal interfaces
- Deterministic IDs: proxmox:task:{upid}, aws:state:{id}:{state}
- Graceful error handling with LoggerService
Copilot AI review requested due to automatic review settings April 16, 2026 08:28
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR combines a major documentation restructure/rewrite with several backend security/auth hardening changes (streaming auth tickets, improved expert-mode gating, cumulative failed-login counters, and multiple path-traversal/validation guards).

Changes:

  • Rewrites and reorganizes docs (new deployment docs, simplified integration guides, and new internal reference docs).
  • Backend security/auth updates: SSE stream ticket support, expert-mode gating fix, bcrypt cost increase, cumulative failed-attempt tracking (migration 011), and stricter validations/guards.
  • Adds/extends journal collectors (Proxmox task collection + AWS EC2 state collection) and adjusts tests accordingly.

Reviewed changes

Copilot reviewed 51 out of 71 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
docs/kubernetes-deployment.md Removes legacy Kubernetes deployment guide (replaced by new deployment docs).
docs/internal/security-assessment-v0.10.0.md Adds internal security assessment report.
docs/internal/screenshots.md Adds internal screenshots reference doc.
docs/internal/repo_structure_and_config.md Adds internal repo/dotfiles/config overview.
docs/internal/e2e-testing.md Adds internal E2E testing guide.
docs/internal/development/puppetdb-puppetserver-api-endpoints.md Adds internal endpoint usage analysis doc.
docs/internal/development/BACKEND_REFACTORING_OPPORTUNITIES.md Adds internal backend refactoring opportunities doc.
docs/internal/description.md Adds internal technical summary doc.
docs/internal/api-endpoints-reference.md Adds internal consolidated API endpoint reference.
docs/integrations/ssh.md Rewrites SSH integration docs into shorter “sharp” format.
docs/integrations/puppetserver.md Rewrites Puppetserver integration docs into shorter “sharp” format.
docs/integrations/hiera.md Rewrites Hiera integration docs into shorter “sharp” format.
docs/integrations/bolt.md Rewrites Bolt integration docs into shorter “sharp” format.
docs/integrations/aws.md Rewrites AWS integration docs into shorter “sharp” format.
docs/integrations/ansible.md Rewrites Ansible integration docs into shorter “sharp” format.
docs/development.md Expands and updates contributor development guide.
docs/deployment/kubernetes.md Adds new Kubernetes deployment doc (replacement).
docs/deployment/docker.md Adds new Docker deployment doc.
backend/test/unit/error-handling.test.ts Updates test schema setup for new login_attempt_counters table.
backend/test/services/AuthenticationService.bruteforce.test.ts Updates schema setup for new login_attempt_counters table.
backend/test/properties/auth/password-hashing.property.test.ts Updates bcrypt expectations to cost factor 12.
backend/test/properties/auth/authentication-atomicity.property.test.ts Updates schema setup for login_attempt_counters.
backend/test/performance/rbac-performance.test.ts Adjusts perf thresholds for bcrypt cost increase; updates schema shapes.
backend/test/integration/auth-flow.test.ts Minor integration test adjustment (permission grant call moved/removed).
backend/test/integration/api.test.ts Hardens expert-mode error detail exposure test (req.expertMode vs raw header).
backend/test/database/migration-integration.test.ts Updates migration count to include new migration 011.
backend/test/AuthenticationService.test.ts Updates logging assertions and schema init for login_attempt_counters.
backend/src/services/journal/JournalCollectors.ts Adds Proxmox + AWS journal collectors and event-type mappers.
backend/src/services/AuthenticationService.ts Bcrypt cost increase, structured logging, cumulative failed-attempt counter logic.
backend/src/server.ts Adds streamAuthMiddleware ordering; adds RBAC gate to /api/debug.
backend/src/routes/streaming.ts Adds stream ticket issuance + pre-auth middleware for SSE auth.
backend/src/middleware/securityMiddleware.ts Tightens CORP policy; removes NODE_ENV=test rate-limit bypass.
backend/src/middleware/errorHandler.ts Stops reading raw X-Expert-Mode header; uses role-gated req.expertMode.
backend/src/integrations/ssh/SSHService.ts Warns loudly when SSH host key verification is disabled.
backend/src/integrations/hiera/HieraService.ts Adds guard against path traversal in hierarchy interpolation resolution.
backend/src/integrations/hiera/FactService.ts Validates nodeId and prevents path traversal in local fact file lookup.
backend/src/integrations/ansible/AnsibleService.ts Validates hostname to prevent Ansible inventory/INI injection.
backend/src/database/migrations/011_cumulative_failed_attempts.sql Adds login_attempt_counters table for permanent lockout tracking.
backend/src/config/schema.ts Changes CORS defaults to empty + URL-validated origins.
.mdd/docs/01-docs-rewrite.md Adds docs rewrite MDD tracking doc.
.mdd/.startup.md Adds MDD startup metadata file for the branch.
.kiro/steering/user-interaction.md Updates Kiro steering guidance (clarity/style).
.kiro/steering/typescript-best-practices.md Updates TypeScript steering guidance.
.kiro/steering/testing-best-practices.md Updates testing steering guidance.
.kiro/steering/mcp-best-practices.md Updates MCP steering guidance.
.kiro/steering/kirograph.md Updates KiroGraph steering doc.
.kiro/steering/development-standards.md Updates development standards steering doc.
.kiro/specs/journal-enhancements/tasks.md Adds journal-enhancements implementation plan.
.kiro/specs/journal-enhancements/requirements.md Adds journal-enhancements requirements.
.kiro/specs/journal-enhancements/design.md Adds journal-enhancements design doc.
.kiro/specs/journal-enhancements/.config.kiro Adds journal-enhancements spec config.

Comment thread backend/src/routes/streaming.ts Outdated
Comment thread backend/src/services/journal/JournalCollectors.ts Outdated
Comment thread backend/src/services/AuthenticationService.ts
- 4.1 Add GlobalTimelineFilters, getGlobalTimeline, getGlobalEntryCount
- 5.1 Add GET /api/journal/global/stream SSE endpoint
- 5.2 Integrate Proxmox and AWS collectors into node stream
- ProxmoxService/Integration expose getClient() for task history
- Server.ts passes integrationManager to journal router
- Simplify and tighten project description with clearer value proposition
- Convert features list to em-dash format for improved readability
- Reorganize table of contents to prioritize screenshots and move project structure after configuration
- Remove redundant project structure section from main README
- Update prerequisite links to use inline markdown format
- Clarify installation instructions and remove unnecessary explanatory text
- Fix HTML entity encoding in heading (& to &)
- Improve punctuation consistency throughout document
- 7.1 Refactor JournalTimeline with mode prop (node/global)
- 7.2 Compact single-line entry display with expand-on-click
- 7.3 Update NodeDetailPage to pass mode=node
- 8.1 Create GlobalJournalPage with filter bar
- 8.2 Register /journal route and add nav link
- Update JournalTimeline tests for new mode prop
- Fix conflicting $effect blocks causing infinite reload loop in global mode
- Replace text inputs with proper node/group multi-select panel
- Fetch inventory on mount with search, source filter, select all/clear
- Nodes use checkboxes (multi-select), groups use radio (single select)
- Show integration badges on nodes and groups
- Add targetsExpanded state with chevron toggle
- Selection summary shown inline in header when collapsed
- Starts expanded by default
Copilot AI review requested due to automatic review settings April 16, 2026 09:19
- Link navigates to /nodes/{nodeId} detail page
- stopPropagation prevents expand/collapse toggle on click
- Styled as primary-colored link with hover state
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 63 out of 83 changed files in this pull request and generated 5 comments.

Comments suppressed due to low confidence (1)

backend/src/middleware/securityMiddleware.ts:66

  • Rate limiting is now always enabled even when NODE_ENV === 'test' (the previous test-environment bypass was removed). This can make CI/tests flaky or unexpectedly fail if a test issues >100 requests/min with the same user: key or IP.

Suggested fix: either restore the explicit test bypass, or configure express-rate-limit with a test-friendly store/reset strategy in the test harness so tests don’t depend on timing/window state.

export function createRateLimitMiddleware(): (req: Request, res: Response, next: NextFunction) => void {
  return rateLimit({
    windowMs: 60 * 1000, // 1 minute window
    max: 100, // Limit each user to 100 requests per window
    message: {
      error: "Too many requests from this user, please try again later.",
      retryAfter: 60,
    },
    standardHeaders: true, // Return rate limit info in `RateLimit-*` headers
    legacyHeaders: false, // Disable `X-RateLimit-*` headers

    // Use user ID as the key for rate limiting
    keyGenerator: (req: Request): string => {
      // If user is authenticated, use their user ID
      if (req.user?.userId) {
        return `user:${req.user.userId}`;
      }

      // For unauthenticated requests, use IP address with proper IPv6 handling
      return ipKeyGenerator(req.ip ?? req.socket.remoteAddress ?? "");
    },

Comment thread backend/src/services/journal/JournalCollectors.ts
Comment thread backend/src/routes/streaming.ts
Comment on lines +184 to +201
// Resolve groupId to nodeIds if provided
let nodeIds: string[] | undefined;
if (query.nodeIds) {
nodeIds = query.nodeIds.split(",");
}

if (query.groupId && deps.integrationManager) {
const inventory = await deps.integrationManager.getAggregatedInventory();
const group = inventory.groups.find((g) => g.id === query.groupId);
if (!group) {
send("source_error", { source: "global", message: "Group not found" });
clearInterval(heartbeat);
send("complete", {});
res.end();
return;
}
nodeIds = group.nodes;
}
Copy link

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

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

nodeIds is parsed via query.nodeIds.split(",") without trimming or filtering empties. Trailing commas or spaces (e.g. nodeIds=a, b,) will produce " b" / "" values, which can cause getGlobalTimeline()’s Zod validation to fail (nodeIds must be non-empty) and the stream to return a generic source_error.

Suggested fix: normalize with split(',').map(s => s.trim()).filter(Boolean) before passing to JournalService.

Copilot uses AI. Check for mistakes.
Comment on lines +140 to +143
function buildStreamUrl(): string {
if (mode === "node") {
return `/api/journal/${encodeURIComponent(nodeId ?? '')}/stream`;
}
Copy link

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

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

In mode === "node", nodeId is optional but buildStreamUrl() will still generate /api/journal//stream when nodeId is missing (encodeURIComponent(nodeId ?? '')). That will silently attempt an invalid stream URL.

Suggested fix: make Props a discriminated union so nodeId is required when mode: "node", and/or add a runtime guard that sets streamError and aborts startStream() when nodeId is absent.

Copilot uses AI. Check for mistakes.
Comment on lines +416 to +428
// 2. Query last recorded state from journal_entries
let previousState: string | undefined;
try {
const rows = await db.query<{ details: string }>(
`SELECT details FROM journal_entries WHERE nodeId = ? AND source = 'aws' ORDER BY timestamp DESC LIMIT 1`,
[nodeId],
);
if (rows.length > 0) {
const details = typeof rows[0].details === "string"
? (JSON.parse(rows[0].details) as Record<string, unknown>)
: (rows[0].details as Record<string, unknown>);
previousState = details.currentState as string | undefined;
}
Copy link

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

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

collectAWSStateEntry reads the “previous state” from journal_entries where source = 'aws', but this stream collector never records the emitted state entry back into journal_entries. Because of that (and because other AWS journal entries created by AWS actions won’t have details.currentState), previousState will typically stay undefined, causing a new “state change” entry to be emitted every time the stream is opened.

Suggested fix: persist AWS state entries via JournalService.recordEvent() (or a dedicated table) and query only those entries (e.g., filter by action or a dedicated details.kind) when determining previousState.

Copilot uses AI. Check for mistakes.
…ction

- Replace single event type values with comma-separated array support in global/node stream filters
- Add csvToEnumArray helper to parse and validate comma-separated enum values from query parameters
- Implement collectGlobalExecutionEntries collector for bolt, ansible, and ssh execution history
- Implement collectGlobalPuppetDBEntries collector for PuppetDB report aggregation
- Update event type enum to use 'unknown' instead of 'error', 'warning', 'info' for consistency
- Add NodeStreamQuerySchema for node-specific filtering with source and eventType support
- Refactor global stream to initialize with dynamic source list based on configured collectors
- Parallelize collection from multiple sources (journal, executions, puppetdb) with individual error handling
- Add source-specific filtering logic to skip collectors when excluded by query parameters
- Create MultiSelectDropdown component for frontend multi-value selection
- Update JournalTimeline and GlobalJournalPage to support filtering by multiple sources and event types
- Add comprehensive test coverage for new filtering and collection logic
- Update version to 1.1.0 in root package.json
- Update version to 1.1.0 in backend/package.json
- Update version to 1.1.0 in frontend/package.json
- Update version label to 1.1.0 in Navigation.svelte component
- Update OCI image version label to 1.1.0 in all Dockerfiles
- Remove update-version.sh script in favor of manual version management
Copilot AI review requested due to automatic review settings April 16, 2026 13:37
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 73 out of 93 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

backend/src/server.ts:1219

  • streamAuthMiddleware is mounted after an earlier /api/executions authMiddleware chain, so requests to /api/executions/:id/stream without an Authorization header will be rejected before streamAuthMiddleware has a chance to convert ?ticket=/?token= into a header. This effectively breaks EventSource auth via query params/tickets. Consider moving streamAuthMiddleware to run before any auth middleware for these paths (e.g., a dedicated app.use('/api/executions', streamAuthMiddleware) placed before the auth-protected routers, or mount the streaming router before the general /api/executions router).

Comment on lines +373 to +378
<a
href="/nodes/{encodeURIComponent(entry.nodeId)}"
onclick={(e) => { e.preventDefault(); e.stopPropagation(); router.navigate(`/nodes/${encodeURIComponent(entry.nodeId)}`); }}
class="shrink-0 rounded bg-gray-100 px-1.5 py-0.5 text-xs font-mono text-primary-600 hover:text-primary-800 hover:bg-primary-50 dark:bg-gray-700 dark:text-primary-400 dark:hover:text-primary-300 dark:hover:bg-primary-900/20"
title="Go to node {entry.nodeId}"
>{entry.nodeId}</a>
Copy link

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

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

The node link href is currently a literal string (/nodes/{encodeURIComponent(entry.nodeId)}) rather than a Svelte expression, so it will render with curly braces and not navigate correctly (and breaks “open in new tab”/copy link). Use an interpolated expression for href (and keep it consistent with the router.navigate target).

Copilot uses AI. Check for mistakes.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Alessandro Franceschi <al@example42.com>
Copilot AI review requested due to automatic review settings April 21, 2026 13:26
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Alessandro Franceschi <al@example42.com>
@alvagante
Copy link
Copy Markdown
Member Author

@copilot apply changes based on the comments in this thread

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 73 out of 93 changed files in this pull request and generated 3 comments.

Comment on lines 11 to 15
eventType TEXT NOT NULL CHECK (eventType IN (
'provision', 'destroy', 'start', 'stop', 'reboot', 'suspend', 'resume',
'command_execution', 'task_execution', 'puppet_run', 'package_install',
'config_change', 'note', 'error', 'warning', 'info'
'config_change', 'note', 'unknown'
)),
Copy link

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

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

This PR modifies an existing numbered migration (008_journal_entries.sql) by changing the eventType CHECK constraint. In a migration-based workflow, shipped migrations should be immutable; changing an old migration can break existing environments and makes migration history non-reproducible. Instead, revert this change in 008 and add a new numbered migration that performs the schema/data transition (SQLite will likely require table rebuild + data copy).

Copilot uses AI. Check for mistakes.
Comment on lines +214 to +242

// ============================================================================
// Proxmox Task Collector
// ============================================================================

/**
* Shape of a task record from GET /api2/json/nodes/{node}/tasks
*/
export interface ProxmoxTaskRecord {
upid: string;
node: string;
starttime: number;
type: string;
status?: string;
user: string;
id: string;
}

/**
* Minimal interface for Proxmox client to avoid circular deps
*/
export interface ProxmoxClientLike {
get(endpoint: string): Promise<unknown>;
}

/**
* Map a Proxmox task type string to a JournalEventType.
*/
export function mapProxmoxTaskType(taskType: string): JournalEventType {
Copy link

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

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

JournalCollectors.ts has grown substantially and now mixes multiple unrelated collectors (execution mapping, PuppetDB, Proxmox, AWS, and global collectors) plus several public interfaces in one file. This makes the module harder to maintain and increases merge-conflict risk. Consider splitting by integration/concern (e.g., collectors/proxmox.ts, collectors/aws.ts, collectors/executions.ts) and re-exporting the functions from a small index module.

Copilot uses AI. Check for mistakes.
Comment thread backend/src/server.ts
Comment on lines 1204 to 1218
createExecutionsRouter(executionRepository, executionQueue, batchExecutionService),
);
app.use(
"/api/executions",
streamAuthMiddleware, // resolve ?ticket= / ?token= before auth check
authMiddleware,
rateLimitMiddleware,
createStreamingRouter(streamingManager, executionRepository),
);
app.use(
"/api/streaming",
streamAuthMiddleware, // resolve ?ticket= / ?token= before auth check
authMiddleware,
rateLimitMiddleware,
createStreamingRouter(streamingManager, executionRepository),
Copy link

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

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

streamAuthMiddleware is mounted at the router level for /api/executions and /api/streaming, which means ?ticket= / ?token= will be accepted as authentication for all endpoints under those prefixes (not just SSE). This unnecessarily broadens the “auth via query string” surface area. Consider scoping the middleware only to the SSE endpoints (e.g., /:id/stream and /stats if needed), and when redeeming ticket, validate it matches the requested :id (and ideally user) before translating it into an Authorization header.

Copilot uses AI. Check for mistakes.
Copilot AI review requested due to automatic review settings April 21, 2026 14:05
@alvagante alvagante review requested due to automatic review settings April 21, 2026 14:05
Copilot AI review requested due to automatic review settings April 21, 2026 14:08
@alvagante alvagante review requested due to automatic review settings April 21, 2026 14:08
Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 21, 2026

@copilot apply changes based on the comments in this thread

Done — I applied the unresolved review-thread fixes and pushed them in fed05c0 (with the main set in 450ea12 and follow-ups in 16ee29b). Screenshot: https://github.com/user-attachments/assets/4b413c4c-6ade-4268-8d10-684e01f2d9f5

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • httpstat.us
    • Triggering command: /opt/hostedtoolcache/node/24.14.1/x64/bin/node /opt/hostedtoolcache/node/24.14.1/x64/bin/node --conditions node --conditions development --experimental-import-meta-resolve --require /home/REDACTED/work/pabawi/pabawi/node_modules/vitest/suppress-warnings.cjs /home/REDACTED/work/pabawi/pabawi/node_modules/vitest/dist/workers/forks.js l/config (dns block)
    • Triggering command: /opt/hostedtoolcache/node/24.14.1/x64/bin/node /opt/hostedtoolcache/node/24.14.1/x64/bin/node --conditions node --conditions development --experimental-import-meta-resolve --require /home/REDACTED/work/pabawi/pabawi/node_modules/vitest/suppress-warnings.cjs /home/REDACTED/work/pabawi/pabawi/node_modules/vitest/dist/workers/forks.js /home/REDACTED/.ca-c node_modules/vittsc &amp;&amp; mkdir -p dist/database &amp;&amp; cp -r src/database/migrations dist/database/ 1/x64/bin/node -c dist/database &amp;&amp; cp -r src/database/migrations dist/database/ --64 ache/node/24.14.1/x64/bin/node lines; --global ve ache/node/24.14.--conditions (dns block)
  • puppetdb.example.com
    • Triggering command: /opt/hostedtoolcache/node/24.14.1/x64/bin/node /opt/hostedtoolcache/node/24.14.1/x64/bin/node --conditions node --conditions development --experimental-import-meta-resolve --require /home/REDACTED/work/pabawi/pabawi/node_modules/vitest/suppress-warnings.cjs /home/REDACTED/work/pabawi/pabawi/node_modules/vitest/dist/workers/forks.js l/config (dns block)
    • Triggering command: /opt/hostedtoolcache/node/24.14.1/x64/bin/node /opt/hostedtoolcache/node/24.14.1/x64/bin/node --conditions node --conditions development --experimental-import-meta-resolve --require /home/REDACTED/work/pabawi/pabawi/node_modules/vitest/suppress-warnings.cjs /home/REDACTED/work/pabawi/pabawi/node_modules/vitest/dist/workers/forks.js (dns block)
    • Triggering command: /opt/hostedtoolcache/node/24.14.1/x64/bin/node /opt/hostedtoolcache/node/24.14.1/x64/bin/node --conditions node --conditions development --experimental-import-meta-resolve --require /home/REDACTED/work/pabawi/pabawi/node_modules/vitest/suppress-warnings.cjs /home/REDACTED/work/pabawi/pabawi/node_modules/vitest/dist/workers/forks.js /home/REDACTED/.ca-c node_modules/vitnpm run build --workspace=backend node_modules/vitest/dist/workers/forks.js de/n�� workspace=backend --workspace=frontend n/vite -I /home/REDACTED/.ca/home/REDACTED/work/_temp/ghcca-node/node/bin/npm -I sh (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Copilot AI review requested due to automatic review settings April 21, 2026 17:03
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 74 out of 95 changed files in this pull request and generated 4 comments.

Comment thread backend/src/server.ts
Comment on lines 1200 to 1212
app.use(
"/api/executions",
authMiddleware,
rateLimitMiddleware,
createExecutionsRouter(executionRepository, executionQueue, batchExecutionService),
);
app.use(
"/api/executions",
streamAuthMiddleware, // resolve ?ticket= / ?token= before auth check
authMiddleware,
rateLimitMiddleware,
createStreamingRouter(streamingManager, executionRepository),
);
Copy link

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

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

/api/executions/:id/stream requests that rely on ?token= / ?ticket= will hit the first /api/executions middleware chain (which runs authMiddleware before streamAuthMiddleware) and get rejected with 401 before reaching the streaming router. To make query-based SSE auth work, streamAuthMiddleware needs to run before any authMiddleware mounted at /api/executions (e.g., include streamAuthMiddleware in the first /api/executions chain, or merge the routers under a single app.use('/api/executions', streamAuthMiddleware, authMiddleware, ...)).

Copilot uses AI. Check for mistakes.
Comment on lines +53 to +72
export function streamAuthMiddleware(
req: Request,
_res: Response,
next: NextFunction,
): void {
if (!req.headers.authorization) {
if (typeof req.query.ticket === "string") {
const ticketData = streamTickets.get(req.query.ticket);
if (ticketData && ticketData.expiresAt >= Date.now()) {
streamTickets.delete(req.query.ticket); // single-use
req.headers.authorization = `Bearer ${ticketData.bearerToken}`;
}
delete (req.query as Record<string, unknown>).ticket;
} else if (typeof req.query.token === "string") {
// Legacy fallback — JWT in URL (kept for backward compatibility)
req.headers.authorization = `Bearer ${req.query.token}`;
delete (req.query as Record<string, unknown>).token;
}
}
next();
Copy link

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

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

streamAuthMiddleware retrieves a StreamTicket that includes executionId/userId, but it never validates that the ticket is being used for the matching /:id/stream request (or that the user matches). This means a valid ticket can be replayed against any route under the mount point to obtain the bearer token. Consider restricting ticket handling to only the stream endpoint(s) and verifying req.params.id === ticketData.executionId (and optionally req.user.userId === ticketData.userId after auth) before accepting the ticket.

Copilot uses AI. Check for mistakes.
Comment thread README.md
Comment on lines 211 to 220
### Version History

- **v1.0.0**: Configuration refactor (`.env` as single source of truth), Proxmox and AWS provisioning, Node Journal, setup wizard `.env` snippet generators, Integration Status Dashboard
- **v0.10.0**: AWS EC2 integration. Integration configuration management
- **v0.9.0**: Proxmox integration. Node journal
- **v0.8.0**: RBAC authentication. SSH integrations. Inventory groups
- **v0.7.0**: Ansible Integration. Class-aware Hiera lookups
- **v0.6.0**: Code consolidation and fixing
- **v0.5.0**: Report filtering, Puppet run history visualization, enhanced expert mode with frontend logging
- **v0.10.0**: AWS EC2 integration, integration configuration management
- **v0.9.0**: Proxmox integration, Node Journal
- **v0.8.0**: RBAC authentication, SSH integration, inventory groups
- **v0.7.0**: Ansible integration, class-aware Hiera lookups
- **v0.6.0**: Code consolidation and fixes
- **v0.5.0**: Report filtering, Puppet run history visualization, enhanced expert mode
- **v0.4.0**: Hiera integration, enhanced plugin architecture
Copy link

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

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

The README still describes the current release as v1.0.0 in the version history section, but this PR updates the app/version labels to v1.1.0. Align the README’s version history (and any other explicit version mentions) with the new release version to avoid confusion.

Copilot uses AI. Check for mistakes.
Comment on lines 383 to 387
<button
type="button"
class="w-full px-4 py-3 text-left hover:bg-gray-50 dark:hover:bg-gray-750 focus:outline-none focus:ring-2 focus:ring-inset focus:ring-primary-500"
onclick={() => hasDetails && toggleExpand(entry.id)}
class="w-full flex items-center gap-2 px-3 py-2 text-left text-sm hover:bg-gray-50 dark:hover:bg-gray-750 rounded-lg border border-transparent hover:border-gray-200 dark:hover:border-gray-700 focus:outline-none focus:ring-2 focus:ring-inset focus:ring-primary-500"
onclick={() => toggleExpand(entry.id)}
aria-expanded={expanded}
Copy link

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

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

The entry row toggles expandedIds on every click, even when hasDetails is false. This makes the button look interactive but does nothing (no chevron and no expanded panel). Consider restoring the guard so expand/collapse is only toggled when there’s actually something to expand (or disable the button when hasDetails is false).

Copilot uses AI. Check for mistakes.
- Global journal view: build nodeNameMap from inventory sourceData to
  display hostnames instead of raw URIs like proxmox:minis:107
- Node journal view: query journal entries by all alternative IDs
  (hostname + source-specific IDs) so entries stored under Proxmox/AWS
  URIs are found when navigating by hostname
- Proxmox task collector: resolve proxmox:node:vmid from sourceData
  when route nodeId is a hostname
- JournalService.getNodeTimeline now accepts string | string[]
- collectExecutionEntries now accepts string | string[]
When more than 3 consecutive entries share the same source, event type,
and summary (e.g. repeated Puppet runs with no changes), they are
collapsed into a single row showing the count badge and time range.
Clicking the group row expands it to show all individual entries with
full detail access. Groups under the threshold remain as individual rows.
@alvagante alvagante merged commit 1699992 into main Apr 21, 2026
5 checks passed
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.

3 participants