Skip to content

Commit 817dda8

Browse files
ZiuChenZiuChen
committed
feat: address code review issues in action version hover/quick fix
- Extract shared module (actionVersionUtils.ts) to eliminate duplicated parsing, caching, and API logic between hover and code action providers - Use non-interactive getSession() to prevent auth prompts during passive hover/code-action triggers - Skip dynamic refs containing `${{` expressions to avoid corrupting YAML with incorrect replacements - Suppress Quick Fix for SHA-pinned action refs to preserve supply-chain security posture - Remove unnecessary `md.isTrusted = true` on hover MarkdownString - Respect CancellationToken in both providers to abort early on stale requests - Fix misleading "latest semver" comment to match actual date-sorted tag selection behavior
1 parent de38cfb commit 817dda8

File tree

3 files changed

+134
-167
lines changed

3 files changed

+134
-167
lines changed

‎src/hover/actionVersionCodeActionProvider.ts‎

Lines changed: 15 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -1,84 +1,6 @@
11
import * as vscode from "vscode";
22

3-
import {TTLCache} from "@actions/languageserver/utils/cache";
4-
5-
import {getSession} from "../auth/auth";
6-
import {getClient} from "../api/api";
7-
8-
const USES_PATTERN = /uses:\s*(['"]?)([^@\s'"]+)@([^\s'"#]+)/;
9-
const CACHE_TTL_MS = 5 * 60 * 1000;
10-
11-
const cache = new TTLCache(CACHE_TTL_MS);
12-
13-
interface ActionVersionInfo {
14-
latest: string;
15-
latestMajor?: string;
16-
}
17-
18-
function parseUsesReference(
19-
line: string
20-
): {owner: string; name: string; actionPath: string; currentRef: string; refStart: number; refEnd: number} | undefined {
21-
const match = USES_PATTERN.exec(line);
22-
if (!match) {
23-
return undefined;
24-
}
25-
26-
const actionPath = match[2];
27-
const currentRef = match[3];
28-
29-
const [owner, name] = actionPath.split("/");
30-
if (!owner || !name) {
31-
return undefined;
32-
}
33-
34-
// Find the position of the @ref part
35-
const fullMatchStart = match.index + match[0].indexOf(match[2]);
36-
const refStart = fullMatchStart + actionPath.length + 1; // +1 for @
37-
const refEnd = refStart + currentRef.length;
38-
39-
return {owner, name, actionPath, currentRef, refStart, refEnd};
40-
}
41-
42-
function extractMajorTag(tag: string): string | undefined {
43-
const match = /^(v?\d+)[\.\d]*/.exec(tag);
44-
return match ? match[1] : undefined;
45-
}
46-
47-
async function fetchLatestVersion(owner: string, name: string): Promise<ActionVersionInfo | undefined> {
48-
const session = await getSession(true);
49-
if (!session) {
50-
return undefined;
51-
}
52-
53-
const cacheKey = `action-latest-version:${owner}/${name}`;
54-
return cache.get<ActionVersionInfo | undefined>(cacheKey, undefined, async () => {
55-
const client = getClient(session.accessToken);
56-
57-
try {
58-
const {data} = await client.repos.getLatestRelease({owner, repo: name});
59-
if (data.tag_name) {
60-
const major = extractMajorTag(data.tag_name);
61-
return {latest: data.tag_name, latestMajor: major};
62-
}
63-
} catch {
64-
// No release found
65-
}
66-
67-
try {
68-
const {data} = await client.repos.listTags({owner, repo: name, per_page: 10});
69-
if (data.length > 0) {
70-
const semverTag = data.find(t => /^v?\d+\.\d+/.test(t.name));
71-
const tag = semverTag || data[0];
72-
const major = extractMajorTag(tag.name);
73-
return {latest: tag.name, latestMajor: major};
74-
}
75-
} catch {
76-
// Ignore
77-
}
78-
79-
return undefined;
80-
});
81-
}
3+
import {parseUsesReference, fetchLatestVersion, isShaRef} from "./actionVersionUtils";
824

835
export class ActionVersionCodeActionProvider implements vscode.CodeActionProvider {
846
static readonly providedCodeActionKinds = [vscode.CodeActionKind.QuickFix];
@@ -87,17 +9,30 @@ export class ActionVersionCodeActionProvider implements vscode.CodeActionProvide
879
document: vscode.TextDocument,
8810
range: vscode.Range | vscode.Selection,
8911
_context: vscode.CodeActionContext,
90-
_token: vscode.CancellationToken
12+
token: vscode.CancellationToken
9113
): Promise<vscode.CodeAction[] | undefined> {
9214
const actions: vscode.CodeAction[] = [];
9315

9416
for (let lineNum = range.start.line; lineNum <= range.end.line; lineNum++) {
17+
if (token.isCancellationRequested) {
18+
return actions.length > 0 ? actions : undefined;
19+
}
20+
9521
const line = document.lineAt(lineNum).text;
9622
const ref = parseUsesReference(line);
9723
if (!ref) {
9824
continue;
9925
}
10026

27+
// Don't offer to replace SHA-pinned refs — it would change the security posture
28+
if (isShaRef(ref.currentRef)) {
29+
continue;
30+
}
31+
32+
if (token.isCancellationRequested) {
33+
return actions.length > 0 ? actions : undefined;
34+
}
35+
10136
const versionInfo = await fetchLatestVersion(ref.owner, ref.name);
10237
if (!versionInfo) {
10338
continue;

‎src/hover/actionVersionHoverProvider.ts‎

Lines changed: 7 additions & 87 deletions
Original file line numberDiff line numberDiff line change
@@ -1,95 +1,12 @@
11
import * as vscode from "vscode";
22

3-
import {TTLCache} from "@actions/languageserver/utils/cache";
4-
5-
import {getSession} from "../auth/auth";
6-
import {getClient} from "../api/api";
7-
8-
const USES_PATTERN = /uses:\s*(['"]?)([^@\s'"]+)@([^\s'"#]+)/;
9-
const CACHE_TTL_MS = 5 * 60 * 1000; // 5 minutes
10-
11-
const cache = new TTLCache(CACHE_TTL_MS);
12-
13-
interface ActionVersionInfo {
14-
latest: string;
15-
/** The latest major version tag, e.g. "v4" */
16-
latestMajor?: string;
17-
}
18-
19-
/**
20-
* Parses the `uses:` value from a workflow line and returns owner, name, and current ref.
21-
*/
22-
function parseUsesReference(
23-
line: string
24-
): {owner: string; name: string; currentRef: string; valueStart: number; valueEnd: number} | undefined {
25-
const match = USES_PATTERN.exec(line);
26-
if (!match) {
27-
return undefined;
28-
}
29-
30-
const actionPath = match[2]; // e.g. "actions/checkout" or "actions/cache/restore"
31-
const currentRef = match[3];
32-
33-
const [owner, name] = actionPath.split("/");
34-
if (!owner || !name) {
35-
return undefined;
36-
}
37-
38-
const valueStart = match.index + match[0].indexOf(match[2]);
39-
const valueEnd = valueStart + actionPath.length + 1 + currentRef.length; // +1 for @
40-
41-
return {owner, name, currentRef, valueStart, valueEnd};
42-
}
43-
44-
async function fetchLatestVersion(owner: string, name: string): Promise<ActionVersionInfo | undefined> {
45-
const session = await getSession(true);
46-
if (!session) {
47-
return undefined;
48-
}
49-
50-
const cacheKey = `action-latest-version:${owner}/${name}`;
51-
return cache.get<ActionVersionInfo | undefined>(cacheKey, undefined, async () => {
52-
const client = getClient(session.accessToken);
53-
54-
// Try latest release first
55-
try {
56-
const {data} = await client.repos.getLatestRelease({owner, repo: name});
57-
if (data.tag_name) {
58-
const major = extractMajorTag(data.tag_name);
59-
return {latest: data.tag_name, latestMajor: major};
60-
}
61-
} catch {
62-
// No release found, fallback to tags
63-
}
64-
65-
// Fallback: list tags and find latest semver
66-
try {
67-
const {data} = await client.repos.listTags({owner, repo: name, per_page: 10});
68-
if (data.length > 0) {
69-
// Find the latest semver-like tag
70-
const semverTag = data.find(t => /^v?\d+\.\d+/.test(t.name));
71-
const tag = semverTag || data[0];
72-
const major = extractMajorTag(tag.name);
73-
return {latest: tag.name, latestMajor: major};
74-
}
75-
} catch {
76-
// Ignore
77-
}
78-
79-
return undefined;
80-
});
81-
}
82-
83-
function extractMajorTag(tag: string): string | undefined {
84-
const match = /^(v?\d+)[\.\d]*/.exec(tag);
85-
return match ? match[1] : undefined;
86-
}
3+
import {parseUsesReference, fetchLatestVersion} from "./actionVersionUtils";
874

885
export class ActionVersionHoverProvider implements vscode.HoverProvider {
896
async provideHover(
907
document: vscode.TextDocument,
918
position: vscode.Position,
92-
_token: vscode.CancellationToken
9+
token: vscode.CancellationToken
9310
): Promise<vscode.Hover | undefined> {
9411
const line = document.lineAt(position).text;
9512
const ref = parseUsesReference(line);
@@ -102,13 +19,16 @@ export class ActionVersionHoverProvider implements vscode.HoverProvider {
10219
return undefined;
10320
}
10421

22+
if (token.isCancellationRequested) {
23+
return undefined;
24+
}
25+
10526
const versionInfo = await fetchLatestVersion(ref.owner, ref.name);
106-
if (!versionInfo) {
27+
if (!versionInfo || token.isCancellationRequested) {
10728
return undefined;
10829
}
10930

11031
const md = new vscode.MarkdownString();
111-
md.isTrusted = true;
11232

11333
const isCurrentLatest = ref.currentRef === versionInfo.latest || ref.currentRef === versionInfo.latestMajor;
11434

‎src/hover/actionVersionUtils.ts‎

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
import {TTLCache} from "@actions/languageserver/utils/cache";
2+
3+
import {getSession} from "../auth/auth";
4+
import {getClient} from "../api/api";
5+
6+
const USES_PATTERN = /uses:\s*(['"]?)([^@\s'"]+)@([^\s'"#]+)/;
7+
const CACHE_TTL_MS = 5 * 60 * 1000; // 5 minutes
8+
9+
const cache = new TTLCache(CACHE_TTL_MS);
10+
11+
export interface ActionVersionInfo {
12+
latest: string;
13+
/** The latest major version tag, e.g. "v4" */
14+
latestMajor?: string;
15+
}
16+
17+
export interface UsesReference {
18+
owner: string;
19+
name: string;
20+
actionPath: string;
21+
currentRef: string;
22+
/** Start of the full "owner/repo@ref" value */
23+
valueStart: number;
24+
/** End of the full "owner/repo@ref" value */
25+
valueEnd: number;
26+
/** Start of just the ref part after @ */
27+
refStart: number;
28+
/** End of just the ref part after @ */
29+
refEnd: number;
30+
}
31+
32+
/**
33+
* Parses the `uses:` value from a workflow line and returns owner, name, and current ref.
34+
* Returns undefined for dynamic refs containing expression syntax like `${{`.
35+
*/
36+
export function parseUsesReference(line: string): UsesReference | undefined {
37+
const match = USES_PATTERN.exec(line);
38+
if (!match) {
39+
return undefined;
40+
}
41+
42+
const actionPath = match[2]; // e.g. "actions/checkout" or "actions/cache/restore"
43+
const currentRef = match[3];
44+
45+
// Skip dynamic refs (e.g. ${{ github.ref }})
46+
if (currentRef.includes("${{")) {
47+
return undefined;
48+
}
49+
50+
const [owner, name] = actionPath.split("/");
51+
if (!owner || !name) {
52+
return undefined;
53+
}
54+
55+
const fullMatchStart = match.index + match[0].indexOf(match[2]);
56+
const valueStart = fullMatchStart;
57+
const refStart = fullMatchStart + actionPath.length + 1; // +1 for @
58+
const refEnd = refStart + currentRef.length;
59+
const valueEnd = refEnd;
60+
61+
return {owner, name, actionPath, currentRef, valueStart, valueEnd, refStart, refEnd};
62+
}
63+
64+
export function extractMajorTag(tag: string): string | undefined {
65+
const match = /^(v?\d+)[\.\d]*/.exec(tag);
66+
return match ? match[1] : undefined;
67+
}
68+
69+
/**
70+
* Returns true if the ref looks like a commit SHA (40-char hex string).
71+
*/
72+
export function isShaRef(ref: string): boolean {
73+
return /^[0-9a-f]{40}$/i.test(ref);
74+
}
75+
76+
export async function fetchLatestVersion(owner: string, name: string): Promise<ActionVersionInfo | undefined> {
77+
const session = await getSession();
78+
if (!session) {
79+
return undefined;
80+
}
81+
82+
const cacheKey = `action-latest-version:${owner}/${name}`;
83+
return cache.get<ActionVersionInfo | undefined>(cacheKey, undefined, async () => {
84+
const client = getClient(session.accessToken);
85+
86+
// Try latest release first
87+
try {
88+
const {data} = await client.repos.getLatestRelease({owner, repo: name});
89+
if (data.tag_name) {
90+
const major = extractMajorTag(data.tag_name);
91+
return {latest: data.tag_name, latestMajor: major};
92+
}
93+
} catch {
94+
// No release found, fallback to tags
95+
}
96+
97+
// Fallback: list tags and pick the first semver-like tag (tags are returned in creation-date order)
98+
try {
99+
const {data} = await client.repos.listTags({owner, repo: name, per_page: 10});
100+
if (data.length > 0) {
101+
const semverTag = data.find(t => /^v?\d+\.\d+/.test(t.name));
102+
const tag = semverTag || data[0];
103+
const major = extractMajorTag(tag.name);
104+
return {latest: tag.name, latestMajor: major};
105+
}
106+
} catch {
107+
// Ignore
108+
}
109+
110+
return undefined;
111+
});
112+
}

0 commit comments

Comments
 (0)