Merge pull request #54 from github/joshmgross/validate-action-inputs

Validate action inputs

actions-languageserver/src/connection.ts

@@ -1,6 +1,7 @@
-import { hover } from "@github/actions-languageservice/hover";
+import {hover} from "@github/actions-languageservice/hover";
-import { registerLogger, setLogLevel } from "@github/actions-languageservice/log";
+import {registerLogger, setLogLevel} from "@github/actions-languageservice/log";
-import { validate } from "@github/actions-languageservice/validate";
+import {validate, ValidationConfig} from "@github/actions-languageservice/validate";
+import {Octokit} from "@octokit/rest";
 import {
   CompletionItem,
   Connection,
@@ -12,12 +13,13 @@ import {
   TextDocuments,
   TextDocumentSyncKind
 } from "vscode-languageserver";
-import { TextDocument } from "vscode-languageserver-textdocument";
+import {TextDocument} from "vscode-languageserver-textdocument";
-import { contextProviders } from "./context-providers";
+import {contextProviders} from "./context-providers";
-import { InitializationOptions, RepositoryContext } from "./initializationOptions";
+import {InitializationOptions, RepositoryContext} from "./initializationOptions";
-import { onCompletion } from "./on-completion";
+import {onCompletion} from "./on-completion";
-import { TTLCache } from "./utils/cache";
+import {TTLCache} from "./utils/cache";
-import { valueProviders } from "./value-providers";
+import {valueProviders} from "./value-providers";
+import {getActionInputs} from "./value-providers/action-inputs";
 
 export function initConnection(connection: Connection) {
   const documents: TextDocuments<TextDocument> = new TextDocuments(TextDocument);
@@ -82,11 +84,21 @@ export function initConnection(connection: Connection) {
 
   async function validateTextDocument(textDocument: TextDocument): Promise<void> {
     const repoContext = repos.find(repo => textDocument.uri.startsWith(repo.workspaceUri));
-    const result = await validate(
+
-      textDocument,
+    const config: ValidationConfig = {
-      valueProviders(sessionToken, repoContext, cache),
+      valueProviderConfig: valueProviders(sessionToken, repoContext, cache),
-      contextProviders(sessionToken, repoContext, cache)
+      contextProviderConfig: contextProviders(sessionToken, repoContext, cache),
-    );
+      getActionInputs: async action => {
+        if (sessionToken) {
+          const octokit = new Octokit({
+            auth: sessionToken
+          });
+          return await getActionInputs(octokit, cache, action);
+        }
+        return undefined;
+      }
+    };
+    const result = await validate(textDocument, config);
 
     connection.sendDiagnostics({uri: textDocument.uri, diagnostics: result});
   }

actions-languageserver/src/value-providers.ts

@@ -4,7 +4,7 @@ import {ValueProviderKind} from "@github/actions-languageservice/value-providers
 import {Octokit} from "@octokit/rest";
 import {RepositoryContext} from "./initializationOptions";
 import {TTLCache} from "./utils/cache";
-import {getActionInputs} from "./value-providers/action-inputs";
+import {getActionInputValues} from "./value-providers/action-inputs";
 import {getEnvironments} from "./value-providers/job-environment";
 import {getRunnerLabels} from "./value-providers/runs-on";
 
@@ -36,7 +36,7 @@ export function valueProviders(
     },
     "step-with": {
       kind: ValueProviderKind.AllowedValues,
-      get: (context: WorkflowContext) => getActionInputs(octokit, cache, context)
+      get: (context: WorkflowContext) => getActionInputValues(octokit, cache, context)
     }
   };
 }

actions-languageserver/src/value-providers/action-inputs.ts

@@ -1,12 +1,33 @@
+import {
+  actionIdentifier,
+  ActionInputs,
+  ActionReference,
+  parseActionReference
+} from "@github/actions-languageservice/action";
 import {WorkflowContext} from "@github/actions-languageservice/context/workflow-context";
 import {Value} from "@github/actions-languageservice/value-providers/config";
 import {isActionStep} from "@github/actions-workflow-parser/model/type-guards";
 import {Octokit, RestEndpointMethodTypes} from "@octokit/rest";
 import {parse} from "yaml";
-import {actionIdentifier, ActionReference, parseActionReference} from "../utils/action-reference";
 import {TTLCache} from "../utils/cache";
 
-export async function getActionInputs(client: Octokit, cache: TTLCache, context: WorkflowContext): Promise<Value[]> {
+export async function getActionInputs(
+  client: Octokit,
+  cache: TTLCache,
+  action: ActionReference
+): Promise<ActionInputs | undefined> {
+  const inputs = await cache.get(`${actionIdentifier(action)}/action-inputs`, undefined, () =>
+    fetchActionInputs(client, action)
+  );
+
+  return inputs;
+}
+
+export async function getActionInputValues(
+  client: Octokit,
+  cache: TTLCache,
+  context: WorkflowContext
+): Promise<Value[]> {
   if (!context.step || !isActionStep(context.step)) {
     return [];
   }
@@ -15,18 +36,23 @@ export async function getActionInputs(client: Octokit, cache: TTLCache, context:
   if (!action) {
     return [];
   }
+  const inputs = await getActionInputs(client, cache, action);
+  if (!inputs) {
+    return [];
+  }
 
-  const inputs = await cache.get(`${actionIdentifier(action)}/action-inputs`, undefined, () =>
+  return Object.entries(inputs).map(([inputName, input]) => {
-    fetchActionInputs(client, action)
+    return {
-  );
+      label: inputName,
-
+      description: input.description
-  return inputs;
+    };
+  });
 }
 
-async function fetchActionInputs(client: Octokit, action: ActionReference): Promise<Value[]> {
+async function fetchActionInputs(client: Octokit, action: ActionReference): Promise<ActionInputs | undefined> {
   const metadata = await getActionMetadata(client, action);
   if (!metadata) {
-    return [];
+    return undefined;
   }
 
   return parseActionMetadata(metadata);
@@ -71,7 +97,7 @@ async function getActionMetadata(client: Octokit, action: ActionReference): Prom
 }
 
 type ActionMetadata = {
-  inputs?: Record<string, ActionInput>;
+  inputs?: ActionInputs;
 };
 
 // https://docs.github.com/en/actions/creating-actions/metadata-syntax-for-github-actions#inputs
@@ -83,20 +109,7 @@ type ActionInput = {
 };
 
 // https://docs.github.com/en/actions/creating-actions/metadata-syntax-for-github-actions
-async function parseActionMetadata(content: string): Promise<Value[]> {
+async function parseActionMetadata(content: string): Promise<ActionInputs> {
-  const inputs = new Array<Value>();
-
   const metadata: ActionMetadata = parse(content);
-  if (metadata.inputs === undefined) {
+  return metadata.inputs ?? {};
-    return inputs;
-  }
-
-  for (const [name, input] of Object.entries(metadata.inputs)) {
-    inputs.push({
-      label: name,
-      description: input.description
-    });
-  }
-
-  return inputs;
 }

actions-languageservice/src/action.test.ts

@@ -1,4 +1,4 @@
-import {actionIdentifier, parseActionReference as parse} from "./action-reference";
+import {actionIdentifier, parseActionReference as parse} from "./action";
 
 describe("parseActionReference", () => {
   it("basic action", () => {

actions-languageservice/src/action.ts

@@ -1,3 +1,13 @@
+// https://docs.github.com/en/actions/creating-actions/metadata-syntax-for-github-actions#inputs
+export type ActionInput = {
+  description: string;
+  required?: boolean;
+  default?: string;
+  deprecationMessage?: string;
+};
+
+export type ActionInputs = Record<string, ActionInput>;
+
 export type ActionReference = {
   owner: string;
   name: string;

actions-languageservice/src/validate-action.ts

@@ -0,0 +1,82 @@
+import {isMapping} from "@github/actions-workflow-parser";
+import {isActionStep} from "@github/actions-workflow-parser/model/type-guards";
+import {Step} from "@github/actions-workflow-parser/model/workflow-template";
+import {ScalarToken} from "@github/actions-workflow-parser/templates/tokens/scalar-token";
+import {TemplateToken} from "@github/actions-workflow-parser/templates/tokens/template-token";
+import {Diagnostic, DiagnosticSeverity} from "vscode-languageserver-types";
+import {parseActionReference} from "./action";
+import {mapRange} from "./utils/range";
+import {ValidationConfig} from "./validate";
+
+export async function validateAction(
+  diagnostics: Diagnostic[],
+  stepToken: TemplateToken,
+  step: Step | undefined,
+  config: ValidationConfig | undefined
+): Promise<void> {
+  if (!isMapping(stepToken) || !step || !isActionStep(step) || !config?.getActionInputs) {
+    return;
+  }
+
+  const action = parseActionReference(step.uses.value);
+  if (!action) {
+    return;
+  }
+
+  const actionInputs = await config.getActionInputs(action);
+  if (actionInputs === undefined) {
+    return;
+  }
+
+  let withKey: ScalarToken | undefined;
+  let withToken: TemplateToken | undefined;
+  for (const {key, value} of stepToken) {
+    if (key.toString() === "with") {
+      withKey = key;
+      withToken = value;
+      break;
+    }
+  }
+
+  const stepInputs = new Map<string, ScalarToken>();
+  if (withToken && isMapping(withToken)) {
+    for (const {key} of withToken) {
+      stepInputs.set(key.toString(), key);
+    }
+  }
+
+  for (const [input, inputToken] of stepInputs) {
+    if (!actionInputs[input]) {
+      diagnostics.push({
+        severity: DiagnosticSeverity.Error,
+        range: mapRange(inputToken.range),
+        message: `Invalid action input '${input}'`
+      });
+    }
+
+    const deprecationMessage = actionInputs[input]?.deprecationMessage;
+    if (deprecationMessage) {
+      diagnostics.push({
+        severity: DiagnosticSeverity.Warning,
+        range: mapRange(inputToken.range),
+        message: deprecationMessage
+      });
+    }
+  }
+
+  const missingRequiredInputs = Object.entries(actionInputs).filter(
+    ([inputName, input]) => input.required && !stepInputs.has(inputName)
+  );
+
+  if (missingRequiredInputs.length > 0) {
+    const message =
+      missingRequiredInputs.length === 1
+        ? `Missing required input \`${missingRequiredInputs[0][0]}\``
+        : `Missing required inputs: ${missingRequiredInputs.map(input => `\`${input[0]}\``).join(", ")}`;
+    diagnostics.push({
+      severity: DiagnosticSeverity.Error,
+      range: mapRange((withKey || stepToken).range), // Highlight the whole step if we don't have a with key
+      message: message
+    });
+  }
+}

actions-languageservice/src/validate.actions.test.ts

@@ -0,0 +1,259 @@
+import {Diagnostic, DiagnosticSeverity} from "vscode-languageserver-types";
+import {ActionInput, ActionReference} from "./action";
+import {registerLogger} from "./log";
+import {createDocument} from "./test-utils/document";
+import {TestLogger} from "./test-utils/logger";
+import {validate, ValidationConfig} from "./validate";
+import {ValueProviderKind} from "./value-providers/config";
+
+registerLogger(new TestLogger());
+
+const validationConfig: ValidationConfig = {
+  getActionInputs: async (ref: ActionReference) => {
+    let inputs: Record<string, ActionInput> = {};
+    switch (ref.owner + "/" + ref.name + "@" + ref.ref) {
+      case "actions/checkout@v3":
+        inputs = {
+          repository: {
+            description: "Repository name with owner",
+            default: "${{ github.repository }}"
+          }
+        };
+        break;
+      case "actions/setup-node@v1":
+        inputs = {
+          version: {
+            description: "Deprecated. Use node-version instead. Will not be supported after October 1, 2019",
+            deprecationMessage:
+              "The version property will not be supported after October 1, 2019. Use node-version instead"
+          }
+        };
+        break;
+      case "actions/cache@v1":
+        inputs = {
+          path: {
+            description: "A directory to store and save the cache",
+            required: true
+          },
+          key: {
+            description: "An explicit key for restoring and saving the cache",
+            required: true
+          },
+          "restore-keys": {
+            description: "An ordered list of keys to use for restoring the cache if no cache hit occurred for key",
+            required: false
+          }
+        };
+        break;
+    }
+    return inputs;
+  }
+};
+describe("validate action steps", () => {
+  it("valid action reference", async () => {
+    const input = `
+on: push
+jobs:
+  build:
+    runs-on: ubuntu-latest
+    steps:
+    - uses: actions/checkout@v2
+`;
+    const result = await validate(createDocument("wf.yaml", input), validationConfig);
+
+    expect(result).toEqual([]);
+  });
+
+  it("invalid input", async () => {
+    const input = `
+on: push
+jobs:
+  build:
+    runs-on: ubuntu-latest
+    steps:
+    - uses: actions/checkout@v2
+      with:
+        notanoption: true 
+`;
+    const result = await validate(createDocument("wf.yaml", input), validationConfig);
+
+    expect(result).toEqual([
+      {
+        message: "Invalid action input 'notanoption'",
+        range: {
+          end: {
+            character: 19,
+            line: 8
+          },
+          start: {
+            character: 8,
+            line: 8
+          }
+        },
+        severity: DiagnosticSeverity.Error
+      }
+    ]);
+  });
+
+  it("deprecated input", async () => {
+    const input = `
+on: push
+jobs:
+  build:
+    runs-on: ubuntu-latest
+    steps:
+    - uses: actions/setup-node@v1
+      with:
+        version: 10 
+`;
+    const result = await validate(createDocument("wf.yaml", input), validationConfig);
+
+    expect(result).toEqual([
+      {
+        message: "The version property will not be supported after October 1, 2019. Use node-version instead",
+        range: {
+          end: {
+            character: 15,
+            line: 8
+          },
+          start: {
+            character: 8,
+            line: 8
+          }
+        },
+        severity: DiagnosticSeverity.Warning
+      }
+    ]);
+  });
+
+  it("missing required input", async () => {
+    const input = `
+on: push
+jobs:
+  build:
+    runs-on: ubuntu-latest
+    steps:
+    - uses: actions/cache@v1
+      with:
+        key: \${{ runner.os }}-node-\${{ hashFiles('**/package-lock.json') }}
+`;
+    const result = await validate(createDocument("wf.yaml", input), validationConfig);
+
+    expect(result).toEqual([
+      {
+        message: "Missing required input `path`",
+        range: {
+          end: {
+            character: 10,
+            line: 7
+          },
+          start: {
+            character: 6,
+            line: 7
+          }
+        },
+        severity: DiagnosticSeverity.Error
+      }
+    ]);
+  });
+
+  it("multiple missing required inputs", async () => {
+    const input = `
+on: push
+jobs:
+  build:
+    runs-on: ubuntu-latest
+    steps:
+    - uses: actions/cache@v1
+      with:
+        restore-keys: \${{ runner.os }}-node-
+`;
+    const result = await validate(createDocument("wf.yaml", input), validationConfig);
+
+    expect(result).toEqual([
+      {
+        message: "Missing required inputs: `path`, `key`",
+        range: {
+          end: {
+            character: 10,
+            line: 7
+          },
+          start: {
+            character: 6,
+            line: 7
+          }
+        },
+        severity: DiagnosticSeverity.Error
+      }
+    ]);
+  });
+
+  it("missing required inputs without a `with` key", async () => {
+    const input = `
+on: push
+jobs:
+  build:
+    runs-on: ubuntu-latest
+    steps:
+    - uses: actions/cache@v1
+`;
+    const result = await validate(createDocument("wf.yaml", input), validationConfig);
+
+    expect(result).toEqual([
+      {
+        message: "Missing required inputs: `path`, `key`",
+        range: {
+          end: {
+            character: 0,
+            line: 7
+          },
+          start: {
+            character: 6,
+            line: 6
+          }
+        },
+        severity: DiagnosticSeverity.Error
+      }
+    ]);
+  });
+
+  it("skips extra validation from custom value provider", async () => {
+    const input = `
+on: push
+jobs:
+  build:
+    runs-on: ubuntu-latest
+    steps:
+    - uses: actions/checkout@v2
+      with:
+        notanoption: true 
+`;
+    const config = validationConfig;
+    config.valueProviderConfig = {
+      "step-with": {
+        kind: ValueProviderKind.AllowedValues,
+        get: async () => {
+          return [{label: "repository", description: "Repository name with owner."}];
+        }
+      }
+    };
+    const result = await validate(createDocument("wf.yaml", input), config);
+
+    expect(result).toEqual([
+      {
+        message: "Invalid action input 'notanoption'",
+        range: {
+          end: {
+            character: 19,
+            line: 8
+          },
+          start: {
+            character: 8,
+            line: 8
+          }
+        },
+        severity: DiagnosticSeverity.Error
+      }
+    ]);
+  });
+});

actions-languageservice/src/validate.test.ts

@@ -70,7 +70,7 @@ jobs:
     steps:
     - run: echo`
       ),
-      defaultValueProviders
+      {valueProviderConfig: defaultValueProviders}
     );
 
     expect(result.length).toBe(1);
@@ -103,7 +103,7 @@ jobs:
     steps:
     - run: echo`
       ),
-      defaultValueProviders
+      {valueProviderConfig: defaultValueProviders}
     );
 
     expect(result.length).toBe(1);
@@ -139,7 +139,7 @@ jobs:
     steps:
     - run: echo`
       ),
-      defaultValueProviders
+      {valueProviderConfig: defaultValueProviders}
     );
 
     expect(result[0]).toEqual({
@@ -168,7 +168,7 @@ jobs:
     runs-on:
     - ubuntu-latest`
       ),
-      defaultValueProviders
+      {valueProviderConfig: defaultValueProviders}
     );
 
     expect(result.length).toBe(1);

actions-languageservice/src/validate.ts

@@ -16,6 +16,7 @@ import {TemplateToken} from "@github/actions-workflow-parser/templates/tokens/te
 import {File} from "@github/actions-workflow-parser/workflows/file";
 import {TextDocument} from "vscode-languageserver-textdocument";
 import {Diagnostic, DiagnosticSeverity, URI} from "vscode-languageserver-types";
+import {ActionInputs, ActionReference} from "./action";
 
 import {ContextProviderConfig} from "./context-providers/config";
 import {getContext, Mode} from "./context-providers/default";
@@ -26,9 +27,16 @@ import {nullTrace} from "./nulltrace";
 import {getAllowedContext} from "./utils/allowed-context";
 import {findToken} from "./utils/find-token";
 import {mapRange} from "./utils/range";
+import {validateAction} from "./validate-action";
 import {ValueProviderConfig, ValueProviderKind} from "./value-providers/config";
 import {defaultValueProviders} from "./value-providers/default";
 
+export type ValidationConfig = {
+  valueProviderConfig?: ValueProviderConfig;
+  contextProviderConfig?: ContextProviderConfig;
+  getActionInputs?(action: ActionReference): Promise<ActionInputs | undefined>;
+};
+
 /**
  * Validates a workflow file
  *
@@ -37,9 +45,8 @@ import {defaultValueProviders} from "./value-providers/default";
  */
 export async function validate(
   textDocument: TextDocument,
+  config?: ValidationConfig
   // TODO: Support multiple files, context for API calls
-  valueProviderConfig?: ValueProviderConfig,
-  contextProviderConfig?: ContextProviderConfig
 ): Promise<Diagnostic[]> {
   const file: File = {
     name: textDocument.uri,
@@ -55,14 +62,7 @@ export async function validate(
       const template = convertWorkflowTemplate(result.context, result.value, ErrorPolicy.TryConversion);
 
       // Validate expressions and value providers
-      await additionalValidations(
+      await additionalValidations(diagnostics, textDocument.uri, template, result.value, config);
-        diagnostics,
-        textDocument.uri,
-        template,
-        result.value,
-        valueProviderConfig,
-        contextProviderConfig
-      );
     }
 
     // For now map parser errors directly to diagnostics
@@ -86,8 +86,7 @@ async function additionalValidations(
   documentUri: URI,
   template: WorkflowTemplate,
   root: TemplateToken,
-  valueProviderConfig: ValueProviderConfig | undefined,
+  config?: ValidationConfig
-  contextProviderConfig: ContextProviderConfig | undefined
 ) {
   for (const [parent, token, key] of TemplateToken.traverse(root)) {
     // If  the token is a value in a pair, use the key definition for validation
@@ -103,18 +102,27 @@ async function additionalValidations(
         diagnostics,
         token,
         allowedContext,
-        contextProviderConfig,
+        config?.contextProviderConfig,
         getProviderContext(documentUri, template, root, token)
       );
     }
 
+    if (token.definition?.key === "regular-step") {
+      const context = getProviderContext(documentUri, template, root, token);
+      await validateAction(diagnostics, token, context.step, config);
+    }
+
     // Allowed values coming from the schema have already been validated. Only check if
     // a value provider is defined for a token and if it is, validate the values match.
-    if (valueProviderConfig && token.range && validationDefinition) {
+    if (config?.valueProviderConfig && token.range && validationDefinition) {
       const defKey = validationDefinition.key;
+      if (defKey === "step-with") {
+        // Action inputs should be validated already in validateAction
+        continue;
+      }
 
       // Try a custom value provider first
-      let valueProvider = valueProviderConfig[defKey];
+      let valueProvider = config.valueProviderConfig[defKey];
       if (!valueProvider) {
         // fall back to default
         valueProvider = defaultValueProviders[defKey];