Medic by patrickhuie19 · Pull Request #1447 · smartcontractkit/.github

patrickhuie19 · 2026-03-06T07:17:59Z

Bringing https://github.com/smartcontractkit/medic-test/pulls to this repo. Next step is to use the reusable workflows in core.

github-actions · 2026-03-06T07:18:18Z

👋 patrickhuie19, thanks for creating this pull request!

To help reviewers, please consider creating future PRs as drafts first. This allows you to self-review and make any final changes before notifying the team.

Once you're ready, you can mark it as "Ready for review" to request feedback. Thanks!

github-advanced-security

CodeQL found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.

erikburt

I’ve left more detailed comments inline, but at this size it’s difficult to fully vet in a single pass.

Some higher-level feedback:

I think this probably belongs in its own repo. It’s not just a single action, it’s a collection of related actions and workflows, and because of that, seems like a suite/product in itself. Having it's own repository would make ownership and versioning a lot clearer. Can expand on this further if needed.
- I’m happy to help set up a new repo.
- If it must live in this repository, then all the code should live in the top-level apps/medic directory probably, rather than .github/actions/medic.
You're most of the way there, but each major medic feature should likely be its own independently versioned action, with shared functionality pulled into a common library.
- For example comment-parser, doesn't have an action.yml but is does have it's own dist. lib is not an actual lib, it's a shared folder. Although, probably not big enough to warrant it being a separate library yet, still helps with organization imo.
This kind of goes with the above, but right now, the code is tough to reason about and maintain. I think it would benefit from more structural cleanup. Clearer boundaries between individual functionality, tests utilities, common functions, etc.
Documentation would be helpful as well

erikburt · 2026-03-06T20:44:30Z

.github/workflows/medic-merge-conflict.yml

+      - name: Checkout medic
+        uses: actions/checkout@v4
+        with:
+          repository: smartcontractkit/medic-test
+          path: _medic
+
+      - name: Setup Node.js
+        uses: actions/setup-node@v4
+        with:
+          node-version: '20'
+
+      - name: Parse comment
+        id: parse
+        run: node _medic/.github/actions/medic/dist/comment-parser/index.js
+        env:
+          INPUT_GITHUB-TOKEN: ${{ secrets.GITHUB_TOKEN }}
+          INPUT_COMMENT-BODY: ${{ github.event.comment.body }}
+          INPUT_ISSUE-NUMBER: ${{ github.event.issue.number }}
+          INPUT_COMMENTER-LOGIN: ${{ github.event.comment.user.login }}
+


Seems like you properly split up the other functions/actions into their own action.ymls, but this one should be as well.

erikburt · 2026-03-06T20:45:28Z

.github/workflows/medic-merge-conflict.yml

+        id: check
+        run: |
+          echo "Found /medic generate conflict command"
+          echo "should_generate=true" >> $GITHUB_OUTPUT


I recommend using ... | tee -a "${GITHUB_OUTPUT}" everywhere so you can see what values are actually being set.

erikburt · 2026-03-06T20:46:15Z

.github/workflows/medic-workflow-retry-auto.yml

+        uses: actions/checkout@v4
+
+      - name: Authenticate to GCP
+        uses: google-github-actions/auth@v2


Pin to a sha.

erikburt · 2026-03-06T20:46:53Z

.github/workflows/medic-workflow-retry-auto.yml

+          export_environment_variables: true
+
+      - name: Auto retry failed workflow
+        uses: smartcontractkit/medic-test/.github/actions/medic/workflow-retry-auto@main


ideally this is versioned and tagged, not pointing to main. could add reusable workflows in a later PR.

erikburt · 2026-03-06T20:47:46Z

.github/workflows/medic-merge-conflict.yml

+      - name: Checkout medic
+        uses: actions/checkout@v4
+        with:
+          repository: smartcontractkit/medic-test
+          path: _medic


Using old repo all over the place. And based on below comment, this shouldn't be necesary.

erikburt · 2026-03-06T20:56:11Z

.github/actions/medic/src/workflow-retry/claude-client.ts

