Skip to content

fix(kits): share the proposal policy and harden proposal scanning - #87

Merged
mrgoonie merged 8 commits into
mainfrom
feat/proposal-scan-hardening
Oct 15, 2026
Merged

mrgoonie merged 8 commits into
mainfrom
feat/proposal-scan-hardening

Conversation

@mrgoonie

@mrgoonie mrgoonie commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

Refs #84 (SkillX side; AgentKit consumption pending). Closes #86. Part of #31.

Shared proposal policy (#84)

  • New skillx.kit-proposal-policy/v1 in packages/contracts/src/fixtures/kit-proposal-policy-v1.json. It holds:
    • the forbidden-key exact and contains lists;
    • key normalization in a fixed order: length limit, percent-decode (3 rounds, refused if an escape remains), only printable ASCII and fullwidth forms, NFKC pinned to Unicode 15.0, printable ASCII, lowercase A-Z, strip non-[a-z0-9];
    • the 64-code-point key limit;
    • the content fields;
    • the refusing content rules;
    • content.secretKeys (password and secret stems for content assignments);
    • the local-path system allowlist as exact entries and subtrees.
  • Shared vectors in kit-proposal-policy-vectors-v1.json, covering:
    • keys: encodings, case and width variants, nested keys, embedded JSON strings, the size limit, envelope fields that must pass;
    • local paths, both refused and allowed;
    • secrets, with the exact rule sets;
    • YAML password blocks.
  • @skillx/contracts exports normalizeProposalKey, isForbiddenProposalKey, findForbiddenProposalKey and secretKeyKind. Key paths escape . and [ (proposal["a.b"]), and a key over the limit is reported as […]. The web envelope now uses them in place of its local list.

Scan hardening (#86)

  • Quadratic regex cost. Every detector is now linear in its input:
    • separator runs are collapsed;
    • scheme names are bounded;
    • each path run is read once;
    • JWTs are checked per token;
    • the dotenv key is tested after capture;
    • YAML blocks are compared through a Set.
  • Linear-growth test. 26 hostile inputs run at 128 KiB and 1 MiB; 8x the text may cost at most 24x the time (best of 3 runs, 5 ms floor). No wall-clock bound. The old code took over 60 s on 200 KB of slashes.
  • Paths the scan now refuses:
    • flag-glued paths (-I/opt/x, -isystem/..., --include=/...);
    • vscode://file/... and other editor file links;
    • segments with non-ASCII letters (/Usuários/...);
    • /usr/home and /usr/local/<org>.
  • Paths that stay allowed:
    • -I/usr/include;
    • the /usr/local system directories;
    • network-scheme URLs (https, ssh, s3, ...).
  • YAML password blocks. Detection now covers list items (- password:) and quoted keys ("password":).
  • Inherited activation gate. A revision is gated when it or any ancestor is session-derived, through any origin (manual, rebase, resolution). See the round-1 decisions below.
  • Shared aliases. Documented that AgentKit kit composition owns the canonical list and that operators set SKILLX_SHARED_SKILL_ALIASES from it. There is no code change.
  • SKILLX_PERSONAL_KITS_API stays "0". No migration.

Round-1 review decisions

  • B1, a manual revision no longer clears the gate. The parent chain is walked through all origins; any session-derived ancestor gates the revision, and a missing ancestor counts as gated (fail closed). Consequence: once a kit has had a session-derived revision, every later activation of that kit needs a web session, and API-key activation and going public are permanently off for that kit.
  • H1, post-activation visibility. The private-only check covers the document's visibility after activation, not only the kit's current visibility (409 proposal.private_kit_required).
  • M1, public needs a human. Any activation or rollback to a public document needs a web session, whatever its origin (403 kit.interactive_approval_required). Gated revisions stay private-only.
  • M2, tighter path allowlist. /usr/src, /proc/self/root, /proc/*/root and /proc/*/cwd are refused. /usr/share, /usr/local/{share,etc,opt} and /etc allow only exact well-known entries, not subtrees. Vectors for each.
  • M3, camelCase secrets. dbPassword, clientSecret, apiKey, accessToken and similar in YAML, JSON, .env and code key: value / key = value are refused (secret-password, new secret-assignment, secret-dotenv), matched through the shared key normalization so content rules and envelope keys agree. Vectors for each.
  • Lows. NFKC Unicode version pinned and the normalization order specified; key paths escape . and [; the path detector decodes %2E and reads zero-width and NFD forms; a linear-ratio cost test replaces the wall-clock bound; the "never activated" docs wording fixed; the URL strip stops at ,, ) and ], so a glued path is still read; YAML anchors and tags (&pw, !secret) before inline and block values; $-prefixed values that are not a whole $VAR count as real; a JSON string layer holding JSON is checked; ssh/sftp URLs to user homes refused.

Round-2 review decisions

  • M1, assignments inside values and flags. The value is read in a lookahead capped at 256 characters, so the scan resumes after the separator (linear) and finds Server=db;User Id=sa;Password=..., ?user=a&password=..., a=1,password=.... - is out of the key lookbehind, so --password=, --client-secret=, --api-key= are caught. Vectors plus hostile cost inputs (a=a=, -a-a-a=, "k":, 128-char keys, long values).
  • M2, the gate follows the document parent lineage. Activation checks the history of every parent revision the lock lineage loads (any kit); one session-derived ancestor anywhere gates the revision, and a missing or unreadable parent fails closed. Test: an API-key private kit whose parent is a session revision of another kit gets 403, and activates from a web session.
  • L1, docs. The gate follows revision history, not content. A session revision that never became effective gates nothing but its rebases, resolutions and child kits; a rollback to an earlier private revision without a session ancestor ends the gate.
  • L2, a new kit always starts private. Creation stores private whatever the first document says; only an activation sets visibility (public needs a web session). Test plus docs.
  • L3, stems with qualifiers. A secret or password stem may be followed by up to two qualifiers (content.secretKeys.qualifiers, or v2/digits): secret_key_base, api_key_prod, private_key_id, passwordValue. :=, => and cfg["password"] = are read. Negative vectors: secretName, passwordless, passwordPolicy, tokenizerName, token_count, secretsManager, --password-file=.
  • L4, paths. / ∕ ⁄ \ read as separators; %25 layers decode up to 3 rounds, and an encoded separator or dot left after that refuses the text.
  • L5. The URL strip also stops at ;, |, &, (, [, {, }.
  • L6. Up to two anchors or tags before a value (&a !s).
  • L7. Covered by the M1 cost inputs.

Validation

  • pnpm test: 71 files, 1763 passed, 2 skipped.
  • pnpm typecheck: clean.
  • pnpm build: ok.

Follow-up (AgentKit)

AgentKit has to vendor both updated JSON files, apply the new normalization order (fullwidth-only NFKC, refuse leftover escapes), the secretKeys stems and qualifiers, the exact/subtrees allowlist shape before upload, and run the vectors in its tests. AgentKit is not changed here.

🤖 Generated with Claude Code

Add skillx.kit-proposal-policy/v1 with the forbidden-key lists, key normalization, the refusing content rules and the local path system allowlist, plus shared vectors that SkillX and AgentKit both test against. Refs #84.
Make every proposal detector linear in its input, refuse flag-glued paths, editor file links, non-ASCII segments, /usr/home and /usr/local/<org>, find YAML password list items and quoted keys, and require a web session to activate rebase or resolution drafts that carry a session-derived revision forward. Refs #86.
A revision is gated when it or any ancestor, through any origin, is
session-derived; a missing ancestor counts as gated. A gated revision
activates only from a web session, on a private kit, as a private
document. Activating any public document now needs a web session.
Pin the key normalization order and Unicode version, escape key paths,
and match password and secret assignments through the shared key
normalization. The path allowlist keeps exact well-known entries and
binary or library subtrees only, and the path detector reads %2E, NFD
and zero-width forms. The cost test asserts linear growth.
Check the history of every parent revision the lock lineage loads, so a
kit whose document parent is a session-derived revision activates only
from a web session. A new kit always starts private.
Read an assignment value in a bounded lookahead so keys inside values and
after flags are found, accept := and =>, and allow up to two qualifiers
after a secret stem. Read Unicode slashes, refuse escapes left after three
decoding rounds, and stop URLs at more delimiters.
@mrgoonie
mrgoonie merged commit 5c41a51 into main Oct 15, 2026
1 check passed
@mrgoonie
mrgoonie deleted the feat/proposal-scan-hardening branch October 15, 2026 10:38
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.

Harden personal kit proposal scanning before enabling the API flag

1 participant