Skip to content

chore(ci): SHA-pin actions, gate publish/deploy with environments, fix RCE in tag input - #31

Merged
Makisuo merged 1 commit into
mainfrom
security/ci-hardening
May 8, 2026
Merged

Makisuo merged 1 commit into
mainfrom
security/ci-hardening

Conversation

@Makisuo

@Makisuo Makisuo commented May 8, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Addresses DeepSec findings under slugs rce, secrets-exposure, other-ci-supply-chain, iam-permissions (14 HIGH).

  • 14 unique action refs SHA-pinned across 11 workflows (e.g. actions/checkout@34e114876b… with version comment).
  • 11 workflows now declare explicit permissions: (defaults to contents: read where possible).
  • 8 publish/deploy jobs gated on GitHub Environments: publish-images, publish-charts, pr-preview, pullfrog-agent, production, staging, tinybird-cd.
  • RCE fix in publish-otel-collector-maple.yml: inputs.tag was interpolated directly into a run: script. Now passed via INPUT_TAG env var and validated against ^[A-Za-z0-9._-]{1,128}$. Verified by simulation: 6/6 attack inputs (\$(touch /tmp/pwn), tag; rm -rf /, etc.) rejected, 3/3 valid versions accepted.
  • Tinybird tokens scoped to the deploy step instead of workflow-level env:, so install steps run without secrets.
  • PR-preview workflow now requires Environment approval before Doppler secrets reach PR-controlled install/build code.

⚠️ Required repo configuration before merge

The Environments referenced above must be created in Settings → Environments with required-reviewer rules. Without them, jobs run as before (no regression) but without the approval gating the workflow file claims.

Test plan

  • Every uses: line is a 40-char SHA with # v… comment.
  • All 11 workflows have permissions: blocks.
  • 8 publish/deploy jobs declare environment:.
  • Tag-validation regex blocks shell-injection attempts.
  • Reviewer: confirm the named Environments exist in repo Settings before merging.

🤖 Generated with Claude Code


View in Codesmith
Need help on this PR? Tag @codesmith with what you need.

  • Let Codesmith autofix CI failures and bot reviews

…th environments, fix tag injection

Addresses DeepSec findings under slugs `rce`, `secrets-exposure`,
`other-ci-supply-chain`, and `iam-permissions`.

Changes per workflow:
- publish-otel-collector-maple.yml: move `inputs.tag` into env var, add
  regex validator (`^[A-Za-z0-9._-]{1,128}$`), add `environment:
  publish-images`, pin all action SHAs.
- publish-{k8s-infra,maple-otel}-chart.yml: add `environment:
  publish-charts`, pin SHAs.
- deploy-pr-preview.yml: add `environment: pr-preview` so secrets do
  not flow into PR-controlled install/build until reviewed; pin SHAs.
- deploy-{prd,stg}.yml: add `environment: production`/`staging`; pin SHAs.
- pullfrog.yml: add `environment: pullfrog-agent`; pin SHAs.
- tinybird-cd.yml: move TINYBIRD_HOST/TINYBIRD_TOKEN from job-level
  `env:` to deploy step only; install runs without secrets; add
  `permissions: contents: read`; pin SHAs.
- tinybird-ci.yml: same secret scoping; quote `$TINYBIRD_*` in run;
  add permissions block; pin SHAs.
- ci.yml: add `permissions: contents: read`; pin SHAs.
- otel-collector-maple-tests.yml: pin SHAs.

Note: `publish-images`, `publish-charts`, `pr-preview`,
`pullfrog-agent`, `production`, `staging`, and `tinybird-cd`
environments must be created in repo Settings → Environments and
configured with required reviewers / branch protection. The workflow
references these by name; without the repo-side configuration, jobs
will run as before but without approval gating.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@Makisuo
Makisuo merged commit 6e125fb into main May 8, 2026
3 of 4 checks passed
@Makisuo
Makisuo deleted the security/ci-hardening branch May 8, 2026 22:49
Makisuo added a commit that referenced this pull request Aug 4, 2026
Base UI's Menu.GroupLabel throws `MenuGroupContext is missing` (production
error #31) unless a Menu.Group wraps it — a full error-boundary crash on
menu open, not a warning. DropdownMenuLabel and ContextMenuLabel are thin
wrappers over it, so a bare label took out the page.

It had already bitten twice (a warning comment in app-sidebar, a regression
test in dashboard-list) before taking out two more menus:

- PlanetScale "Alert on this" — the reported crash. The dropdown died on
  open, so the alert form was unreachable.
- The issues bulk bar's Severity and "Move to" menus, same cause.

Fix it at the primitive instead of the call site a third time: the group
wrappers set a flag and the labels self-wrap in a Menu.Group only when
nothing above them did. Detecting rather than always wrapping matters — a
nested group would capture the label's id and strip aria-labelledby off the
real group. RadioGroup provides the flag too, and ContextMenu gets the same
guard since it re-exports the identical Base UI parts.

Also keep the prefilled metric selectable in the query panel: the options
list is one fetched page, so a metric arriving by prefill (a widget, or an
"Alert on this" suggestion) often isn't in it, leaving the combobox with no
item to render the selection from.

Menu-open crashes only reproduce under a real render, so each test was
checked to fail without its fix — the packages/ui one reproduces the exact
`MenuGroupContext is missing` error.
Makisuo added a commit that referenced this pull request Aug 17, 2026
The trigger was a bare clock icon, sitting next to a time-range picker
that carries a clock icon of its own — so it read as a second time
control rather than as auto-refresh.

Three changes, all about naming the thing:

- The glyph is now the same reload arrow as the Reload button beside it,
  so the pair reads as one control: reload now, or reload every N.
- The trigger is always labelled "Auto", not just when a cadence is set.
  An icon-only button left the viewer guessing what it did.
- The menu carries an "Auto-refresh" group label, so the control explains
  itself on open — including for screen readers, and for a viewer who
  arrived straight from a `?refresh=` link.

Adds a render test. Base UI's GroupLabel throws production error #31
outside a Group — a full error-boundary crash, and only reachable by
actually opening the menu — so a menu with a label needs a render test
rather than a type check.
Makisuo added a commit that referenced this pull request Aug 17, 2026
* feat(dashboards): configurable auto-refresh interval

Dashboards only refreshed when a viewer clicked Reload, so a board left
on a wall monitor went stale. Add a Grafana-style cadence dropdown next
to Reload, on both the signed-in board and the share page.

Nearly all the machinery already existed with no callers: the
`refreshIntervalSeconds` document field and its closed literal set, the
v2 wire field, the version-history label, `updateDashboardRefreshInterval`,
and `PageRefreshProvider`'s `autoRefreshMs`/`autoRefreshPaused` props.
This wires them up and adds the control.

Resolution is `?refresh=` (per viewer) → the board's saved default → off.
Picking a cadence always writes the param, so a read-only viewer can
start or silence auto-refresh without dirtying the document; in edit mode
it additionally saves the board default, the one path that cuts a version.
Ticks pause while editing or previewing a version, and the existing
hidden-tab guard keeps an idle board from polling.

`refresh` is written as a number so URLs read `?refresh=30` rather than
the `?refresh="30"` TanStack emits to preserve string-ness; the schema
accepts both, and anything outside the literal set falls back instead of
failing the route.

On the share page the resolved window was memoized immutably, so a
relative share would have re-fetched an identical window forever. It now
re-resolves unsnapped on each tick, matching the signed-in board. Single
-widget shares also keep the cadence through redaction, which previously
only the whole-board branch carried.

* fix(dashboards): make the auto-refresh control read as auto-refresh

The trigger was a bare clock icon, sitting next to a time-range picker
that carries a clock icon of its own — so it read as a second time
control rather than as auto-refresh.

Three changes, all about naming the thing:

- The glyph is now the same reload arrow as the Reload button beside it,
  so the pair reads as one control: reload now, or reload every N.
- The trigger is always labelled "Auto", not just when a cadence is set.
  An icon-only button left the viewer guessing what it did.
- The menu carries an "Auto-refresh" group label, so the control explains
  itself on open — including for screen readers, and for a viewer who
  arrived straight from a `?refresh=` link.

Adds a render test. Base UI's GroupLabel throws production error #31
outside a Group — a full error-boundary crash, and only reachable by
actually opening the menu — so a menu with a label needs a render test
rather than a type check.

* refactor(dashboards): join reload and cadence into one split button

Reload and the cadence dropdown were two separate outline buttons sitting
next to each other, which is what made the cadence half ambiguous — an
adjacent control is just another control, and beside the time-range
picker it read as a second time control.

Grafana attaches them, and attachment is what carries the meaning: "30s"
welded to "Reload" can only be read one way. So the two halves are now
one split button sharing a seam, and the cadence half is back to showing
just the interval (or "Off") rather than explaining itself with a word.

`RefreshControls` takes the reload action as a prop and `PageRefreshControls`
binds it to the page refresh context, because the share page drives its
own refresh and has no such context. `ReloadControls` stays as-is for
traces, metrics, service-map and infra, which have no cadence to offer.

* fix(dashboards): stop the grid indenting tiles by a gutter

react-grid-layout's `containerPadding` defaults to `margin`, so the grid
padded its own outside edge with a full gutter on top of whatever padding
the page had already applied. Tiles ended up 12px inside everything
stacked above them — section headers, the page title, and on a shared
board the time-range label and refresh controls sitting directly over a
left edge that did not line up with them.

The gutter belongs between tiles; the surrounding layout owns the outer
padding. Horizontal container padding is now zero, so a tile's left edge
meets its container. Vertical keeps the margin, leaving the gap under a
section header exactly as it was.

Tiles gain a gutter of width on each side as a result, which is the
correct amount of room and not a resize: the column count and every
stored layout are untouched.

This branch was previously deployed

1 inactive deployment
pr-preview — c4b8c050 Deployed May 8, 2026 by Makisuo via deploy-pr-preview #104
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.

1 participant