+    const AnthropicVertex = mod.default;
+    client = new AnthropicVertex({
+      projectId,
+      region: process.env.CLOUD_ML_REGION || 'us-east5',


Ideally this is an input, mixing random env vars and inputs can be confusing (same with ANTHROPIC_VERTEX_PROJECT_ID) above.

erikburt · 2026-03-06T20:58:37Z

.github/actions/medic/src/workflow-retry/log-service.ts

+
+  try {
+    const response = await octokit.request(
+      'GET /repos/{owner}/{repo}/actions/jobs/{job_id}/logs',


Why not use octokit.rest.actions.downloadJobLogsForWorkflowRun. It also accepts the request object afaict.

erikburt · 2026-03-06T21:09:07Z

.github/actions/medic/src/fixer-main.ts

+  const execPromise = exec.exec(
+    'claude',
+    ['--dangerously-skip-permissions', '-p', prompt, '--output-format', 'stream-json', '--verbose'],
+    {


This technically leaks credentials in 2 ways.

This exec call inherits env vars, so INPUT_GITHUB-TOKEN is leaked to Claude. Also when using actions/checkout in a workflow, credentials are also saved to the repo unless the action is called with persist-credentials: false.

This is a blocker. The LLM should only have access to what it needs. Since Claude code can execute arbitrary commands, the risk is too high. Also note that the prompt injection can be dynamically fetched too.

Claude should not have internet access.

I think Claude should run in a separate step with no secrets in the environment variable, and with persist-credentials: false

erikburt · 2026-03-06T21:12:50Z

.github/actions/medic/src/conflict-generator.ts

@@ -0,0 +1,292 @@
+/**


I think files like this should probably live in __tests__ or in a e2e folder of some sort.

erikburt · 2026-03-06T21:14:20Z

.github/actions/medic/src/export-prompt.ts

+import { PROMPT_TEMPLATE } from './lib/prompts.js';
+
+process.stdout.write(PROMPT_TEMPLATE);


Is this needed?

timweri

After looking at this closer, we need more hardenings on the agent:

Remove access to secrets

We shouldn't let the agent get access to the Github token or any secrets. Use deterministic code to prepare agent inputs and handle agent outputs. The agent step should have no access to secrets

Sandbox and lock down permissions

We can harden the agent more:

Run in sandbox mode
Deny read to sensitive folders
Limit network access. Maybe with allowedDomains.
Run in dontAsk permission mode instead of god mode and allow only needed tools (Read, Edit)

I think the agent should not be able to call any bash command. It should only read, edit and provide text output. Everything else should be scripted.

timweri · 2026-03-09T03:20:56Z

.github/workflows/medic-workflow-retry.yml

+    steps:
+      - name: React with eyes
+        run: |
+          gh api repos/${{ github.repository }}/issues/comments/${{ github.event.comment.id }}/reactions \


The CodeQL flag is correct here. Even when some values are not easily manipulated for injection, we should put these workflow variables as environment variables as good practice.

Example:

env: REPO: ${{ github.repository }} COMMENT_ID: ${{ github.event.comment.id }} run: gh api repos/"$REPO"/issues/comments/"$COMMENT_ID"/reactions ...

timweri · 2026-03-09T03:21:51Z

.github/workflows/medic-workflow-retry.yml

+      - name: Checkout medic
+        uses: actions/checkout@v4
+        with:
+          repository: smartcontractkit/medic-test


Do we need this?

timweri · 2026-03-09T03:26:36Z

.github/workflows/medic-merge-conflict.yml

@@ -0,0 +1,334 @@
+name: Medic - Merge Conflict Resolution


I think we should keep this PR light with just CI retry.

timweri · 2026-03-09T03:31:49Z

.github/actions/medic/package.json

@@ -0,0 +1,58 @@
+{
+    "name": "medic-action",


We should prefix this to be @chainlink/medic-action

timweri · 2026-03-09T04:17:24Z

.github/actions/medic/src/fixer-main.ts

+  const execPromise = exec.exec(
+    'claude',
+    ['--dangerously-skip-permissions', '-p', prompt, '--output-format', 'stream-json', '--verbose'],
+    {


This is a blocker. The LLM should only have access to what it needs. Since Claude code can execute arbitrary commands, the risk is too high. Also note that the prompt injection can be dynamically fetched too.

Claude should not have internet access.

timweri · 2026-03-09T04:28:04Z

.github/actions/medic/src/fixer-main.ts

+  const execPromise = exec.exec(
+    'claude',
+    ['--dangerously-skip-permissions', '-p', prompt, '--output-format', 'stream-json', '--verbose'],
+    {


I think Claude should run in a separate step with no secrets in the environment variable, and with persist-credentials: false

timweri · 2026-03-09T04:33:07Z

.github/workflows/medic-merge-conflict.yml

+        id: direct-pr
+        if: inputs.pr-number != ''
+        run: |
+          PR_DATA=$(gh pr view ${{ inputs.pr-number }} --json number,headRefName,baseRefName,author --jq '{number: .number, headRefName: .headRefName, baseRefName: .baseRefName, author: .author.login, currentAttempts: 0}')


Same for ${{ inputs.pr-number }}. This should put it as an env and then use "$ENV".

timweri · 2026-03-09T04:49:17Z

.github/actions/medic/src/fixer-main.ts

+          attempt,
+          tokens: claudeResult.tokens,
+          author: pr.user?.login || 'unknown',
+          details: errorMessage,


Error outputs from Claude should be treated caution. For example, it might include phishing markdown injection. I think if Claude fails, the error should be restricted. Overall, Claude open output in comments should not be trusted.

adding medic workflows

1694098

patrickhuie19 requested review from erikburt and timweri March 6, 2026 07:17

patrickhuie19 requested a review from a team as a code owner March 6, 2026 07:18

github-advanced-security bot found potential problems Mar 6, 2026

View reviewed changes

erikburt requested changes Mar 6, 2026

View reviewed changes

timweri requested changes Mar 9, 2026

View reviewed changes

		import { PROMPT_TEMPLATE } from './lib/prompts.js';

		process.stdout.write(PROMPT_TEMPLATE);

Conversation

patrickhuie19 commented Mar 6, 2026

Uh oh!

github-actions bot commented Mar 6, 2026

Uh oh!

github-advanced-security bot left a comment

Choose a reason for hiding this comment

Uh oh!

erikburt left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

timweri left a comment

Choose a reason for hiding this comment

Remove access to secrets

Sandbox and lock down permissions

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants