Skip to content

Security hardening - prevent command injection, limit SSE connections, harden container#39

Open
keithdealwis-ui wants to merge 1 commit intocalesthio:masterfrom
keithdealwis-ui:security-hardening
Open

Security hardening - prevent command injection, limit SSE connections, harden container#39
keithdealwis-ui wants to merge 1 commit intocalesthio:masterfrom
keithdealwis-ui:security-hardening

Conversation

@keithdealwis-ui
Copy link
Copy Markdown

Summary

Three security improvements to harden Crucix against common attack vectors:

  1. Command injection prevention - Validate PORT environment variable
  2. DoS mitigation - Limit concurrent SSE connections
  3. Container security - Fix health check + non-root user

All changes are non-breaking and backward compatible.


1. Command Injection Prevention (crucix.config.mjs)

Issue: PORT env var is passed unsanitized to child_process.exec() when auto-opening browser:

exec(`${openCmd} "http://localhost:${config.port}"`, ...);

Attack vector: Malicious .env file could inject shell commands:

PORT=3117"; rm -rf / #

Fix: Validate PORT is numeric and within safe range (1024-65535) before use.

CWE: CWE-78: OS Command Injection


2. Denial of Service Mitigation (server.mjs)

Issue: /events SSE endpoint has no connection limit:

app.get('/events', (req, res) => {
  sseClients.add(res);  // No limit
});

Attack vector: Attacker opens 10,000+ connections → memory exhaustion → process crash.

Fix: Return HTTP 503 when sseClients.size >= 100.

CWE: CWE-400: Uncontrolled Resource Consumption


3. Container Security (Dockerfile)

Issues:

  • Health check uses wget but node:22-alpine doesn't include it → silent failure
  • Container runs as root → unnecessary privilege escalation risk

Fixes:

  • Install wget via apk add --no-cache wget
  • Create non-root user crucix:1001 and run as that user

CWE: CWE-250: Execution with Unnecessary Privileges


Testing

All changes tested on production deployment:

  • ✅ PORT validation rejects invalid values (tested with non-numeric input)
  • ✅ SSE endpoint returns 503 after 100 connections
  • ✅ Docker health check passes (tested with docker compose up)
  • ✅ Container runs as non-root user (verified with docker exec ... whoami)

Impact

  • Severity: Medium (command injection) + Low (DoS, privilege escalation)
  • Scope: All deployments exposed to untrusted .env files or public internet
  • Breaking changes: None
  • Performance impact: Negligible (single integer comparison per SSE connection)

Happy to address any feedback or concerns.

- Validate PORT env var to prevent command injection via exec()
- Add 100 connection limit to SSE endpoint to prevent DoS
- Install wget in Dockerfile for health checks
- Add non-root user (crucix:1001) to container

Co-Authored-By: Claude Code <[email protected]>
@calesthio
Copy link
Copy Markdown
Owner

I haven't been able to get to this yet because work has been busy, but I definitely plan to review it over the weekend.

schergr pushed a commit to schergr/Crucix that referenced this pull request Mar 23, 2026
- Replace single &calesthio#39; handler with generic numeric/hex entity decoder
  so &calesthio#39; and other unpadded entities are properly converted
- Dedup urgent OSINT posts against all hot memory runs (last 3 sweeps)
  instead of only the previous sweep, preventing posts that drop out
  of one sweep from reappearing as "new" in the next

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
@calesthio
Copy link
Copy Markdown
Owner

This is a worthwhile hardening change overall.

Running the container as non-root is a real improvement, and I think the unprivileged-port policy is reasonable if the intended deployment model is "Crucix binds to 3117 internally and 80/443 are handled by Docker port mapping or a reverse proxy."

Requested changes before merge:

  1. Please make validatePort() treat an unset/empty PORT as the normal default path without warning. Right now ordinary startup logs [Crucix] Invalid PORT, using default 3117, which makes a healthy default config look broken.

  2. Please make the validation strict instead of parseInt()-loose. Values like 3117junk currently get accepted as 3117; those should be rejected.

  3. Please document the intended deployment policy clearly: app binds only to unprivileged ports (1024-65535), and 80/443 should be terminated by Docker port mapping or a reverse proxy.

