Skip to content

fix: tighten tag-value validation; auto-capture repository from GITHUB_REPOSITORY#12

Merged
lukekim merged 2 commits into
trunkfrom
fix/tag-value-validation-and-auto-repo
May 2, 2026
Merged

fix: tighten tag-value validation; auto-capture repository from GITHUB_REPOSITORY#12
lukekim merged 2 commits into
trunkfrom
fix/tag-value-validation-and-auto-repo

Conversation

@lukekim
Copy link
Copy Markdown
Contributor

@lukekim lukekim commented May 2, 2026

Summary

Two related fixes prompted by a real failed run:

```
PUT /v1/apps/4573 failed: 400 Bad Request — Tag validation failed:
Invalid value for key "repository": Must contain only alphanumeric
characters and the symbols _@-
```

The user's workflow set `tags: "repository: lukekim/home"` and the API rejected the `/` mid-deploy.

1. Tighten tag-value validation locally

The Spice Cloud API allows only alphanumerics and `_@-` in tag values, but the action only enforced length. Add a local value pattern check matching the API rule so the failure surfaces with a clear actionable error before any state-changing API call:

```
tags: value for "repository" must contain only letters, numbers, and "_@-" (got "lukekim/home").
```

2. Auto-capture `repository` from `GITHUB_REPOSITORY`

When `GITHUB_REPOSITORY` is set (always true on GitHub-hosted runners), the action now auto-adds a `repository` tag derived from that env var, rewriting `/` to `_` so the value passes API validation. So a workflow on `lukekim/home` will now have its app tagged with `repository: lukekim_home` for free, with no `tags:` input required. Setting `repository:` explicitly in the user's `tags` input still wins on conflict — no surprise overrides.

New helpers in `src/tags.ts`, all unit-tested:

  • `sanitizeTagValue(value)` — replaces any disallowed character with `_`, truncates to 256 chars.
  • `deriveDefaultTags(env)` — currently returns `{ repository: sanitized($GITHUB_REPOSITORY) }` when set.
  • `mergeWithDefaultTags(userTags, env)` — merges user tags on top of defaults (user wins).

3. Drive-by

Drop the post-update mutation of `app.tags` in `maybeUpdateAppMetadata`. Nothing downstream reads it, and the mutation made shared `App` objects in tests leak state across cases.

Test plan

  • `npm run all` — 83 tests pass (up from 73), lint/typecheck/build/dist freshness clean.
  • New tests cover: API value-pattern enforcement, `sanitizeTagValue`, `deriveDefaultTags` empty/set, user-overrides-default merging, and the end-to-end `runDeploy` paths.
  • CI green on this PR across Ubuntu/macOS/Windows.
  • After merge: re-run the `lukekim/home` workflow and confirm the `repository` tag is auto-set without a 400.

Out of scope

  • Auto-capturing other GitHub-context tags (e.g. `commit`, `actor`, `branch`). `commit_sha`/`branch` already flow into the deployment metadata as separate fields, so they don't need to be tags. Happy to add more if useful — call out which ones.

…B_REPOSITORY

Bug
- The Spice Cloud API rejects tag values that contain anything besides
  alphanumerics and `_@-`, but the action only validated value length.
  A user passing `tags: |\n  repository: owner/repo` would hit the API
  with a 400 Bad Request mid-deploy. Add a local value pattern check
  matching the API rule so the failure surfaces locally with a clear
  error before any state-changing call.

Auto-capture
- When `GITHUB_REPOSITORY` is set (always true on GitHub-hosted runners),
  the action now auto-adds a `repository` tag derived from that env var,
  rewriting `/` to `_` so the value passes API validation. User-supplied
  `repository:` in the `tags` input still wins on conflict.
- New `sanitizeTagValue`, `deriveDefaultTags`, and `mergeWithDefaultTags`
  helpers in `src/tags.ts`, all unit-tested.

Other
- Drop the post-update mutation of `app.tags` in `maybeUpdateAppMetadata`.
  Nothing downstream reads it, and the mutation made shared `App` objects
  in tests leak state across cases.
