Skip to content

Potential fix for code scanning alert no. 6: Workflow does not contain permissions - #3

Merged
btravers merged 1 commit into
mainfrom
alert-autofix-6
Dec 9, 2025
Merged

btravers merged 1 commit into
mainfrom
alert-autofix-6

Conversation

@btravers

@btravers btravers commented Dec 9, 2025

Copy link
Copy Markdown
Collaborator

Potential fix for https://github.com/btravers/temporal-contract/security/code-scanning/6

The best way to fix the problem is to explicitly add a permissions block that restricts the GITHUB_TOKEN to only those permissions necessary for the jobs. You should add this at the workflow root level (just after the name: and before on:) so it applies to all jobs. For this workflow, the minimal required permission is typically contents: read, as the jobs only check out code and run analysis—they do not push to the repository or open PRs. There is no indication from the job steps that more permissions are required.

No significant restructuring is needed: add

permissions:
  contents: read

immediately under the workflow name at the top of .github/workflows/ci.yml. No additional imports, method definitions, or variable definitions are needed.

Suggested fixes powered by Copilot Autofix. Review carefully before merging.

…n permissions

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
@btravers
btravers marked this pull request as ready for review December 9, 2025 13:13
Copilot AI review requested due to automatic review settings December 9, 2025 13:13
@btravers
btravers merged commit 05416d5 into main Dec 9, 2025
6 checks passed
@btravers
btravers deleted the alert-autofix-6 branch December 9, 2025 13:13

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 addresses a security code scanning alert by adding an explicit permissions block to restrict the GITHUB_TOKEN to the minimum necessary permissions. The workflow is configured to use contents: read, following the principle of least privilege, as the CI jobs only checkout code and run analysis tasks without requiring write access.

Key Changes:

  • Added explicit permissions block with contents: read at the workflow root level

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

btravers pushed a commit that referenced this pull request May 5, 2026
…l/query/update name constraints

Two TypeScript soundness bugs were collapsing the typed-client surface to
`unknown` for every workflow (audit findings #2 and #3):

1. `WorkflowDefinition` was unparameterized over its `input`/`output`
   schemas. Both fields were typed as `AnySchema` directly, so the
   inferred return of `defineWorkflow({ input, output })` widened the
   schema literal types away — every downstream `ClientInferInput<…>`
   resolved to `unknown` even though the contract was authored with full
   schema literals.

2. The empty-collection generics on `WorkflowDefinition` defaulted to
   `Record<string, ActivityDefinition>` (and analogous wide records for
   signals/queries/updates/searchAttributes). A constraint like
   `TSignalName extends keyof TContract["workflows"][T]["signals"] & string`
   collapsed to `string` for those defaults, so a typo in `signalName` —
   on a workflow declaring no signals at all — slipped past the type
   checker and was only caught by the runtime guard inside `signalWithStart`.

The fix:

- Parameterize `WorkflowDefinition<TInput, TOutput, TActivities, TSignals,
  TQueries, TUpdates, TSearchAttributes>` and thread the schema literal
  types through `defineWorkflow`. Empty-collection generics now default
  to `Record<string, never>` so `keyof` of the default resolves to `never`.
- Introduce `AnyWorkflowDefinition` as the widened constraint variant for
  use in `Record<string, …>` constraint positions (`ContractDefinition`'s
  `TWorkflows`, the bind-handler signatures, etc.) so real contracts —
  which legitimately contain non-empty activities/signals/queries/updates
  — still satisfy the constraint.
- Add `SignalNamesOf<W>` / `QueryNamesOf<W>` / `UpdateNamesOf<W>` helpers
  and use them everywhere the previous `keyof … & string` pattern lived
  (`client.ts:90, 424` and `worker/workflow.ts:387-440`).
- Add `& string` to every `TWorkflowName extends keyof TContract["workflows"]`
  constraint at the typed-client and typed-worker call sites, so the
  `as string` / `String(workflowName)` casts at the Temporal-API call sites
  go away. Error-message helpers that legitimately stringify the name keep
  their broader `string | number | symbol` parameter — those are downstream
  of the typed surface and don't affect inference.
- Keep the runtime defensive branch in `signalWithStart` for raw-call /
  union-typed-name corner cases (its TODO is the existing comment).
- New `expectTypeOf` tests in `packages/contract/src/types-inference.spec.ts`
  and `packages/client/src/types-inference.spec.ts` pin both fixes against
  regression: workflow input/output preservation, `SignalNamesOf` resolving
  to `never` on no-signal workflows, and the typed-client `args` /
  `signalArgs` shapes.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
btravers pushed a commit that referenced this pull request May 5, 2026
…l/query/update name constraints

Two TypeScript soundness bugs were collapsing the typed-client surface to
`unknown` for every workflow (audit findings #2 and #3):

1. `WorkflowDefinition` was unparameterized over its `input`/`output`
   schemas. Both fields were typed as `AnySchema` directly, so the
   inferred return of `defineWorkflow({ input, output })` widened the
   schema literal types away — every downstream `ClientInferInput<…>`
   resolved to `unknown` even though the contract was authored with full
   schema literals.

2. The empty-collection generics on `WorkflowDefinition` defaulted to
   `Record<string, ActivityDefinition>` (and analogous wide records for
   signals/queries/updates/searchAttributes). A constraint like
   `TSignalName extends keyof TContract["workflows"][T]["signals"] & string`
   collapsed to `string` for those defaults, so a typo in `signalName` —
   on a workflow declaring no signals at all — slipped past the type
   checker and was only caught by the runtime guard inside `signalWithStart`.

The fix:

- Parameterize `WorkflowDefinition<TInput, TOutput, TActivities, TSignals,
  TQueries, TUpdates, TSearchAttributes>` and thread the schema literal
  types through `defineWorkflow`. Empty-collection generics now default
  to `Record<string, never>` so `keyof` of the default resolves to `never`.
- Introduce `AnyWorkflowDefinition` as the widened constraint variant for
  use in `Record<string, …>` constraint positions (`ContractDefinition`'s
  `TWorkflows`, the bind-handler signatures, etc.) so real contracts —
  which legitimately contain non-empty activities/signals/queries/updates
  — still satisfy the constraint.
- Add `SignalNamesOf<W>` / `QueryNamesOf<W>` / `UpdateNamesOf<W>` helpers
  and use them everywhere the previous `keyof … & string` pattern lived
  (`client.ts:90, 424` and `worker/workflow.ts:387-440`).
- Add `& string` to every `TWorkflowName extends keyof TContract["workflows"]`
  constraint at the typed-client and typed-worker call sites, so the
  `as string` / `String(workflowName)` casts at the Temporal-API call sites
  go away. Error-message helpers that legitimately stringify the name keep
  their broader `string | number | symbol` parameter — those are downstream
  of the typed surface and don't affect inference.
- Keep the runtime defensive branch in `signalWithStart` for raw-call /
  union-typed-name corner cases (its TODO is the existing comment).
- New `expectTypeOf` tests in `packages/contract/src/types-inference.spec.ts`
  and `packages/client/src/types-inference.spec.ts` pin both fixes against
  regression: workflow input/output preservation, `SignalNamesOf` resolving
  to `never` on no-signal workflows, and the typed-client `args` /
  `signalArgs` shapes.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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