Once those are addressed, I think this PR is in good shape.

mdrong22 pushed a commit to mdrong22/Crucix-Main that referenced this pull request Mar 25, 2026
commit 53f6d81
Merge: 8c1ea37 5c08355
Author: Calesthio <[email protected]>
Date:   Wed Mar 25 10:21:02 2026 -0700

    Merge pull request calesthio#68 from schergr/fix/osint-signal-truncation

    Fix remaining OSINT signal text truncation

commit 5c08355
Author: calesthio <[email protected]>
Date:   Tue Mar 24 18:48:55 2026 -0700

    Fix Telegram dedup identity and legacy Markdown escaping

commit b7322f1
Author: Greg Scher <[email protected]>
Date:   Mon Mar 23 13:01:32 2026 -0400

    Fix HTML entity decoding and broaden OSINT dedup window

    - Replace single &calesthio#39; handler with generic numeric/hex entity decoder
      so &calesthio#39; and other unpadded entities are properly converted
    - Dedup urgent OSINT posts against all hot memory runs (last 3 sweeps)
      instead of only the previous sweep, preventing posts that drop out
      of one sweep from reappearing as "new" in the next

    Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

commit 31c305c
Author: Greg Scher <[email protected]>
Date:   Mon Mar 23 12:57:37 2026 -0400

    Escape Markdown in alert signals and cap OSINT text in ideas prompt

    Addresses PR review: escape Markdown-sensitive characters in
    _formatTieredAlert signal bullets to prevent Telegram Bot API
    rejections, and add a 1500-char budget for URGENT_OSINT in
    compactSweepForLLM to bound prompt size while keeping full text upstream.

    Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

commit 2d166c2
Author: Greg Scher <[email protected]>
Date:   Sat Mar 21 12:59:30 2026 -0400

    Remove remaining text truncation across delta engine, memory, and ideas

    The prior fix (753c676) only removed truncation at source ingestion and
    alert formatting. Signals were still being cut to 120 chars in the delta
    engine, 80 chars in memory snapshots, and 120 chars in the ideas LLM
    context — so OSINT posts arrived at the alerter already truncated.

    Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

commit 753c676
Author: Greg Scher <[email protected]>
Date:   Fri Mar 20 16:49:58 2026 -0400

    Remove text truncation limits from Telegram posts

    Posts were being cut to 300 chars (source ingestion) and 150 chars
    (alert evaluation), losing valuable OSINT context. The sendMessage
    chunker already handles the 4096-char Telegram API limit.

    Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
mdrong22 pushed a commit to mdrong22/Crucix-Main that referenced this pull request Mar 25, 2026
commit 53f6d81
Merge: 8c1ea37 5c08355
Author: Calesthio <[email protected]>
Date:   Wed Mar 25 10:21:02 2026 -0700

    Merge pull request calesthio#68 from schergr/fix/osint-signal-truncation

    Fix remaining OSINT signal text truncation

commit 5c08355
Author: calesthio <[email protected]>
Date:   Tue Mar 24 18:48:55 2026 -0700

    Fix Telegram dedup identity and legacy Markdown escaping

commit b7322f1
Author: Greg Scher <[email protected]>
Date:   Mon Mar 23 13:01:32 2026 -0400

    Fix HTML entity decoding and broaden OSINT dedup window

    - Replace single &calesthio#39; handler with generic numeric/hex entity decoder
      so &calesthio#39; and other unpadded entities are properly converted
    - Dedup urgent OSINT posts against all hot memory runs (last 3 sweeps)
      instead of only the previous sweep, preventing posts that drop out
      of one sweep from reappearing as "new" in the next

    Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

commit 31c305c
Author: Greg Scher <[email protected]>
Date:   Mon Mar 23 12:57:37 2026 -0400

    Escape Markdown in alert signals and cap OSINT text in ideas prompt

    Addresses PR review: escape Markdown-sensitive characters in
    _formatTieredAlert signal bullets to prevent Telegram Bot API
    rejections, and add a 1500-char budget for URGENT_OSINT in
    compactSweepForLLM to bound prompt size while keeping full text upstream.

    Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

commit 2d166c2
Author: Greg Scher <[email protected]>
Date:   Sat Mar 21 12:59:30 2026 -0400

    Remove remaining text truncation across delta engine, memory, and ideas

    The prior fix (753c676) only removed truncation at source ingestion and
    alert formatting. Signals were still being cut to 120 chars in the delta
    engine, 80 chars in memory snapshots, and 120 chars in the ideas LLM
    context — so OSINT posts arrived at the alerter already truncated.

    Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

commit 753c676
Author: Greg Scher <[email protected]>
Date:   Fri Mar 20 16:49:58 2026 -0400

    Remove text truncation limits from Telegram posts

    Posts were being cut to 300 chars (source ingestion) and 150 chars
    (alert evaluation), losing valuable OSINT context. The sendMessage
    chunker already handles the 4096-char Telegram API limit.

    Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
mdrong22 pushed a commit to mdrong22/Crucix-Main that referenced this pull request Mar 25, 2026
commit 8d99875
Author: Matt <[email protected]>
Date:   Wed Mar 25 14:19:07 2026 -0500

    updated commodities

commit a9d77d9
Author: Matt-Drong <[email protected]>
Date:   Wed Mar 25 13:44:05 2026 -0500

    Squashed commit of the following:

    commit 53f6d81
    Merge: 8c1ea37 5c08355
    Author: Calesthio <[email protected]>
    Date:   Wed Mar 25 10:21:02 2026 -0700

        Merge pull request calesthio#68 from schergr/fix/osint-signal-truncation

        Fix remaining OSINT signal text truncation

    commit 5c08355
    Author: calesthio <[email protected]>
    Date:   Tue Mar 24 18:48:55 2026 -0700

        Fix Telegram dedup identity and legacy Markdown escaping

    commit b7322f1
    Author: Greg Scher <[email protected]>
    Date:   Mon Mar 23 13:01:32 2026 -0400

        Fix HTML entity decoding and broaden OSINT dedup window

        - Replace single &calesthio#39; handler with generic numeric/hex entity decoder
          so &calesthio#39; and other unpadded entities are properly converted
        - Dedup urgent OSINT posts against all hot memory runs (last 3 sweeps)
          instead of only the previous sweep, preventing posts that drop out
          of one sweep from reappearing as "new" in the next

        Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

    commit 31c305c
    Author: Greg Scher <[email protected]>
    Date:   Mon Mar 23 12:57:37 2026 -0400

        Escape Markdown in alert signals and cap OSINT text in ideas prompt

        Addresses PR review: escape Markdown-sensitive characters in
        _formatTieredAlert signal bullets to prevent Telegram Bot API
        rejections, and add a 1500-char budget for URGENT_OSINT in
        compactSweepForLLM to bound prompt size while keeping full text upstream.

        Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

    commit 2d166c2
    Author: Greg Scher <[email protected]>
    Date:   Sat Mar 21 12:59:30 2026 -0400

        Remove remaining text truncation across delta engine, memory, and ideas

        The prior fix (753c676) only removed truncation at source ingestion and
        alert formatting. Signals were still being cut to 120 chars in the delta
        engine, 80 chars in memory snapshots, and 120 chars in the ideas LLM
        context — so OSINT posts arrived at the alerter already truncated.

        Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

    commit 753c676
    Author: Greg Scher <[email protected]>
    Date:   Fri Mar 20 16:49:58 2026 -0400

        Remove text truncation limits from Telegram posts

        Posts were being cut to 300 chars (source ingestion) and 150 chars
        (alert evaluation), losing valuable OSINT context. The sendMessage
        chunker already handles the 4096-char Telegram API limit.

        Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

commit b54ddce
Author: Matt <[email protected]>
Date:   Wed Mar 25 02:22:48 2026 -0500

    sure why not
mdrong22 pushed a commit to mdrong22/Crucix-Main that referenced this pull request Mar 29, 2026
commit 212fefd
Merge: 3f3d050 1b80539
Author: Calesthio <[email protected]>
Date:   Sat Mar 28 23:14:48 2026 -0700

    Merge pull request calesthio#85 from calesthio/codex/gold-silver-macro

    feat: add gold and silver to macro + markets context

commit 1b80539
Author: calesthio <[email protected]>
Date:   Sat Mar 28 22:53:23 2026 -0700

    feat(markets): surface gold and silver across dashboard and briefs

