Skip to content
Merged

050 #15

Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions .env.example
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
# Application
PORT=3000
LOG_LEVEL=info

# Bolt command whitelist
BOLT_COMMAND_WHITELIST_ALLOW_ALL=false
BOLT_COMMAND_WHITELIST=["ls","pwd","whoami","uptime"]
BOLT_EXECUTION_TIMEOUT=300000

# PuppetDB integration (optional)
PUPPETDB_ENABLED=false
PUPPETDB_SERVER_URL=
PUPPETDB_PORT=8081
PUPPETDB_TOKEN=
PUPPETDB_SSL_ENABLED=false
PUPPETDB_SSL_CA=
PUPPETDB_SSL_CERT=
PUPPETDB_SSL_KEY=

# Puppetserver integration (optional)
PUPPETSERVER_ENABLED=false
PUPPETSERVER_SERVER_URL=
PUPPETSERVER_PORT=8140
PUPPETSERVER_TOKEN=
PUPPETSERVER_SSL_ENABLED=false
PUPPETSERVER_SSL_CA=
PUPPETSERVER_SSL_CERT=
PUPPETSERVER_SSL_KEY=

# Hiera integration (optional)
HIERA_ENABLED=false
HIERA_CONTROL_REPO_PATH=
HIERA_CONFIG_PATH=hiera.yaml
Comment on lines +1 to +33
Copy link

Copilot AI Jan 25, 2026

Choose a reason for hiding this comment

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

The addition of .env.example is a good practice, but it should be mentioned in the CHANGELOG under "Added" section for version 0.5.0, as this is a new file that helps users configure the application. This improves documentation and onboarding for new users.

Copilot uses AI. Check for mistakes.
Comment on lines +1 to +33
Copy link

Copilot AI Jan 25, 2026

Choose a reason for hiding this comment

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

The root-level .env.example file is missing many important configuration options that exist in backend/.env.example, such as BOLT_PROJECT_PATH, DATABASE_PATH, cache configuration, streaming configuration, and many integration-specific settings. Users following the docker-compose.yml setup will need these configuration options. Consider either:

  1. Making the root .env.example comprehensive (copy from backend/.env.example), or
  2. Adding a prominent comment directing users to backend/.env.example for the full list of configuration options, or
  3. Documenting in docker-deployment.md that users should use backend/.env.example as their starting point.

The current minimal example may lead to configuration issues for users trying to set up integrations via docker-compose.

Copilot uses AI. Check for mistakes.
57 changes: 47 additions & 10 deletions .github/SECURITY_AUDIT.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,15 @@ This security audit identifies several **CRITICAL** vulnerabilities and security

**Issue:**
The application has **zero authentication or authorization mechanisms**. Anyone who can access the endpoint can:

- Execute arbitrary commands on infrastructure
- View sensitive system information
- Trigger Puppet runs
- Access execution history
- View Hiera data (potentially containing secrets)

**Evidence:**

```markdown
# From README.md lines 60-65
- **No Built-in Authentication**: Pabawi currently has no user authentication or authorization system
Expand All @@ -46,17 +48,20 @@ The application has **zero authentication or authorization mechanisms**. Anyone
```

**Attack Scenario:**

1. Attacker gains network access to server running Pabawi
2. Sends requests to `http://server:3000/api/nodes/{node}/command`
3. Executes malicious commands on entire infrastructure

**Recommendation:**

- **IMMEDIATE:** Add authentication middleware (JWT, OAuth2, or API keys)
- Implement role-based access control (RBAC)
- Add audit logging for all privileged operations
- Consider implementing request signing for API calls

**Mitigation (Short-term):**

