From db997db55248f1a050e492b5a1928cbed31ad689 Mon Sep 17 00:00:00 2001 From: Tianwen Chen Date: Mon, 13 Oct 2025 16:58:05 +1100 Subject: [PATCH 01/11] JobDefinition schema validation for five fields - options - yaml_variables - id_tokens - secrets - interruptible - --- .../ci_job_definitions_config.json | 222 +++++++++++++++++- spec/models/ci/job_definition_spec.rb | 106 ++++++++- 2 files changed, 325 insertions(+), 3 deletions(-) diff --git a/app/validators/json_schemas/ci_job_definitions_config.json b/app/validators/json_schemas/ci_job_definitions_config.json index 34ae945bb2831c..20bcd16f7df101 100644 --- a/app/validators/json_schemas/ci_job_definitions_config.json +++ b/app/validators/json_schemas/ci_job_definitions_config.json @@ -1,5 +1,225 @@ { "$schema": "http://json-schema.org/draft-07/schema#", "description": "CI job configuration options", - "type": "object" + "type": "object", + "additionalProperties": false, + "properties": { + "id_tokens": { + "$ref": "#/definitions/idTokensDefinition" + }, + "interruptible": { + "type": "boolean" + }, + "options": true, + "run_steps": true, + "secrets": { + "$ref": "#/definitions/secretsDefinition" + }, + "tag_list": true, + "yaml_variables": true + }, + "definitions": { + "idTokensDefinition": { + "type": "object", + "patternProperties": { + ".*": { + "type": "object", + "required": [ + "aud" + ], + "properties": { + "aud": { + "oneOf": [ + { + "type": "string" + }, + { + "type": "array", + "items": { + "type": "string" + }, + "minItems": 1, + "uniqueItems": true + } + ] + } + }, + "additionalProperties": false + } + } + }, + "secretsDefinition": { + "type": "object", + "patternProperties": { + ".*": { + "type": "object", + "patternProperties": { + "^vault$": { + "type": "object", + "required": [ + "path", + "field", + "engine" + ], + "properties": { + "path": { + "type": "string" + }, + "field": { + "type": "string" + }, + "engine": { + "type": "object", + "required": [ + "name", + "path" + ], + "properties": { + "path": { + "type": "string" + }, + "name": { + "type": "string" + } + }, + "additionalProperties": false + } + }, + "additionalProperties": false + }, + "^gitlab_secrets_manager$": { + "type": "object", + "required": [ + "name" + ], + "properties": { + "name": { + "type": "string" + } + }, + "additionalProperties": false + }, + "^gcp_secret_manager$": { + "type": "object", + "required": [ + "name" + ], + "properties": { + "name": { + "type": "string" + }, + "version": { + "type": [ + "string", + "null" + ] + } + }, + "additionalProperties": false + }, + "^azure_key_vault$": { + "type": "object", + "required": [ + "name" + ], + "properties": { + "name": { + "type": "string" + }, + "version": { + "type": [ + "string", + "null" + ] + } + }, + "additionalProperties": false + }, + "^aws_secrets_manager$": { + "type": "object", + "required": [ + "secret_id" + ], + "properties": { + "secret_id": { + "type": "string" + }, + "version_id": { + "type": [ + "string", + "null" + ] + }, + "version_stage": { + "type": [ + "string", + "null" + ] + }, + "region": { + "type": [ + "string", + "null" + ] + }, + "role_arn": { + "type": [ + "string", + "null" + ] + }, + "role_session_name": { + "type": [ + "string", + "null" + ] + }, + "field": { + "type": [ + "string", + "null" + ] + } + }, + "additionalProperties": false + }, + "^file$": { + "type": "boolean" + }, + "^token$": { + "type": "string" + } + }, + "anyOf": [ + { + "required": [ + "vault" + ] + }, + { + "required": [ + "gcp_secret_manager" + ] + }, + { + "required": [ + "azure_key_vault" + ] + }, + { + "required": [ + "gitlab_secrets_manager" + ] + }, + { + "required": [ + "aws_secrets_manager" + ] + } + ], + "additionalProperties": false + } + } + } + } } diff --git a/spec/models/ci/job_definition_spec.rb b/spec/models/ci/job_definition_spec.rb index 7fb663c5027c21..9467f44a26f68e 100644 --- a/spec/models/ci/job_definition_spec.rb +++ b/spec/models/ci/job_definition_spec.rb @@ -24,9 +24,75 @@ subject(:job_definition) { build(:ci_job_definition, project: project, config: config) } context 'with valid config' do - let(:config) { { options: { script: ['echo test'] } } } + context 'with id_tokens' do + let(:config) do + { + id_tokens: { + TEST_JWT_TOKEN: { + aud: 'https://gitlab.test' + } + } + } + end + + it { is_expected.to be_valid } + end + + context 'with interruptible' do + let(:config) { { interruptible: true } } + + it { is_expected.to be_valid } + + context 'when false' do + let(:config) { { interruptible: false } } + + it { is_expected.to be_valid } + end + end + + context 'with options' do + let(:config) { { options: { script: ['echo test'] } } } + + it { is_expected.to be_valid } + end + + context 'with run_steps' do + let(:config) do + { run_steps: [{ 'name' => 'step1', 'step' => 'echo', 'inputs' => { 'message' => 'Hello, World!' } }] } + end + + it { is_expected.to be_valid } + end - it { is_expected.to be_valid } + context 'with secrets' do + let(:config) do + { secrets: { + DATABASE_PASSWORD: { + vault: { + engine: { name: 'kv-v2', path: 'kv-v2' }, + path: 'production/db', + field: 'password' + } + } + } } + end + + it { is_expected.to be_valid } + end + + context 'with tag_list' do + let(:config) { { tag_list: ['build'] } } + + it { is_expected.to be_valid } + end + + context 'with yaml_variables' do + let(:config) do + { yaml_variables: [{ key: 'YAML_VARIABLE', value: 'value' }] } + end + + it { is_expected.to be_valid } + end end context 'with invalid config structure' do @@ -36,6 +102,42 @@ expect(job_definition).not_to be_valid expect(job_definition.errors[:config]).to include('must be a valid json schema') end + + context 'with invalid config properties' do + let(:config) { { unknown_property: 'random value' } } + + it 'is invalid' do + expect(job_definition).not_to be_valid + expect(job_definition.errors[:config]).to include('must be a valid json schema') + end + end + + context 'with invalid id_tokens' do + let(:config) { { id_tokens: { TEST_JWT_TOKEN: { id_token: { aud: nil } } } } } + + it 'is invalid' do + expect(job_definition).not_to be_valid + expect(job_definition.errors[:config]).to include('must be a valid json schema') + end + end + + context 'with invalid secrets' do + let(:config) { { secrets: { DATABASE_PASSWORD: { vault: {} } } } } + + it 'is invalid' do + expect(job_definition).not_to be_valid + expect(job_definition.errors[:config]).to include('must be a valid json schema') + end + end + + context 'with invalid interruptible' do + let(:config) { { interruptible: {} } } + + it 'is invalid' do + expect(job_definition).not_to be_valid + expect(job_definition.errors[:config]).to include('must be a valid json schema') + end + end end end end -- GitLab From 62d189df9e582269fa83191f309b3d6110721883 Mon Sep 17 00:00:00 2001 From: Tianwen Chen Date: Tue, 21 Oct 2025 11:21:47 +1100 Subject: [PATCH 02/11] Change to reference other schema json instead of duplicating --- app/models/ci/job_definition.rb | 29 ++- .../ci_job_definition_config.json | 23 ++ .../ci_job_definitions_config.json | 225 ------------------ ...ob_definition_config_schema_validation.yml | 10 + spec/models/ci/job_definition_spec.rb | 179 +++++++++++++- 5 files changed, 232 insertions(+), 234 deletions(-) create mode 100644 app/validators/json_schemas/ci_job_definition_config.json delete mode 100644 app/validators/json_schemas/ci_job_definitions_config.json create mode 100644 config/feature_flags/gitlab_com_derisk/ci_job_definition_config_schema_validation.yml diff --git a/app/models/ci/job_definition.rb b/app/models/ci/job_definition.rb index e93ad4e5369dd6..7a6e51e5e2f8d1 100644 --- a/app/models/ci/job_definition.rb +++ b/app/models/ci/job_definition.rb @@ -31,10 +31,7 @@ class JobDefinition < Ci::ApplicationRecord belongs_to :project validates :project, presence: true - - # rubocop:disable Database/JsonbSizeLimit -- no updates - validates :config, json_schema: { filename: 'ci_job_definitions_config' } - # rubocop:enable Database/JsonbSizeLimit + validate :validate_config_json_schema attribute :config, ::Gitlab::Database::Type::SymbolizedJsonb.new @@ -75,5 +72,29 @@ def self.sanitize_and_checksum(config) def readonly? persisted? end + + def validate_config_json_schema + return if config.blank? + return if Feature.disabled?(:ci_job_definition_config_schema_validation, project) + + validator = JsonSchemaValidator.new({ + filename: 'ci_job_definition_config', + attributes: [:config], + detail_errors: true + }) + + validator.validate(self) + return if errors[:config].empty? + + Gitlab::AppJsonLogger.warn( + class: self.class.name, + message: 'Invalid config schema detected', + job_definition_checksum: checksum, + project_id: project_id, + schema_errors: errors[:config] + ) + + errors.delete(:config) if Rails.env.production? + end end end diff --git a/app/validators/json_schemas/ci_job_definition_config.json b/app/validators/json_schemas/ci_job_definition_config.json new file mode 100644 index 00000000000000..3cd0d7dd329395 --- /dev/null +++ b/app/validators/json_schemas/ci_job_definition_config.json @@ -0,0 +1,23 @@ +{ + "$schema": "http://json-schema.org/draft-07/schema#", + "description": "CI job configuration options", + "type": "object", + "additionalProperties": false, + "properties": { + "id_tokens": { + "$ref": "./build_metadata_id_tokens.json" + }, + "interruptible": { + "type": "boolean" + }, + "options": { + "$ref": "./build_metadata_config_options.json" + }, + "run_steps": true, + "secrets": { + "$ref": "./build_metadata_secrets.json" + }, + "tag_list": true, + "yaml_variables": true + } +} diff --git a/app/validators/json_schemas/ci_job_definitions_config.json b/app/validators/json_schemas/ci_job_definitions_config.json deleted file mode 100644 index 20bcd16f7df101..00000000000000 --- a/app/validators/json_schemas/ci_job_definitions_config.json +++ /dev/null @@ -1,225 +0,0 @@ -{ - "$schema": "http://json-schema.org/draft-07/schema#", - "description": "CI job configuration options", - "type": "object", - "additionalProperties": false, - "properties": { - "id_tokens": { - "$ref": "#/definitions/idTokensDefinition" - }, - "interruptible": { - "type": "boolean" - }, - "options": true, - "run_steps": true, - "secrets": { - "$ref": "#/definitions/secretsDefinition" - }, - "tag_list": true, - "yaml_variables": true - }, - "definitions": { - "idTokensDefinition": { - "type": "object", - "patternProperties": { - ".*": { - "type": "object", - "required": [ - "aud" - ], - "properties": { - "aud": { - "oneOf": [ - { - "type": "string" - }, - { - "type": "array", - "items": { - "type": "string" - }, - "minItems": 1, - "uniqueItems": true - } - ] - } - }, - "additionalProperties": false - } - } - }, - "secretsDefinition": { - "type": "object", - "patternProperties": { - ".*": { - "type": "object", - "patternProperties": { - "^vault$": { - "type": "object", - "required": [ - "path", - "field", - "engine" - ], - "properties": { - "path": { - "type": "string" - }, - "field": { - "type": "string" - }, - "engine": { - "type": "object", - "required": [ - "name", - "path" - ], - "properties": { - "path": { - "type": "string" - }, - "name": { - "type": "string" - } - }, - "additionalProperties": false - } - }, - "additionalProperties": false - }, - "^gitlab_secrets_manager$": { - "type": "object", - "required": [ - "name" - ], - "properties": { - "name": { - "type": "string" - } - }, - "additionalProperties": false - }, - "^gcp_secret_manager$": { - "type": "object", - "required": [ - "name" - ], - "properties": { - "name": { - "type": "string" - }, - "version": { - "type": [ - "string", - "null" - ] - } - }, - "additionalProperties": false - }, - "^azure_key_vault$": { - "type": "object", - "required": [ - "name" - ], - "properties": { - "name": { - "type": "string" - }, - "version": { - "type": [ - "string", - "null" - ] - } - }, - "additionalProperties": false - }, - "^aws_secrets_manager$": { - "type": "object", - "required": [ - "secret_id" - ], - "properties": { - "secret_id": { - "type": "string" - }, - "version_id": { - "type": [ - "string", - "null" - ] - }, - "version_stage": { - "type": [ - "string", - "null" - ] - }, - "region": { - "type": [ - "string", - "null" - ] - }, - "role_arn": { - "type": [ - "string", - "null" - ] - }, - "role_session_name": { - "type": [ - "string", - "null" - ] - }, - "field": { - "type": [ - "string", - "null" - ] - } - }, - "additionalProperties": false - }, - "^file$": { - "type": "boolean" - }, - "^token$": { - "type": "string" - } - }, - "anyOf": [ - { - "required": [ - "vault" - ] - }, - { - "required": [ - "gcp_secret_manager" - ] - }, - { - "required": [ - "azure_key_vault" - ] - }, - { - "required": [ - "gitlab_secrets_manager" - ] - }, - { - "required": [ - "aws_secrets_manager" - ] - } - ], - "additionalProperties": false - } - } - } - } -} diff --git a/config/feature_flags/gitlab_com_derisk/ci_job_definition_config_schema_validation.yml b/config/feature_flags/gitlab_com_derisk/ci_job_definition_config_schema_validation.yml new file mode 100644 index 00000000000000..68c78adff468c0 --- /dev/null +++ b/config/feature_flags/gitlab_com_derisk/ci_job_definition_config_schema_validation.yml @@ -0,0 +1,10 @@ +--- +name: ci_job_definition_config_schema_validation +description: +feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/560157 +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/208586 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/577626 +milestone: '18.5' +group: group::ci platform +type: gitlab_com_derisk +default_enabled: false diff --git a/spec/models/ci/job_definition_spec.rb b/spec/models/ci/job_definition_spec.rb index 9467f44a26f68e..43a3feefff9fd2 100644 --- a/spec/models/ci/job_definition_spec.rb +++ b/spec/models/ci/job_definition_spec.rb @@ -99,16 +99,31 @@ let(:config) { 'invalid' } it 'is invalid' do + expect(Gitlab::AppJsonLogger).to receive(:warn).with( + class: described_class.name, + message: 'Invalid config schema detected', + job_definition_checksum: job_definition.checksum, + project_id: job_definition.project_id, + schema_errors: ['value at root is not an object'] + ) expect(job_definition).not_to be_valid - expect(job_definition.errors[:config]).to include('must be a valid json schema') + expect(job_definition.errors[:config]).to include('value at root is not an object') end context 'with invalid config properties' do let(:config) { { unknown_property: 'random value' } } it 'is invalid' do + expect(Gitlab::AppJsonLogger).to receive(:warn).with( + class: described_class.name, + message: 'Invalid config schema detected', + job_definition_checksum: job_definition.checksum, + project_id: job_definition.project_id, + schema_errors: ['object property at `/unknown_property` is a disallowed additional property'] + ) expect(job_definition).not_to be_valid - expect(job_definition.errors[:config]).to include('must be a valid json schema') + expect(job_definition.errors[:config]).to include( + 'object property at `/unknown_property` is a disallowed additional property') end end @@ -116,8 +131,20 @@ let(:config) { { id_tokens: { TEST_JWT_TOKEN: { id_token: { aud: nil } } } } } it 'is invalid' do + expect(Gitlab::AppJsonLogger).to receive(:warn).with( + class: described_class.name, + message: 'Invalid config schema detected', + job_definition_checksum: job_definition.checksum, + project_id: job_definition.project_id, + schema_errors: [ + 'object property at `/id_tokens/TEST_JWT_TOKEN/id_token` is a disallowed additional property', + 'object at `/id_tokens/TEST_JWT_TOKEN` is missing required properties: aud' + ] + ) expect(job_definition).not_to be_valid - expect(job_definition.errors[:config]).to include('must be a valid json schema') + expect(job_definition.errors[:config]).to include( + 'object property at `/id_tokens/TEST_JWT_TOKEN/id_token` is a disallowed additional property', + 'object at `/id_tokens/TEST_JWT_TOKEN` is missing required properties: aud') end end @@ -125,8 +152,18 @@ let(:config) { { secrets: { DATABASE_PASSWORD: { vault: {} } } } } it 'is invalid' do + expect(Gitlab::AppJsonLogger).to receive(:warn).with( + class: described_class.name, + message: 'Invalid config schema detected', + job_definition_checksum: job_definition.checksum, + project_id: job_definition.project_id, + schema_errors: [ + 'object at `/secrets/DATABASE_PASSWORD/vault` is missing required properties: path, field, engine' + ] + ) expect(job_definition).not_to be_valid - expect(job_definition.errors[:config]).to include('must be a valid json schema') + expect(job_definition.errors[:config]).to include( + 'object at `/secrets/DATABASE_PASSWORD/vault` is missing required properties: path, field, engine') end end @@ -134,8 +171,140 @@ let(:config) { { interruptible: {} } } it 'is invalid' do + expect(Gitlab::AppJsonLogger).to receive(:warn).with( + class: described_class.name, + message: 'Invalid config schema detected', + job_definition_checksum: job_definition.checksum, + project_id: job_definition.project_id, + schema_errors: ['value at `/interruptible` is not a boolean'] + ) expect(job_definition).not_to be_valid - expect(job_definition.errors[:config]).to include('must be a valid json schema') + expect(job_definition.errors[:config]).to include( + 'value at `/interruptible` is not a boolean') + end + end + + context 'when env is production' do + before do + allow(Rails.env).to receive(:production?).and_return(true) + end + + it 'logs the validation errors but behaves like valid' do + expect(Gitlab::AppJsonLogger).to receive(:warn).with( + class: described_class.name, + message: 'Invalid config schema detected', + job_definition_checksum: job_definition.checksum, + project_id: job_definition.project_id, + schema_errors: ['value at root is not an object'] + ) + expect(job_definition).to be_valid + end + + context 'with invalid config properties' do + let(:config) { { unknown_property: 'random value' } } + + it 'logs the validation errors but behaves like valid' do + expect(Gitlab::AppJsonLogger).to receive(:warn).with( + class: described_class.name, + message: 'Invalid config schema detected', + job_definition_checksum: job_definition.checksum, + project_id: job_definition.project_id, + schema_errors: ['object property at `/unknown_property` is a disallowed additional property'] + ) + expect(job_definition).to be_valid + end + end + + context 'with invalid id_tokens' do + let(:config) { { id_tokens: { TEST_JWT_TOKEN: { id_token: { aud: nil } } } } } + + it 'logs the validation errors but behaves like valid' do + expect(Gitlab::AppJsonLogger).to receive(:warn).with( + class: described_class.name, + message: 'Invalid config schema detected', + job_definition_checksum: job_definition.checksum, + project_id: job_definition.project_id, + schema_errors: [ + 'object property at `/id_tokens/TEST_JWT_TOKEN/id_token` is a disallowed additional property', + 'object at `/id_tokens/TEST_JWT_TOKEN` is missing required properties: aud' + ] + ) + expect(job_definition).to be_valid + end + end + + context 'with invalid secrets' do + let(:config) { { secrets: { DATABASE_PASSWORD: { vault: {} } } } } + + it 'logs the validation errors but behaves like valid' do + expect(Gitlab::AppJsonLogger).to receive(:warn).with( + class: described_class.name, + message: 'Invalid config schema detected', + job_definition_checksum: job_definition.checksum, + project_id: job_definition.project_id, + schema_errors: [ + 'object at `/secrets/DATABASE_PASSWORD/vault` is missing required properties: path, field, engine' + ] + ) + expect(job_definition).to be_valid + end + end + + context 'with invalid interruptible' do + let(:config) { { interruptible: {} } } + + it 'logs the validation errors but behaves like valid' do + expect(Gitlab::AppJsonLogger).to receive(:warn).with( + class: described_class.name, + message: 'Invalid config schema detected', + job_definition_checksum: job_definition.checksum, + project_id: job_definition.project_id, + schema_errors: ['value at `/interruptible` is not a boolean'] + ) + expect(job_definition).to be_valid + end + end + end + + context 'when ff ci_job_definition_config_schema_validation is disabled' do + before do + stub_feature_flags(ci_job_definition_config_schema_validation: false) + end + + it 'is valid' do + expect(job_definition).to be_valid + end + + context 'with invalid config properties' do + let(:config) { { unknown_property: 'random value' } } + + it 'is valid' do + expect(job_definition).to be_valid + end + end + + context 'with invalid id_tokens' do + let(:config) { { id_tokens: { TEST_JWT_TOKEN: { id_token: { aud: nil } } } } } + + it 'is valid' do + expect(job_definition).to be_valid + end + end + + context 'with invalid secrets' do + let(:config) { { secrets: { DATABASE_PASSWORD: { vault: {} } } } } + + it 'is valid' do + expect(job_definition).to be_valid + end + end + + context 'with invalid interruptible' do + let(:config) { { interruptible: {} } } + + it 'is valid' do + expect(job_definition).to be_valid + end end end end -- GitLab From f4cafee08930d9f35c065ae867d1a5266b3d3595 Mon Sep 17 00:00:00 2001 From: Tianwen Chen Date: Tue, 21 Oct 2025 15:27:59 +1100 Subject: [PATCH 03/11] Add validation for tag_list --- .../ci_job_definition_config.json | 4 +- .../json_schemas/ci_job_definition_tags.json | 9 ++ spec/models/ci/job_definition_spec.rb | 92 +++++++++++++++++-- 3 files changed, 97 insertions(+), 8 deletions(-) create mode 100644 app/validators/json_schemas/ci_job_definition_tags.json diff --git a/app/validators/json_schemas/ci_job_definition_config.json b/app/validators/json_schemas/ci_job_definition_config.json index 3cd0d7dd329395..12ac44afe0802b 100644 --- a/app/validators/json_schemas/ci_job_definition_config.json +++ b/app/validators/json_schemas/ci_job_definition_config.json @@ -17,7 +17,9 @@ "secrets": { "$ref": "./build_metadata_secrets.json" }, - "tag_list": true, + "tag_list": { + "$ref": "./ci_job_definition_tags.json" + }, "yaml_variables": true } } diff --git a/app/validators/json_schemas/ci_job_definition_tags.json b/app/validators/json_schemas/ci_job_definition_tags.json new file mode 100644 index 00000000000000..ae2a7a3205e70a --- /dev/null +++ b/app/validators/json_schemas/ci_job_definition_tags.json @@ -0,0 +1,9 @@ +{ + "$schema": "http://json-schema.org/draft-07/schema#", + "description": "CI job tags", + "type": "array", + "items": { + "type": "string" + }, + "uniqueItems": true +} diff --git a/spec/models/ci/job_definition_spec.rb b/spec/models/ci/job_definition_spec.rb index 43a3feefff9fd2..464883c634359d 100644 --- a/spec/models/ci/job_definition_spec.rb +++ b/spec/models/ci/job_definition_spec.rb @@ -84,6 +84,12 @@ let(:config) { { tag_list: ['build'] } } it { is_expected.to be_valid } + + context 'when empty tag_list' do + let(:config) { { tag_list: [] } } + + it { is_expected.to be_valid } + end end context 'with yaml_variables' do @@ -148,6 +154,23 @@ end end + context 'with invalid interruptible' do + let(:config) { { interruptible: {} } } + + it 'is invalid' do + expect(Gitlab::AppJsonLogger).to receive(:warn).with( + class: described_class.name, + message: 'Invalid config schema detected', + job_definition_checksum: job_definition.checksum, + project_id: job_definition.project_id, + schema_errors: ['value at `/interruptible` is not a boolean'] + ) + expect(job_definition).not_to be_valid + expect(job_definition.errors[:config]).to include( + 'value at `/interruptible` is not a boolean') + end + end + context 'with invalid secrets' do let(:config) { { secrets: { DATABASE_PASSWORD: { vault: {} } } } } @@ -167,8 +190,8 @@ end end - context 'with invalid interruptible' do - let(:config) { { interruptible: {} } } + context 'with invalid tags' do + let(:config) { { tag_list: 'one-tag' } } it 'is invalid' do expect(Gitlab::AppJsonLogger).to receive(:warn).with( @@ -176,11 +199,32 @@ message: 'Invalid config schema detected', job_definition_checksum: job_definition.checksum, project_id: job_definition.project_id, - schema_errors: ['value at `/interruptible` is not a boolean'] + schema_errors: [ + 'value at `/tag_list` is not an array' + ] ) expect(job_definition).not_to be_valid expect(job_definition.errors[:config]).to include( - 'value at `/interruptible` is not a boolean') + 'value at `/tag_list` is not an array') + end + + context 'for duplication' do + let(:config) { { tag_list: %w[repeating-tag repeating-tag] } } + + it 'is invalid' do + expect(Gitlab::AppJsonLogger).to receive(:warn).with( + class: described_class.name, + message: 'Invalid config schema detected', + job_definition_checksum: job_definition.checksum, + project_id: job_definition.project_id, + schema_errors: [ + 'array items at `/tag_list` are not unique' + ] + ) + expect(job_definition).not_to be_valid + expect(job_definition.errors[:config]).to include( + 'array items at `/tag_list` are not unique') + end end end @@ -233,6 +277,21 @@ end end + context 'with invalid interruptible' do + let(:config) { { interruptible: {} } } + + it 'logs the validation errors but behaves like valid' do + expect(Gitlab::AppJsonLogger).to receive(:warn).with( + class: described_class.name, + message: 'Invalid config schema detected', + job_definition_checksum: job_definition.checksum, + project_id: job_definition.project_id, + schema_errors: ['value at `/interruptible` is not a boolean'] + ) + expect(job_definition).to be_valid + end + end + context 'with invalid secrets' do let(:config) { { secrets: { DATABASE_PASSWORD: { vault: {} } } } } @@ -250,8 +309,8 @@ end end - context 'with invalid interruptible' do - let(:config) { { interruptible: {} } } + context 'with invalid tags' do + let(:config) { { tag_list: 'one-tag' } } it 'logs the validation errors but behaves like valid' do expect(Gitlab::AppJsonLogger).to receive(:warn).with( @@ -259,10 +318,29 @@ message: 'Invalid config schema detected', job_definition_checksum: job_definition.checksum, project_id: job_definition.project_id, - schema_errors: ['value at `/interruptible` is not a boolean'] + schema_errors: [ + 'value at `/tag_list` is not an array' + ] ) expect(job_definition).to be_valid end + + context 'for duplication' do + let(:config) { { tag_list: %w[repeating-tag repeating-tag] } } + + it 'logs the validation errors but behaves like valid' do + expect(Gitlab::AppJsonLogger).to receive(:warn).with( + class: described_class.name, + message: 'Invalid config schema detected', + job_definition_checksum: job_definition.checksum, + project_id: job_definition.project_id, + schema_errors: [ + 'array items at `/tag_list` are not unique' + ] + ) + expect(job_definition).to be_valid + end + end end end -- GitLab From 64861878e579c7d20bf586bc3ea8fafdbfea9121 Mon Sep 17 00:00:00 2001 From: Tianwen Chen Date: Tue, 21 Oct 2025 15:53:43 +1100 Subject: [PATCH 04/11] Add validation for yaml_variables --- .../ci_job_definition_config.json | 4 +- .../ci_job_definition_yaml_variables.json | 20 +++++ spec/models/ci/job_definition_spec.rb | 76 ++++++++++++++++++- 3 files changed, 97 insertions(+), 3 deletions(-) create mode 100644 app/validators/json_schemas/ci_job_definition_yaml_variables.json diff --git a/app/validators/json_schemas/ci_job_definition_config.json b/app/validators/json_schemas/ci_job_definition_config.json index 12ac44afe0802b..4c7e21f45a6742 100644 --- a/app/validators/json_schemas/ci_job_definition_config.json +++ b/app/validators/json_schemas/ci_job_definition_config.json @@ -20,6 +20,8 @@ "tag_list": { "$ref": "./ci_job_definition_tags.json" }, - "yaml_variables": true + "yaml_variables": { + "$ref": "./ci_job_definition_yaml_variables.json" + } } } diff --git a/app/validators/json_schemas/ci_job_definition_yaml_variables.json b/app/validators/json_schemas/ci_job_definition_yaml_variables.json new file mode 100644 index 00000000000000..3587cd808fa6f9 --- /dev/null +++ b/app/validators/json_schemas/ci_job_definition_yaml_variables.json @@ -0,0 +1,20 @@ +{ + "$schema": "http://json-schema.org/draft-07/schema#", + "description": "CI job yaml variables", + "type": "array", + "items": { + "type": "object", + "additionalProperties": false, + "properties": { + "key": { + "type": "string" + }, + "public": { + "type": "boolean" + }, + "value": { + "type": "string" + } + } + } +} diff --git a/spec/models/ci/job_definition_spec.rb b/spec/models/ci/job_definition_spec.rb index 464883c634359d..6db99963ec2fdf 100644 --- a/spec/models/ci/job_definition_spec.rb +++ b/spec/models/ci/job_definition_spec.rb @@ -190,7 +190,7 @@ end end - context 'with invalid tags' do + context 'with invalid tag_list' do let(:config) { { tag_list: 'one-tag' } } it 'is invalid' do @@ -228,6 +228,44 @@ end end + context 'with invalid yaml_variables' do + let(:config) { { yaml_variables: 'invalid' } } + + it 'is invalid' do + expect(Gitlab::AppJsonLogger).to receive(:warn).with( + class: described_class.name, + message: 'Invalid config schema detected', + job_definition_checksum: job_definition.checksum, + project_id: job_definition.project_id, + schema_errors: [ + 'value at `/yaml_variables` is not an array' + ] + ) + expect(job_definition).not_to be_valid + expect(job_definition.errors[:config]).to include( + 'value at `/yaml_variables` is not an array') + end + + context 'for invalid item' do + let(:config) { { yaml_variables: [{ key: "RAILS_ENV", unknown_property: true }] } } + + it 'is invalid' do + expect(Gitlab::AppJsonLogger).to receive(:warn).with( + class: described_class.name, + message: 'Invalid config schema detected', + job_definition_checksum: job_definition.checksum, + project_id: job_definition.project_id, + schema_errors: [ + 'object property at `/yaml_variables/0/unknown_property` is a disallowed additional property' + ] + ) + expect(job_definition).not_to be_valid + expect(job_definition.errors[:config]).to include( + 'object property at `/yaml_variables/0/unknown_property` is a disallowed additional property') + end + end + end + context 'when env is production' do before do allow(Rails.env).to receive(:production?).and_return(true) @@ -309,7 +347,7 @@ end end - context 'with invalid tags' do + context 'with invalid tag_list' do let(:config) { { tag_list: 'one-tag' } } it 'logs the validation errors but behaves like valid' do @@ -342,6 +380,40 @@ end end end + + context 'with invalid yaml_variables' do + let(:config) { { yaml_variables: 'invalid' } } + + it 'logs the validation errors but behaves like valid' do + expect(Gitlab::AppJsonLogger).to receive(:warn).with( + class: described_class.name, + message: 'Invalid config schema detected', + job_definition_checksum: job_definition.checksum, + project_id: job_definition.project_id, + schema_errors: [ + 'value at `/yaml_variables` is not an array' + ] + ) + expect(job_definition).to be_valid + end + + context 'for invalid item' do + let(:config) { { yaml_variables: [{ key: "RAILS_ENV", unknown_property: true }] } } + + it 'logs the validation errors but behaves like valid' do + expect(Gitlab::AppJsonLogger).to receive(:warn).with( + class: described_class.name, + message: 'Invalid config schema detected', + job_definition_checksum: job_definition.checksum, + project_id: job_definition.project_id, + schema_errors: [ + 'object property at `/yaml_variables/0/unknown_property` is a disallowed additional property' + ] + ) + expect(job_definition).to be_valid + end + end + end end context 'when ff ci_job_definition_config_schema_validation is disabled' do -- GitLab From 8a8159028880a28eb3a84820d94baf72df2a9be7 Mon Sep 17 00:00:00 2001 From: Tianwen Chen Date: Tue, 21 Oct 2025 16:06:55 +1100 Subject: [PATCH 05/11] Add validation for run_steps --- .../ci_job_definition_config.json | 4 ++- spec/models/ci/job_definition_spec.rb | 36 +++++++++++++++++++ 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/app/validators/json_schemas/ci_job_definition_config.json b/app/validators/json_schemas/ci_job_definition_config.json index 4c7e21f45a6742..4b1a8ff3287ef5 100644 --- a/app/validators/json_schemas/ci_job_definition_config.json +++ b/app/validators/json_schemas/ci_job_definition_config.json @@ -13,7 +13,9 @@ "options": { "$ref": "./build_metadata_config_options.json" }, - "run_steps": true, + "run_steps": { + "$ref": "./run_steps.json" + }, "secrets": { "$ref": "./build_metadata_secrets.json" }, diff --git a/spec/models/ci/job_definition_spec.rb b/spec/models/ci/job_definition_spec.rb index 6db99963ec2fdf..dc402b0997baa7 100644 --- a/spec/models/ci/job_definition_spec.rb +++ b/spec/models/ci/job_definition_spec.rb @@ -171,6 +171,25 @@ end end + context 'with invalid run_steps' do + let(:config) { { run_steps: {} } } + + it 'is invalid' do + expect(Gitlab::AppJsonLogger).to receive(:warn).with( + class: described_class.name, + message: 'Invalid config schema detected', + job_definition_checksum: job_definition.checksum, + project_id: job_definition.project_id, + schema_errors: [ + 'value at `/run_steps` is not an array' + ] + ) + expect(job_definition).not_to be_valid + expect(job_definition.errors[:config]).to include( + 'value at `/run_steps` is not an array') + end + end + context 'with invalid secrets' do let(:config) { { secrets: { DATABASE_PASSWORD: { vault: {} } } } } @@ -330,6 +349,23 @@ end end + context 'with invalid run_steps' do + let(:config) { { run_steps: {} } } + + it 'logs the validation errors but behaves like valid' do + expect(Gitlab::AppJsonLogger).to receive(:warn).with( + class: described_class.name, + message: 'Invalid config schema detected', + job_definition_checksum: job_definition.checksum, + project_id: job_definition.project_id, + schema_errors: [ + 'value at `/run_steps` is not an array' + ] + ) + expect(job_definition).to be_valid + end + end + context 'with invalid secrets' do let(:config) { { secrets: { DATABASE_PASSWORD: { vault: {} } } } } -- GitLab From 9566b1ae93cf238f04893d745df7b842627b3b96 Mon Sep 17 00:00:00 2001 From: Tianwen Chen Date: Tue, 21 Oct 2025 16:07:52 +1100 Subject: [PATCH 06/11] Remove the uniqueItems for tag_list --- .../json_schemas/ci_job_definition_tags.json | 3 +- spec/models/ci/job_definition_spec.rb | 36 ------------------- 2 files changed, 1 insertion(+), 38 deletions(-) diff --git a/app/validators/json_schemas/ci_job_definition_tags.json b/app/validators/json_schemas/ci_job_definition_tags.json index ae2a7a3205e70a..1ef5500e994400 100644 --- a/app/validators/json_schemas/ci_job_definition_tags.json +++ b/app/validators/json_schemas/ci_job_definition_tags.json @@ -4,6 +4,5 @@ "type": "array", "items": { "type": "string" - }, - "uniqueItems": true + } } diff --git a/spec/models/ci/job_definition_spec.rb b/spec/models/ci/job_definition_spec.rb index dc402b0997baa7..e9b594aef1ce95 100644 --- a/spec/models/ci/job_definition_spec.rb +++ b/spec/models/ci/job_definition_spec.rb @@ -226,25 +226,6 @@ expect(job_definition.errors[:config]).to include( 'value at `/tag_list` is not an array') end - - context 'for duplication' do - let(:config) { { tag_list: %w[repeating-tag repeating-tag] } } - - it 'is invalid' do - expect(Gitlab::AppJsonLogger).to receive(:warn).with( - class: described_class.name, - message: 'Invalid config schema detected', - job_definition_checksum: job_definition.checksum, - project_id: job_definition.project_id, - schema_errors: [ - 'array items at `/tag_list` are not unique' - ] - ) - expect(job_definition).not_to be_valid - expect(job_definition.errors[:config]).to include( - 'array items at `/tag_list` are not unique') - end - end end context 'with invalid yaml_variables' do @@ -398,23 +379,6 @@ ) expect(job_definition).to be_valid end - - context 'for duplication' do - let(:config) { { tag_list: %w[repeating-tag repeating-tag] } } - - it 'logs the validation errors but behaves like valid' do - expect(Gitlab::AppJsonLogger).to receive(:warn).with( - class: described_class.name, - message: 'Invalid config schema detected', - job_definition_checksum: job_definition.checksum, - project_id: job_definition.project_id, - schema_errors: [ - 'array items at `/tag_list` are not unique' - ] - ) - expect(job_definition).to be_valid - end - end end context 'with invalid yaml_variables' do -- GitLab From 31b9f0f5e260e661c4cfc5b8125ff5190e1afb0d Mon Sep 17 00:00:00 2001 From: Tianwen Chen Date: Tue, 21 Oct 2025 16:10:31 +1100 Subject: [PATCH 07/11] Update ff milestone to 18.6 --- .../ci_job_definition_config_schema_validation.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/feature_flags/gitlab_com_derisk/ci_job_definition_config_schema_validation.yml b/config/feature_flags/gitlab_com_derisk/ci_job_definition_config_schema_validation.yml index 68c78adff468c0..066233a2fc8d98 100644 --- a/config/feature_flags/gitlab_com_derisk/ci_job_definition_config_schema_validation.yml +++ b/config/feature_flags/gitlab_com_derisk/ci_job_definition_config_schema_validation.yml @@ -4,7 +4,7 @@ description: feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/560157 introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/208586 rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/577626 -milestone: '18.5' +milestone: '18.6' group: group::ci platform type: gitlab_com_derisk default_enabled: false -- GitLab From 4f6d2f52881dde7585ba0bc09dfa9826e35ef956 Mon Sep 17 00:00:00 2001 From: Tianwen Chen Date: Tue, 21 Oct 2025 16:36:44 +1100 Subject: [PATCH 08/11] Skip until we rollout the ci_job_definition_config_schema_validation --- .../shared_examples/models/jsonb_column_validation_todo.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/spec/support/shared_examples/models/jsonb_column_validation_todo.yml b/spec/support/shared_examples/models/jsonb_column_validation_todo.yml index df4872a313a56f..b5ad8e1dfeb748 100644 --- a/spec/support/shared_examples/models/jsonb_column_validation_todo.yml +++ b/spec/support/shared_examples/models/jsonb_column_validation_todo.yml @@ -14,6 +14,7 @@ - ApplicationSetting#token_prefixes - Ci::BuildMetadata#config_options - Ci::BuildMetadata#config_variables +- Ci::JobDefinition#config - GeoNodeStatus#status - Geo::Event#payload - Geo::SecondaryUsageData#payload -- GitLab From c2e51f9c721afa6137b3e2d792b325fb7c7bb1fb Mon Sep 17 00:00:00 2001 From: Tianwen Chen Date: Tue, 21 Oct 2025 16:56:07 +1100 Subject: [PATCH 09/11] Add json schema to CODEOWNERS --- .gitlab/CODEOWNERS | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.gitlab/CODEOWNERS b/.gitlab/CODEOWNERS index 6335ea5af6d95c..2344b9d67787ca 100644 --- a/.gitlab/CODEOWNERS +++ b/.gitlab/CODEOWNERS @@ -1615,6 +1615,10 @@ lib/gitlab/checks/ /app/uploaders/job_artifact_uploader.rb /app/validators/json_schemas/build_metadata_id_tokens.json /app/validators/json_schemas/build_metadata_config_options.json +/app/validators/json_schemas/build_metadata_secrets.json +/app/validators/json_schemas/ci_job_definition_config.json +/app/validators/json_schemas/ci_job_definition_tags.json +/app/validators/json_schemas/ci_job_definition_yaml_variables.json /app/workers/build_queue_worker.rb /app/workers/create_pipeline_worker.rb /app/workers/expire_build_artifacts_worker.rb -- GitLab From 1bf31cf205d4007b45f5d3426f2033b017e95eaf Mon Sep 17 00:00:00 2001 From: Tianwen Chen Date: Tue, 21 Oct 2025 18:29:14 +1100 Subject: [PATCH 10/11] Take out public --- .../json_schemas/ci_job_definition_yaml_variables.json | 3 --- 1 file changed, 3 deletions(-) diff --git a/app/validators/json_schemas/ci_job_definition_yaml_variables.json b/app/validators/json_schemas/ci_job_definition_yaml_variables.json index 3587cd808fa6f9..3f81ebe4561011 100644 --- a/app/validators/json_schemas/ci_job_definition_yaml_variables.json +++ b/app/validators/json_schemas/ci_job_definition_yaml_variables.json @@ -9,9 +9,6 @@ "key": { "type": "string" }, - "public": { - "type": "boolean" - }, "value": { "type": "string" } -- GitLab From 5960687866c4f50484e07e131666ca19b2fc5e58 Mon Sep 17 00:00:00 2001 From: Tianwen Chen Date: Wed, 22 Oct 2025 15:36:32 +1100 Subject: [PATCH 11/11] More properties for yaml_variables json schema --- .gitlab/CODEOWNERS | 1 + .../ci_job_definition_yaml_variables.json | 12 ++++++++++++ 2 files changed, 13 insertions(+) diff --git a/.gitlab/CODEOWNERS b/.gitlab/CODEOWNERS index 2344b9d67787ca..996b56efa16043 100644 --- a/.gitlab/CODEOWNERS +++ b/.gitlab/CODEOWNERS @@ -1619,6 +1619,7 @@ lib/gitlab/checks/ /app/validators/json_schemas/ci_job_definition_config.json /app/validators/json_schemas/ci_job_definition_tags.json /app/validators/json_schemas/ci_job_definition_yaml_variables.json +/app/validators/json_schemas/run_steps.json /app/workers/build_queue_worker.rb /app/workers/create_pipeline_worker.rb /app/workers/expire_build_artifacts_worker.rb diff --git a/app/validators/json_schemas/ci_job_definition_yaml_variables.json b/app/validators/json_schemas/ci_job_definition_yaml_variables.json index 3f81ebe4561011..a1720b8a20a167 100644 --- a/app/validators/json_schemas/ci_job_definition_yaml_variables.json +++ b/app/validators/json_schemas/ci_job_definition_yaml_variables.json @@ -6,9 +6,21 @@ "type": "object", "additionalProperties": false, "properties": { + "description": { + "type": "string" + }, "key": { "type": "string" }, + "options": { + "type": "array", + "items": { + "type": "string" + } + }, + "raw": { + "type": "boolean" + }, "value": { "type": "string" } -- GitLab