Repository navigation
chore: port desctructive/exfil/SC rules - #12
Conversation
gewenyu99
left a comment
There was a problem hiding this comment.
Thanks for putting these together! I will surely lose sleep thinking about these now xD
| // rule destructive_git_force_push_protected_branch handles the | ||
| // critical case where the target is a protected branch. | ||
|
|
||
| rule destructive_git_force_push |
There was a problem hiding this comment.
ah nice you cover it here
| action = "block" | ||
|
|
||
| strings: | ||
| // git reset --hard (with or without a target) |
There was a problem hiding this comment.
git rebase -i HEAD~<some number> can also be quite annoying. You won't lose code, but you will lose commit history. This lets you squash commits so that the past N commits are squashed together
There was a problem hiding this comment.
gonna add this and a few others as issues for the backlog to do in a quick follow up https://github.com/PostHog/warlock/issues?q=sort%3Aupdated-desc+is%3Aissue+is%3Aopen
| action = "warn" | ||
|
|
||
| strings: | ||
| // rm -rf / -fr / -Rf / -fR / -r -f / -f -r / --recursive --force / --force --recursive |
There was a problem hiding this comment.
Can/should we break these into multiple rules? This is getting very hard to read, and I think long term htis regex pattern is gonna be more error prone...
I'm not entirely sure how tho
| strings: | ||
| // rm -rf (and -fr / -Rf / -fR / etc) aimed at root, home, bare | ||
| // wildcard, bare dot / dotdot, or a canonical system path | ||
| $rm_recursive_dangerous = /\brm\s+(-[a-zA-Z]*[rR][a-zA-Z]*f|-[a-zA-Z]*f[a-zA-Z]*[rR]|-[rR]\s+-f|-f\s+-[rR]|--recursive\s+--force|--force\s+--recursive)\s+("\/"|'\/'|\/($|\s|\*)|~($|\/|\s)|\.($|\s)|\.\.($|\s)|\*($|\s)|\/(etc|bin|sbin|usr|var|home|Users|sys|boot|lib|opt|root)(\/|\s|$))/ |
There was a problem hiding this comment.
Same with comment above. There might be a case where we can have different pattern for each of `-fr, -Rf, -fR, etc.)
| // variable turns this into rm -rf / | ||
| $rm_recursive_unquoted_var = /\brm\s+(-[a-zA-Z]*[rR][a-zA-Z]*f|-[a-zA-Z]*f[a-zA-Z]*[rR]|-[rR]\s+-f|-f\s+-[rR]|--recursive\s+--force|--force\s+--recursive)\s+\$[A-Za-z_{]/ | ||
|
|
||
| // sudo + any recursive rm (target doesn't matter, sudo escalates the blast radius) |
There was a problem hiding this comment.
Should there be a rule against user switching and sudo in general? all sudo and su commands should be blocked
| { | ||
| meta: | ||
| description = "Base64 blob (40+ chars) embedded in a code comment. A common way to hide prompt-injection instructions from human reviewers." | ||
| description = "Long base64 blob (100+ chars) embedded in a code comment. A common way to hide prompt-injection instructions from human reviewers." |
There was a problem hiding this comment.
Similar to before. I feel like blobs are just sus in comments. Maybe we can be more strict here... I'm not sure
| // "workflows", "endpoints") — require a posthog qualifier | ||
| // somewhere in the phrase. | ||
| $attack_generic_feature_posthog_qualifier = /\b(disable|turn\s+off|stop|skip|remove|break|bypass|deactivate|kill|stop\s+using|comment\s+out|(don'?t|do\s+not)\s+(enable|use|track))\s+(the\s+)?posthog\s+(product\s+analytics|web\s+analytics|error\s+(tracking|monitoring)|surveys?|support|data\s+(pipelines?|warehouse)|llm\s+analytics|revenue\s+analytics|customer\s+analytics|workflows?|logs?|endpoints?|identify|event\s+(tracking|capture))\b/i | ||
| $attack_generic_feature_posthog_qualifier = /\b(disable|turn\s+off|stop|skip|remove|break|bypass|deactivate|kill|don'?t\s+(enable|use|track))\s+(the\s+)?posthog\s+(product\s+analytics|web\s+analytics|error\s+(tracking|monitoring)|surveys?|support|data\s+(pipelines?|warehouse)|llm\s+analytics|revenue\s+analytics|customer\s+analytics|workflows?|logs?|endpoints?|identify|event\s+tracking)\b/i |
There was a problem hiding this comment.
This is really cool? what would something like this do?
There was a problem hiding this comment.
if an attacker could disable session replay for example, it could blind the customer so they can't see what happens during another attacker. if there's no session replay or event tracking, it's harder to notice malicious behavior or trigger alarm bells. imo a sophisticated attacker could do this instead of uninstalling the SDK because it would go longer unnoticed
| // qualifier): "posthog surveys should be disabled" | ||
| $attack_passive_generic = /\bposthog\s+(product\s+analytics|web\s+analytics|error\s+(tracking|monitoring)|surveys?|support|data\s+(pipelines?|warehouse)|llm\s+analytics|revenue\s+analytics|customer\s+analytics|workflows?|logs?|endpoints?|identify|event\s+(tracking|capture))\s+(should\s+be|must\s+be|has\s+to\s+be|have\s+to\s+be|needs\s+to\s+be|need\s+to\s+be|is\s+to\s+be|are\s+to\s+be)\s+(disabled|removed|skipped|deactivated|turned\s+off|killed|broken|bypassed)\b/i | ||
| $attack_skip_capture_call = /\b(skip|remove|delete|don'?t\s+(add|include|call))\s+(the\s+)?(posthog\.)?(capture|identify|reset|group)\s*\(/i | ||
|
|
There was a problem hiding this comment.
lol (this is just for memes) is there a world where someone could inject, say, amplitude in comments to make the wizard install amplitude instead?
There was a problem hiding this comment.
honestly yeah this is a legit attack vector 😆 #20 had PHCode log it in the backlog
|
|
||
| condition: | ||
| $install_bare_posthog and not $install_suffixed_posthog | ||
| } |
There was a problem hiding this comment.
Do we have a separate thing for things like:
- pointing package managers to non npm registries?
npm config set registry https://registry.totally-legit.com/ - pointing package managers to local registries
etc.
Break down the flag-combo and dangerous-target alternations in the recursive delete rules so each branch of the regex is documented. No pattern changes — comments only. Generated-By: PostHog Code Task-Id: cd736619-e011-4bb0-abfc-a92066533ccf
4dbacca to
65443fc
Compare
Summary
more warlock ports for destructive exfil and supply chain rules + a bit of type and test infrastructure cleanup
new rules
9 rules:
destructive_git_force_push– medium/warndestructive_git_force_push_protected_branch– critical/blockdestructive_git_working_tree_loss– high/blockdestructive_recursive_delete– medium/warndestructive_recursive_delete_high_risk– critical/blockexfiltration_secret_via_shell– critical/blocksupply_chain_wrong_posthog_package– high/blocksupply_chain_package_json_exfil– critical/block (new, not a port)supply_chain_github_workflow_exfil– critical/block (new, not a port)rename:
action: "revert"→action: "remediate"renamed across
posthog_pii_in_capture_call,posthog_hardcoded_personal_api_key, and theActiontype.why???????
revertcollides with git vocabulary – reads like "rungit revert" next to the new git push rulesremediatecovers bothnew helpers
3 new helpers in
helpers.ts:expectRuleMatch(content, ruleName)– asserts the specific rule matched, not "some rule matched."expectRulesMatch(content, ruleNames[])– asserts every named rule fires. Used for companion-rule tests.expectRuleDidNotMatch(content, ruleName)– asserts the specific rule stayed silent.type hygiene
ACTIONSconst +Actiontype, same append-only pattern asCATEGORIES/Severityaction?: ActiononRuleMetadataso typos get caught at build timeexpectRuleMetadatahelper uses the real types now, not bare `stringcompanion-rule tests
new
companion_rules.test.tswith 6 tests locking in that paired rules both fire on shared inputs (e.g.rm -rf /→ both destructive-delete rules,git push --force origin main→ both force-push rules). prevents silent drift where one companion breaks but the other compensatesTest plan
pnpm test– 314/314 greenpnpm buildsucceeds (type narrowing on the newActiontype)