```typescript
// backend/src/middleware/auth.ts
export function requireAuth(req: Request, res: Response, next: NextFunction) {
Expand All @@ -80,6 +85,7 @@ export function requireAuth(req: Request, res: Response, next: NextFunction) {
While `spawn()` is used with `shell: false`, the argument escaping in `buildCommandString()` may be insufficient. Arguments are passed directly to Bolt CLI without proper sanitization beyond quote escaping.

**Evidence:**

```typescript
// backend/src/bolt/BoltService.ts:74-86
private buildCommandString(args: string[]): string {
Expand All @@ -101,12 +107,14 @@ childProcess = spawn("bolt", args, {
```

**Vulnerabilities:**

1. Arguments not validated for shell metacharacters (`;`, `|`, `&`, `$()`, etc.)
2. Environment variable `process.env` passed without sanitization
3. No validation of argument count or structure
4. Command whitelist bypassed if `COMMAND_WHITELIST_ALLOW_ALL=true`

**Attack Scenario:**

```bash
# Attacker crafts malicious task parameter
POST /api/nodes/target/task
Expand All @@ -119,13 +127,15 @@ POST /api/nodes/target/task
```

**Recommendation:**

- Implement strict input validation for all Bolt arguments
- Use allowlist for permitted characters in parameters
- Never pass entire `process.env` - use explicit environment variables
- Add argument structure validation
- Log all command executions with full context

**Mitigation:**

```typescript
// Validate arguments against injection patterns
private validateArgument(arg: string): void {
Expand All @@ -148,6 +158,7 @@ private validateArgument(arg: string): void {
CORS configuration not found in the visible code sections, but the application serves both frontend and API. If CORS is too permissive, it allows any origin to make authenticated requests.

**Current Evidence:**

```typescript
// backend/src/server.ts line 1 imports cors
import cors from "cors";
Expand All @@ -156,11 +167,13 @@ import cors from "cors";
```

**Recommendation:**

- Restrict CORS to specific origins only
- Never use `Access-Control-Allow-Origin: *` in production
- Implement CORS whitelist based on environment

**Proper Configuration:**

```typescript
app.use(cors({
origin: process.env.ALLOWED_ORIGINS?.split(',') || ['http://localhost:3000'],
Expand All @@ -184,13 +197,15 @@ app.use(cors({
No security headers detected (helmet.js not found in dependencies or code).

**Missing Headers:**

- `X-Frame-Options: DENY` (Clickjacking protection)
- `X-Content-Type-Options: nosniff` (MIME sniffing protection)
- `Strict-Transport-Security` (HTTPS enforcement)
- `Content-Security-Policy` (XSS protection)
- `X-XSS-Protection: 1; mode=block`

**Recommendation:**

```bash
npm install --save helmet
```
Expand Down Expand Up @@ -227,6 +242,7 @@ app.use(helmet({
While parameterized queries are used, there's risk in dynamic query construction and JSON field searching.

**Evidence:**

```typescript
// backend/src/database/ExecutionRepository.ts:121-150
const sql = `
Expand All @@ -248,11 +264,13 @@ const params = [
```

**Concerns:**

1. JSON fields queried with `LIKE` (noted in architecture.md as not efficiently indexable)
2. No prepared statement caching
3. String concatenation in filter queries could be risky

**Recommendation:**

- Review all filter/search implementations for SQL injection
- Use ORM or query builder (e.g., Knex.js) for complex queries
- Never concatenate user input into SQL strings
Expand All @@ -270,6 +288,7 @@ const params = [
Configuration paths (`BOLT_PROJECT_PATH`, `DATABASE_PATH`, Hiera `controlRepoPath`) are read from environment variables without validation.

**Evidence:**

```dotenv
# .env
BOLT_PROJECT_PATH=~/bolt-project
Expand All @@ -278,13 +297,15 @@ HIERA_CONTROL_REPO_PATH=/path/to/control-repo
```

**Attack Scenario:**

```bash
# Attacker modifies environment or config
BOLT_PROJECT_PATH=../../../etc
DATABASE_PATH=../../../tmp/malicious.db
```

**Recommendation:**

```typescript
// backend/src/config/ConfigService.ts
import path from 'path';
Expand All @@ -310,6 +331,7 @@ function validatePath(inputPath: string, basePath: string): string {
Sensitive data (tokens, SSL certificates) stored in environment variables and potentially logged.

**Evidence:**

```typescript
// backend/src/config/ConfigService.ts:124-147
token: process.env.PUPPETDB_TOKEN,
Expand All @@ -322,12 +344,14 @@ ssl: {
```

**Concerns:**

1. Tokens logged in startup metadata
2. SSL certificates passed as environment strings (should be file paths)
3. No secret rotation mechanism
4. Expert mode may leak tokens in API responses

**Recommendation:**

- Use secret management (HashiCorp Vault, AWS Secrets Manager)
- Store certificate paths, not content, in environment
- Implement secret rotation
Expand Down Expand Up @@ -368,6 +392,7 @@ While Zod is used for validation, some edge cases may not be covered:
4. **File uploads**: No validation (if implemented)

**Recommendation:**

- Add maximum length constraints to all string inputs
- Validate node ID format (alphanumeric + hyphens/underscores only)
- Sanitize PQL queries or use parameterized queries
Expand All @@ -383,6 +408,7 @@ While Zod is used for validation, some edge cases may not be covered:
Error handler exposes stack traces and internal details even in non-expert mode.

**Evidence:**

```typescript
// Line 48-50
if (process.env.NODE_ENV === "development") {
Expand All @@ -391,6 +417,7 @@ if (process.env.NODE_ENV === "development") {
```

**Recommendation:**

- Never expose stack traces in production
- Sanitize error messages sent to client
- Log detailed errors server-side only
Expand All @@ -404,11 +431,13 @@ if (process.env.NODE_ENV === "development") {

**Issue:**
No rate limiting allows:

- Brute force attacks (if authentication added)
- DoS via expensive operations (catalog compilation, report queries)
- Resource exhaustion

**Recommendation:**

```bash
npm install --save express-rate-limit
```
Expand Down Expand Up @@ -442,12 +471,14 @@ app.use('/api/nodes/:id/command', executionLimiter);
ANSI output converted to HTML may be vulnerable to XSS if malicious ANSI codes are crafted.

**Evidence:**

```typescript
// ansiToHtml.ts - converts ANSI to inline styles
// Strings are not explicitly sanitized before HTML insertion
```

**Recommendation:**

- Escape HTML entities before applying styles
- Use DOMPurify to sanitize output
- Never use `innerHTML` without sanitization (only found in test file)
Expand All @@ -468,6 +499,7 @@ childProcess = spawn("bolt", args, {
```

**Recommendation:**

```typescript
// Only pass necessary variables
const cleanEnv = {
Expand All @@ -494,6 +526,7 @@ childProcess = spawn("bolt", args, {
Execution commands, node IDs, and parameters logged extensively. Could leak sensitive data.

**Recommendation:**

- Implement log level filtering
- Redact sensitive parameters
- Rotate logs with retention policy
Expand All @@ -505,16 +538,19 @@ Execution commands, node IDs, and parameters logged extensively. Could leak sens
**Location:** `package.json`

**Issue:**

- Override for `tar: 7.5.6` suggests known vulnerability
- No automated dependency scanning in CI

**Recommendation:**

```bash
npm audit
npm audit fix
```

Add to CI:

```yaml
- name: Security audit
run: npm audit --audit-level=moderate
Expand All @@ -530,6 +566,7 @@ Add to CI:
No HTTPS configuration or HTTP→HTTPS redirect.

**Recommendation:**

- Add TLS configuration for production
- Redirect HTTP to HTTPS
- Use Let's Encrypt for certificates
Expand All @@ -548,19 +585,19 @@ No HTTPS configuration or HTTP→HTTPS redirect.

### Short-term (1-2 weeks)

6. Add rate limiting to all endpoints
7. Implement secret management
8. Add SQL injection protection tests
9. Sanitize error messages in production
10. Add request size limits
1. Add rate limiting to all endpoints
2. Implement secret management
3. Add SQL injection protection tests
4. Sanitize error messages in production
5. Add request size limits

### Medium-term (1 month)

11. Implement RBAC
12. Add audit logging
13. Security scanning in CI/CD
14. Penetration testing
15. Security documentation
1. Implement RBAC
2. Add audit logging
3. Security scanning in CI/CD
4. Penetration testing
5. Security documentation

---

Expand Down
Loading
Loading