Copilot AI review requested due to automatic review settings May 2, 2026 23:26
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR tightens app-tag handling in the deploy action by validating tag values against the API’s allowed character set, auto-deriving a default repository tag from GITHUB_REPOSITORY, and removing a test-only state leak from app-tag mutation. It fits into the action’s deployment flow by moving tag failures earlier and enriching app metadata automatically.

Changes:

  • Add local tag-value validation plus helpers to sanitize and derive default tags from GitHub environment data.
  • Apply default-tag merging during app creation and app metadata updates in the deploy flow.
  • Expand unit tests and update user-facing docs/changelog for the new tag behavior.

Reviewed changes

Copilot reviewed 6 out of 8 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
src/tags.ts Adds tag-value validation and helper functions for sanitizing and deriving default tags.
src/deploy.ts Wires default-tag merging into app creation/update logic and removes post-update tag mutation.
__tests__/tags.test.ts Adds unit coverage for stricter validation and default-tag helper behavior.
__tests__/deploy.test.ts Adds deploy-flow tests for auto-captured and user-overridden repository tags.
README.md Documents stricter tag rules and automatic repository tagging.
CHANGELOG.md Records the new validation and default-tag behavior under Unreleased.
Comments suppressed due to low confidence (2)

src/deploy.ts:139

  • Because the synthesized default tags are spread after app.tags, an existing app.tags.repository will be overwritten even when the user did not supply any repository: tag. That turns the new default into a silent metadata rewrite for existing apps instead of only filling a missing value.
  const merged = { ...(app.tags ?? {}), ...tags };
  const update: UpdateAppBody = { tags: merged };

src/deploy.ts:114

  • The new default-tag behavior on the create-app path is not covered by the deploy tests. The added end-to-end tests only assert the updateApp path, so a regression where createApp stops forwarding the synthesized repository tag would currently slip through.
  const tags = mergeWithDefaultTags(parseTags(inputs.tagsRaw));
  core.info(`App "${inputs.appName}" not found; creating in region "${inputs.region}".`);
  return api.createApp({
    name: inputs.appName,
    region: inputs.region,

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread README.md
>
> **Tag value rule:** the Spice Cloud API allows only alphanumerics and `_@-`. The action validates this locally so you fail fast with a clear error instead of a `400 Bad Request` on the server.
>
> **Auto-captured tags:** when `GITHUB_REPOSITORY` is set (always true on GitHub-hosted runners), the action adds a `repository` tag derived from that env var, with `/` rewritten to `_` so the value passes API validation. Setting `repository:` explicitly in your `tags` overrides the auto-captured value.
Comment thread README.md
| `region` | conditional | — | Spice Cloud region (e.g. `us-east-1`, `us-west-2`). Required for new apps. |
| `visibility` | no | `private` | `public` or `private` — only used on app creation. |
| `tags` | no | — | YAML or JSON map of tag key/value pairs. Merged into existing app tags. |
| `tags` | no | — | YAML or JSON map of tag key/value pairs. Merged into existing app tags. Tag values may contain only alphanumerics and `_@-`. The action auto-adds a `repository` tag from `GITHUB_REPOSITORY` (sanitizing `/` → `_`) unless you override it. |
Comment thread src/tags.ts
Comment on lines +124 to +125
export function sanitizeTagValue(value: string): string {
return value.replace(/[^A-Za-z0-9_@-]/g, "_").slice(0, MAX_VALUE_LENGTH);
Comment thread src/deploy.ts
Comment on lines +135 to 137
const tags = mergeWithDefaultTags(parseTags(inputs.tagsRaw));
if (!tags) return;

@lukekim lukekim mentioned this pull request May 2, 2026
3 tasks
@lukekim lukekim merged commit 399858d into trunk May 2, 2026
4 checks passed
@lukekim lukekim deleted the fix/tag-value-validation-and-auto-repo branch May 2, 2026 23:36
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