Skip to content
Draft
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
24 changes: 24 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,33 @@ Invoke a skill by name in your Agent chat session:
/nethserver-issue
/nethserver-release
/conventional-commit
/nethserver-threat-model
/nethserver-security-audit
/nethserver-security-verify
```

Or reference it naturally:

> "Review this Dockerfile using the nethserver-dockerfile skill"
> "Help me open a PR with nethserver-pr"

## NS8 security audit harness

Three skills form a defensive-security pipeline for auditing `ns8-core` or any
`ns8-<app>` module and validating findings against a live cluster node:

1. **`/nethserver-threat-model`** — map the attack surface (actions,
api-moduled handlers, exposed routes/ports, Redis privilege usage, secret
flows, trust boundaries). Produces `THREAT_MODEL.md`.
2. **`/nethserver-security-audit`** — static audit scoped by the threat model,
covering auth/authorization, action/event input validation, the Redis
privilege boundary, api-moduled handler injection, secrets, and
proxy/X-Forwarded-For trust. Produces `FINDINGS.json` + `FINDINGS.md`.
3. **`/nethserver-security-verify`** — confirm each finding against a live NS8
leader with **read-only** probes (`api-cli`/`runagent`/`podman`/`curl`).
Produces `VERIFIED.md`. Requests live-node access up front; degrades to
static reasoning without it. No state changes unless explicitly authorized.

Run them in order, or run any stage standalone (verify consumes any
`FINDINGS.json`). Container-image hardening is delegated to
`/nethserver-containerfile`.
162 changes: 162 additions & 0 deletions docs/specs/2026-07-10-ns8-security-audit-harness-design.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,162 @@
# NS8 Security Audit Harness — Design

Date: 2026-07-10
Status: Approved (brainstorming), pending implementation plan
Target repo: `nethserver/agents` (adds 3 skills alongside existing 5)

## Goal

A set of Claude Code skills that **audit** an existing NethServer 8 target
(ns8-core **or** any `ns8-<app>` module) for NS8-specific security defects,
then **validate** findings against a live cluster node. Defensive security,
skills-first, matching the `nethserver/agents` `SKILL.md` convention.

Non-goals: design-time authoring guidance, C/C++ memory-bug fuzzing, a
heavyweight Python orchestrator/sandbox pipeline. Container-image hardening is
delegated to the existing `nethserver-containerfile` skill (cross-referenced,
not duplicated).

## NS8 attack surface (what the harness reasons about)

Derived from `ns8-core` component `AGENTS.md` files and module conventions.

- **api-server** (Go/Gin): 14-day JWT with `SECRET` env key; Redis-ACL login;
action-based authz via `filepath.Match` wildcards; **all GET requests bypass
authorization**; optional TOTP 2FA enforced at login; `SENSITIVE_LIST`
redaction; SQLite audit log; WebSocket JWT validation on ping-pong; trusted
proxy / `X-Forwarded-For` handling.
- **api-moduled** (Go): filesystem handlers (`handlers/<name>/post`) executed
with JSON on stdin and `JWT_ID` / `JWT_CLAIMS` env injected; **scope claim
absent = unrestricted token**; `AMLD_JWT_SECRET` must be set;
`AMLD_EXPORT_ENV` leaks host env into handlers. Command-injection and
scope-bypass surface.
- **actions / imageroot** (Python/Bash): numbered step scripts run by the Go
agent; missing `validate-input.json` = unvalidated stdin; injection via
`run_helper` / `agent.tasks.run(p)` / subprocess; `redis_connect(privileged=
True)` privilege boundary; secrets stored in Redis.
- **containers**: rootless / pinned / no-secrets-in-layers — delegated to
`nethserver-containerfile`.
- **live node**: JWT rotation, `/api/login` rate-limiting, 2FA enforcement,
Redis ACL reachability, exposed ports / Traefik routes, actual rootless
container user.

## Architecture — three-stage skill harness

Mirrors the `threat-model → scan → verify` arc of Anthropic's defending-code
harness, but skills-only and NS8-native.

```
nethserver-threat-model → THREAT_MODEL.md
nethserver-security-audit → FINDINGS.json + FINDINGS.md
nethserver-security-verify → VERIFIED.md (needs live node)
```

Each stage is one skill with a single responsibility, communicating through
on-disk artifacts. Stages are independently runnable (verify accepts any
`FINDINGS.json`).

## Repository layout

```
skills/
nethserver-threat-model/
SKILL.md
references/
attack-surface-core.md # api-server/agent/api-moduled surfaces
attack-surface-module.md # actions, handlers, routes, images
nethserver-security-audit/
SKILL.md
references/
taxonomy.md # NS8 threat categories + severities
checks-auth.md # JWT, authz wildcard, GET-bypass, 2FA, scope
checks-action-input.md # validate-input.json, injection sinks
checks-redis.md # privileged=True, ACL, secret keys
checks-api-moduled.md # handler exec, scope, env injection
checks-secrets.md # hardcoded, logs, SENSITIVE_LIST, XFF/proxy
scripts/
scan-antipatterns.sh # grep/rg seeds -> candidate file:line list
nethserver-security-verify/
SKILL.md
references/
probes.md # api-cli/runagent/podman/curl recipes
scripts/
live-probe.sh # read-only leader probes
README.md # add 3 skills to the skills table
```

## Findings schema (contract: audit -> verify)

`FINDINGS.json` — array of objects:

```json
{
"id": "NS8-AUTH-001",
"category": "auth|action-input|redis-privilege|api-moduled|secrets|proxy-trust|container",
"severity": "critical|high|medium|low|info",
"title": "GET route bypasses action authorization",
"location": {"file": "core/api-server/middleware/authorizer.go", "line": 42},
"target_type": "core|module",
"evidence": "code excerpt",
"impact": "what an attacker gains",
"verify_hint": "curl GET /api/... as low-priv user; 200 = confirmed",
"status": "unverified"
}
```

The verify stage rewrites `status` to `confirmed | not-reproducible |
not-applicable`, adds `verify_evidence`, and emits `VERIFIED.md`.

## Skill responsibilities

### nethserver-threat-model
Detect target type (core = presence of `core/api-server`; module = `imageroot/`
+ module `AGENTS.md`). Enumerate: actions and which carry `validate-*.json`,
api-moduled handlers, Traefik routes / exposed ports, every
`redis_connect(privileged=True)`, secret env / Redis keys, trust boundaries.
Output `THREAT_MODEL.md`.

### nethserver-security-audit
Load the threat model, run `scan-antipatterns.sh` for deterministic seeds, then
work each `checks-*.md` domain. **Fans out one subagent per domain, in
parallel** (matches trailofbits `c-review`). Each check = pattern +
why-it-matters + severity + `verify_hint`. Emit `FINDINGS.json` + `FINDINGS.md`.
Container findings cross-reference `nethserver-containerfile`.

### nethserver-security-verify
Inputs: `FINDINGS.json` + a live leader host. **The skill asks the user for
live-node SSH/`api-cli` access up front and states that verification quality is
materially better with it**; without access it degrades to static
reachability reasoning and marks findings `unverified (no live node)`. Per
finding with a `verify_hint`, run the read-only probe via
`api-cli` / `runagent` / `podman top` / curl-Traefik, confirm reachability and
reproduction, write `VERIFIED.md`. Inherits `nethserver8` skill safety rules.

## Live-node safety (non-negotiable)

- Default **read-only**: no `add/remove/update-module`, no Redis writes, no
file edits.
- Any state-changing probe requires explicit `--allow-mutations` and must name
each mutation before running it.
- Preflight: verify host, cluster role, and leader before probing (reuse
`nethserver8` safety rules).
- Structured stdout kept clean; diagnostics to stderr.

## Decisions

- Audit fans out parallel per-domain subagents. (yes)
- Ship `scan-antipatterns.sh` grep-seeder as a deterministic recall floor
alongside LLM analysis. (yes)
- Verify skill proactively requests live-machine access. (yes)

## Testing / acceptance

- **Static**: run the audit against this `ns8-core` checkout and against one
module (`ns8-nethvoice`); it must flag the known `X-Forwarded-For` /
trusted-proxy finding and the GET-request-bypasses-authorization behavior.
- **Live**: run verify against `rl1.leader.default.gs.nethserver.net`,
read-only, confirming probes execute and classify findings without any
mutation.
109 changes: 109 additions & 0 deletions skills/nethserver-security-audit/SKILL.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
---
name: nethserver-security-audit
description: 'Audit a NethServer 8 target (ns8-core or an ns8-<app> module) for NS8-specific security defects: auth/authorization bypass, unvalidated action input, Redis privilege leaks, api-moduled handler injection, secret mishandling, and proxy/XFF trust. Use when the user asks for a security audit/review of NS8 code, or mentions "/security-audit". Produces FINDINGS.json + FINDINGS.md. Second stage of the NS8 security audit harness; consumes THREAT_MODEL.md when present, feeds nethserver-security-verify.'
---

# NethServer 8 Security Audit

## Overview

Second stage of the NS8 security audit harness. Statically audit the target for
NS8-specific security defects and emit machine-readable findings that the
verify stage can confirm against a live node.

Harness order: nethserver-threat-model -> **nethserver-security-audit** ->
nethserver-security-verify.

This skill is read-only on the target. It reads code; it does not modify it.

## Inputs

- The target checkout (ns8-core or an ns8-module).
- `THREAT_MODEL.md` if present (from `nethserver-threat-model`). If absent, run
`nethserver-threat-model` first, or proceed and scope from the code directly
— but say so in the report.

## Step 1 — Seed with the anti-pattern scanner

Run the deterministic grep-seeder to get a candidate list. It is a recall
floor, not the audit itself — it produces `file:line` leads, never verdicts.

```bash
bash scripts/scan-antipatterns.sh <target-dir>
```

Treat every hit as a lead to investigate by reading the surrounding code. Do
not report a scanner hit as a finding without reading and confirming it.

## Step 2 — Audit each domain (fan out in parallel)

Six domains, each with a reference checklist. **Dispatch one subagent per
domain in parallel** (like a parallel-worker code review); each subagent reads
its checklist, examines the relevant entry points from the threat model, and
returns findings in the schema below. If not running subagents, work the
domains sequentially.

| Domain | Checklist | Category tag |
|--------|-----------|--------------|
| Auth / authorization | `references/checks-auth.md` | `auth` |
| Action / event input | `references/checks-action-input.md` | `action-input` |
| Redis privilege | `references/checks-redis.md` | `redis-privilege` |
| api-moduled handlers | `references/checks-api-moduled.md` | `api-moduled` |
| Secrets & proxy trust | `references/checks-secrets.md` | `secrets` / `proxy-trust` |
| Containers | (delegate) | `container` |

Severity rubric lives in `references/taxonomy.md`. Container findings: do not
re-derive image rules — cross-reference the `nethserver-containerfile` skill
and record a single pointer finding per Containerfile that needs its review.

## Step 3 — Every finding needs a verify_hint

A finding is only useful if the verify stage can test it. For each finding,
write a `verify_hint`: a concrete, **read-only** probe against a live node that
would confirm or refute it (e.g. an `api-cli`/curl call and the expected
result). If a finding is not live-testable (pure code-quality), say so in the
hint: `"static-only: <reason>"`.

## Step 4 — Emit findings

Write two files to the target root.

### FINDINGS.json
Array of objects, this exact schema:

```json
{
"id": "NS8-AUTH-001",
"category": "auth|action-input|redis-privilege|api-moduled|secrets|proxy-trust|container",
"severity": "critical|high|medium|low|info",
"title": "short imperative title",
"location": {"file": "relative/path.go", "line": 42},
"target_type": "core|module",
"evidence": "code excerpt or exact pattern matched",
"impact": "what an attacker gains, concretely",
"verify_hint": "read-only probe + expected result, or 'static-only: reason'",
"status": "unverified"
}
```

ID scheme: `NS8-<CATEGORY-ABBR>-<NNN>`, e.g. `NS8-AUTH-001`,
`NS8-REDIS-002`, `NS8-AMLD-001`, `NS8-INPUT-003`, `NS8-SECRET-001`,
`NS8-PROXY-001`.

### FINDINGS.md
Human summary: counts by severity, then one section per finding
(id, severity, location, impact, verify_hint). Order critical -> info.

## Step 5 — Handoff

Report to the user: N findings by severity, and that the next step is
`nethserver-security-verify` against a live node to confirm them. State clearly
that findings are `unverified` until the verify stage runs.

## Rules

- No false confidence: only report what you read and understood. Rank by
severity honestly; do not pad.
- Never modify the target during the audit.
- Prefer precise `file:line`. If a finding spans a pattern, cite the primary
site.
52 changes: 52 additions & 0 deletions skills/nethserver-security-audit/references/checks-action-input.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
# Checks — action & event input (`action-input`)

Scope: action step scripts and event handlers (Python/Bash) in both core and
modules. Input arrives as JSON on stdin (actions) or as an event payload.

## I1. Missing input validation schema
An action directory with no `validate-input.json` accepts **arbitrary
unvalidated JSON** on stdin. Every field the script reads is attacker-influenced
(reachable via `api-cli run [module/<id>/]<action> --data '{...}'`).
- Every action lacking `validate-input.json` = at least a finding; severity
scales with what the script does with the input (see I2/I3).
- verify_hint: `api-cli run <action> --data '{"unexpected":"value"}'` and
observe whether it is rejected pre-execution.

## I2. Command / shell injection sinks
Trace stdin fields to execution sinks:
- Python: `subprocess.*(..., shell=True)`, `os.system`, `os.popen`, f-strings
built into a shell command, `agent.run_helper(...)` with interpolated input.
- Bash: unquoted `"$VAR"` in a command, `eval`, `$(...)` with input, values
passed to `podman`/`systemctl`/`rsync` unsanitized.
- A field flowing into any of these without validation/quoting = high or
critical (critical if the action runs privileged / as root agent).
- verify_hint: send a benign injection marker in the field (e.g. a value that
would create a harmless side effect) — **only on a live TEST node, read-only
intent**; otherwise static-only.

## I3. Path traversal / file write
Stdin fields used as filenames, volume paths, or Redis keys without
canonicalization. `../` traversal, absolute-path override, or key injection
into Redis.
- Severity: high if it writes outside the module boundary.
- verify_hint: `api-cli run <action> --data '{"path":"../../etc/x"}'` on a test
node, expect rejection.

## I4. Output validation & leakage
Missing `validate-output.json`, or scripts that dump internal state/secrets to
stdout. Task output is stored in Redis and readable via the API — treat stdout
as semi-public. Diagnostics must go to stderr (`echo ... >&2`).
- Severity: medium if secrets reach stdout; low for noisy stdout.

## I5. Event payload trust
Event handlers consume payloads produced by other modules/cluster. Apply I1-I3
to event inputs too; a compromised or buggy publisher can feed hostile
payloads.
- Severity: matches the sink reached.

## I6. Numeric / type confusion
JSON Schema present but too loose (e.g. `type: string` with no pattern/enum
where a constrained value is required; missing `additionalProperties: false`).
`additionalProperties` not set to false lets attackers add fields the script
may read.
- Severity: medium; high if a smuggled field reaches a sink.
Loading