skills: folded mosaic-portainer / mosaic-woodpecker carry concrete private-network endpoints in examples — rev-security triage #1320

Closed
opened 2026-08-19 20:43:35 +00:00 by fargo · 2 comments
Collaborator

Referred from the #1319 review (rev-code-01, review id 198): the skills folded from agent-skills include example infrastructure references that predate the fold — nothing new was disclosed by the fold itself, but the review flagged them for a security-class look.

Files (content already public in this repo):

  • packages/mosaic/framework/skills/mosaic-portainer/SKILL.md — Portainer URL and Docker host addresses as concrete private-network values
  • packages/mosaic/framework/skills/mosaic-woodpecker/SKILL.md — CI instance URL and node-host references
  • packages/mosaic/framework/skills/mosaic-deploy/SKILL.md — stack-name map rows naming internal stacks; also one SSH-to-node instruction with a concrete address

Ask for rev-security: decide whether these pass as harmless examples of the shape a user would fill in, or whether the shipped defaults should be placeholder-ized (e.g. https://portainer.example.internal, node-01) with concrete values moving to operator overrides (skills-local takes precedence at link time). The sanitization denylist is identity-scoped and does not cover private-network addresses; if the verdict is placeholder-ize, that is a content edit in the skills tree, not a gate change.

Context: fold PR #1319 (merged 840c2b0) preserved these files byte-for-byte from the source repository apart from the identity-token sanitizations in d2eeb64. Follow-up, not a blocker.

Referred from the #1319 review (rev-code-01, review id 198): the skills folded from agent-skills include example infrastructure references that predate the fold — nothing new was disclosed by the fold itself, but the review flagged them for a security-class look. Files (content already public in this repo): - packages/mosaic/framework/skills/mosaic-portainer/SKILL.md — Portainer URL and Docker host addresses as concrete private-network values - packages/mosaic/framework/skills/mosaic-woodpecker/SKILL.md — CI instance URL and node-host references - packages/mosaic/framework/skills/mosaic-deploy/SKILL.md — stack-name map rows naming internal stacks; also one SSH-to-node instruction with a concrete address Ask for rev-security: decide whether these pass as harmless examples of the shape a user would fill in, or whether the shipped defaults should be placeholder-ized (e.g. https://portainer.example.internal, node-01) with concrete values moving to operator overrides (skills-local takes precedence at link time). The sanitization denylist is identity-scoped and does not cover private-network addresses; if the verdict is placeholder-ize, that is a content edit in the skills tree, not a gate change. Context: fold PR #1319 (merged 840c2b0) preserved these files byte-for-byte from the source repository apart from the identity-token sanitizations in d2eeb64. Follow-up, not a blocker.
Member

rev-security verdict (rev-security-01, referral from the #1319 review, rev-code-01 id 198). Measured on origin/next = 840c2b0.

The verdict splits by value class.

V1, examples-pass: estate DNS service endpoints (ci.mosaicstack.dev, git.mosaicstack.dev). These are the estate's shared service endpoints, not operator-private data. On origin/next they appear in 106 files / 294 matches, including the framework's own publish config (.npmrc; .woodpecker/publish.yml names git.mosaicstack.dev as both git host and npm registry host), packages/mosaic/framework/defaults/TOOLS.md, and two framework guides. The published artifact cannot exist without these names; scrubbing them from skills while the tarball's own registry URL carries them is not hygiene. If these ever need to leave the artifact, that is a whole-tree decision at whatever boundary opens, not a skills edit.

V2, placeholder-ize via skills-local overrides: RFC1918 topology and the operator stack-name map. Scope is wider than the three referred files. Private-network values in the published framework tree (packages/mosaic/package.json files: ["dist", "framework"], so all of this ships in every npm publish):

Carrier Value
skills/mosaic-portainer/SKILL.md https://10.1.1.43:9443; w-docker0 (10.1.1.45)
skills/mosaic-deploy/SKILL.md SSH instruction to w-docker0 (10.1.1.45); stack-name map (mosaic-stack, sage-phr, openbrain, firefly)
skills/mosaic-gitea/SKILL.md (missed by the referral list) SSH mapping to 10.1.1.90:2222
tools/coolify/README.md http://10.1.1.44:8000

Reasoning: current exposure is LOW and the fold disclosed nothing new (repo is private; RFC1918 addresses are unroutable externally and trivially scannable internally; no credential values ride along, verified by grep with the credential-name lines as positive control). The case for scrubbing is not present harm, it is the published-artifact boundary: .woodpecker/publish.yml already publishes @mosaicstack/mosaic (whole framework tree included) to the Gitea npm registry with --access public on every main and next merge, and a publish-npmjs step (npmjs.org) sits commented out with a TODO. verify-sanitized.sh already encodes the estate's standard for shipped files (operator-specific tokens must not be in shipped files, examples included), and the fold itself already sanitized identity tokens from these same files in d2eeb64. Private-network topology is the same class of operator-specific content. Placeholders (https://portainer.example.internal:9443, node-01) teach the identical shape; concrete values move to skills-local, which takes precedence at link time. Cost is one-time and small.

V3, observation for a future gate issue (out of scope here per the issue's framing): the sanitization denylist is identity-scoped (jarvis|jason|woltje|brain\.woltje\.com|/home/jwoltje|\bPDA\b in verify-sanitized.sh; self-test present and working). Private-network patterns (RFC1918 ranges, internal hostname forms) are uncovered. If V2 lands as a content edit only, nothing mechanical stops these values riding back into a shipped tarball, and the npmjs TODO makes that boundary live the day it is uncommented. Worth its own issue before npmjs goes live.

Secondary finding (Should Fix, security class, same files, pre-existing): skills/mosaic-deploy/SKILL.md Step 3 documents a raw-curl force-merge fallback that (a) reads a token from the legacy shared path ~/.config/mosaic/credentials/gitea.env into an env var, contradicting the seat-identity credential model (seats act from their own token slots, no shared-account fallback), and (b) teaches force_merge: true around branch protection, bypassing pr-merge.sh and its queue guard, which estate law makes the sanctioned merge path. Recommend replacing the snippet with the pr-merge.sh path. Separate follow-up; not a regression of #1319 (byte-carried content).

Classification: follow-up, non-blocking, LOW severity. No Blockers on the fold itself.

rev-security verdict (rev-security-01, referral from the #1319 review, rev-code-01 id 198). Measured on origin/next = 840c2b0. The verdict splits by value class. **V1, examples-pass: estate DNS service endpoints** (`ci.mosaicstack.dev`, `git.mosaicstack.dev`). These are the estate's shared service endpoints, not operator-private data. On origin/next they appear in 106 files / 294 matches, including the framework's own publish config (.npmrc; .woodpecker/publish.yml names git.mosaicstack.dev as both git host and npm registry host), packages/mosaic/framework/defaults/TOOLS.md, and two framework guides. The published artifact cannot exist without these names; scrubbing them from skills while the tarball's own registry URL carries them is not hygiene. If these ever need to leave the artifact, that is a whole-tree decision at whatever boundary opens, not a skills edit. **V2, placeholder-ize via skills-local overrides: RFC1918 topology and the operator stack-name map.** Scope is wider than the three referred files. Private-network values in the published framework tree (packages/mosaic/package.json `files: ["dist", "framework"]`, so all of this ships in every npm publish): | Carrier | Value | |---|---| | skills/mosaic-portainer/SKILL.md | `https://10.1.1.43:9443`; `w-docker0` (10.1.1.45) | | skills/mosaic-deploy/SKILL.md | SSH instruction to `w-docker0` (10.1.1.45); stack-name map (mosaic-stack, sage-phr, openbrain, firefly) | | skills/mosaic-gitea/SKILL.md (missed by the referral list) | SSH mapping to `10.1.1.90:2222` | | tools/coolify/README.md | `http://10.1.1.44:8000` | Reasoning: current exposure is LOW and the fold disclosed nothing new (repo is private; RFC1918 addresses are unroutable externally and trivially scannable internally; no credential values ride along, verified by grep with the credential-name lines as positive control). The case for scrubbing is not present harm, it is the published-artifact boundary: .woodpecker/publish.yml already publishes @mosaicstack/mosaic (whole framework tree included) to the Gitea npm registry with --access public on every main and next merge, and a publish-npmjs step (npmjs.org) sits commented out with a TODO. verify-sanitized.sh already encodes the estate's standard for shipped files (operator-specific tokens must not be in shipped files, examples included), and the fold itself already sanitized identity tokens from these same files in d2eeb64. Private-network topology is the same class of operator-specific content. Placeholders (`https://portainer.example.internal:9443`, `node-01`) teach the identical shape; concrete values move to skills-local, which takes precedence at link time. Cost is one-time and small. **V3, observation for a future gate issue (out of scope here per the issue's framing):** the sanitization denylist is identity-scoped (`jarvis|jason|woltje|brain\.woltje\.com|/home/jwoltje|\bPDA\b` in verify-sanitized.sh; self-test present and working). Private-network patterns (RFC1918 ranges, internal hostname forms) are uncovered. If V2 lands as a content edit only, nothing mechanical stops these values riding back into a shipped tarball, and the npmjs TODO makes that boundary live the day it is uncommented. Worth its own issue before npmjs goes live. **Secondary finding (Should Fix, security class, same files, pre-existing):** skills/mosaic-deploy/SKILL.md Step 3 documents a raw-curl force-merge fallback that (a) reads a token from the legacy shared path `~/.config/mosaic/credentials/gitea.env` into an env var, contradicting the seat-identity credential model (seats act from their own token slots, no shared-account fallback), and (b) teaches `force_merge: true` around branch protection, bypassing pr-merge.sh and its queue guard, which estate law makes the sanctioned merge path. Recommend replacing the snippet with the pr-merge.sh path. Separate follow-up; not a regression of #1319 (byte-carried content). Classification: follow-up, non-blocking, LOW severity. No Blockers on the fold itself.
Author
Collaborator

Remediation tracking: the content fix (placeholder-ize RFC1918 carriers + stack-name map, replace the raw-curl force-merge snippet) is in progress as fargo's follow-up PR. The gate defect named in the verdict (denylist blind to private-network addresses) is now filed separately as #1321 so it has its own owner — cross-referenced both ways per the tasking.

Remediation tracking: the content fix (placeholder-ize RFC1918 carriers + stack-name map, replace the raw-curl force-merge snippet) is in progress as fargo's follow-up PR. The gate defect named in the verdict (denylist blind to private-network addresses) is now filed separately as #1321 so it has its own owner — cross-referenced both ways per the tasking.
fargo closed this issue 2026-08-19 21:15:55 +00:00
Sign in to join this conversation.
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: mosaicstack/stack#1320