commit 3f3d050
Merge: 53f6d81 1fb5093
Author: Calesthio <[email protected]>
Date:   Sat Mar 28 22:41:17 2026 -0700

    Merge pull request calesthio#62 from 3byss/master

    feat: LLM provider for Grok

commit 1fb5093
Author: calesthio <[email protected]>
Date:   Sat Mar 28 22:36:33 2026 -0700

    fix(grok): align token caps and docs

commit 18233bc
Merge: 457dba2 e9b04b2
Author: 3byss <[email protected]>
Date:   Fri Mar 27 22:23:07 2026 -0600

    Merge remote-tracking branch 'refs/remotes/origin/master'

commit 457dba2
Author: 3byss <[email protected]>
Date:   Fri Mar 27 22:22:21 2026 -0600

    Fixed any issues with the PR

commit e9b04b2
Merge: caf4430 53f6d81
Author: 3byss <[email protected]>
Date:   Fri Mar 27 22:19:00 2026 -0600

    Merge branch 'calesthio:master' into master

commit 9028d42
Merge: caf4430 53f6d81
Author: 3byss <[email protected]>
Date:   Fri Mar 27 22:13:37 2026 -0600

    Merge remote-tracking branch 'upstream/master'

commit 53f6d81
Merge: 8c1ea37 5c08355
Author: Calesthio <[email protected]>
Date:   Wed Mar 25 10:21:02 2026 -0700

    Merge pull request calesthio#68 from schergr/fix/osint-signal-truncation

    Fix remaining OSINT signal text truncation

commit 5c08355
Author: calesthio <[email protected]>
Date:   Tue Mar 24 18:48:55 2026 -0700

    Fix Telegram dedup identity and legacy Markdown escaping

commit b7322f1
Author: Greg Scher <[email protected]>
Date:   Mon Mar 23 13:01:32 2026 -0400

    Fix HTML entity decoding and broaden OSINT dedup window

    - Replace single &calesthio#39; handler with generic numeric/hex entity decoder
      so &calesthio#39; and other unpadded entities are properly converted
    - Dedup urgent OSINT posts against all hot memory runs (last 3 sweeps)
      instead of only the previous sweep, preventing posts that drop out
      of one sweep from reappearing as "new" in the next

    Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

commit 31c305c
Author: Greg Scher <[email protected]>
Date:   Mon Mar 23 12:57:37 2026 -0400

    Escape Markdown in alert signals and cap OSINT text in ideas prompt

    Addresses PR review: escape Markdown-sensitive characters in
    _formatTieredAlert signal bullets to prevent Telegram Bot API
    rejections, and add a 1500-char budget for URGENT_OSINT in
    compactSweepForLLM to bound prompt size while keeping full text upstream.

    Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

commit caf4430
Merge: cb27575 8c1ea37
Author: 3byss <[email protected]>
Date:   Sat Mar 21 16:17:13 2026 -0600

    Merge branch 'master' into master

commit 2d166c2
Author: Greg Scher <[email protected]>
Date:   Sat Mar 21 12:59:30 2026 -0400

    Remove remaining text truncation across delta engine, memory, and ideas

    The prior fix (753c676) only removed truncation at source ingestion and
    alert formatting. Signals were still being cut to 120 chars in the delta
    engine, 80 chars in memory snapshots, and 120 chars in the ideas LLM
    context — so OSINT posts arrived at the alerter already truncated.

    Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

commit cb27575
Author: 3byss <[email protected]>
Date:   Fri Mar 20 16:26:30 2026 -0600

    Fixed types and created tests

commit 753c676
Author: Greg Scher <[email protected]>
Date:   Fri Mar 20 16:49:58 2026 -0400

    Remove text truncation limits from Telegram posts

    Posts were being cut to 300 chars (source ingestion) and 150 chars
    (alert evaluation), losing valuable OSINT context. The sendMessage
    chunker already handles the 4096-char Telegram API limit.

    Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

commit 17b8d9e
Author: 3byss <[email protected]>
Date:   Fri Mar 20 00:46:44 2026 -0600

    Finished the Grok LLM implementation
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.

2 participants