diff --git a/.gitlab/CODEOWNERS b/.gitlab/CODEOWNERS index 6335ea5af6d95cdfc1db8b7006abb9f24c18bc7f..996b56efa16043c14c50fcd91b90800a2562fb87 100644 --- a/.gitlab/CODEOWNERS +++ b/.gitlab/CODEOWNERS @@ -1615,6 +1615,11 @@ 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/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/models/ci/job_definition.rb b/app/models/ci/job_definition.rb index e93ad4e5369dd62cbd25a453156066bef04b0be8..7a6e51e5e2f8d1075a1f000f493f48481bb16a0d 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 0000000000000000000000000000000000000000..4b1a8ff3287ef5b94fe4e87bb66f69430f999a82 --- /dev/null +++ b/app/validators/json_schemas/ci_job_definition_config.json @@ -0,0 +1,29 @@ +{ + "$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": { + "$ref": "./run_steps.json" + }, + "secrets": { + "$ref": "./build_metadata_secrets.json" + }, + "tag_list": { + "$ref": "./ci_job_definition_tags.json" + }, + "yaml_variables": { + "$ref": "./ci_job_definition_yaml_variables.json" + } + } +} 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 0000000000000000000000000000000000000000..1ef5500e9944004428072bf26701e7cc2722e611 --- /dev/null +++ b/app/validators/json_schemas/ci_job_definition_tags.json @@ -0,0 +1,8 @@ +{ + "$schema": "http://json-schema.org/draft-07/schema#", + "description": "CI job tags", + "type": "array", + "items": { + "type": "string" + } +} 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 0000000000000000000000000000000000000000..a1720b8a20a1675cd559300963239538ef7506b1 --- /dev/null +++ b/app/validators/json_schemas/ci_job_definition_yaml_variables.json @@ -0,0 +1,29 @@ +{ + "$schema": "http://json-schema.org/draft-07/schema#", + "description": "CI job yaml variables", + "type": "array", + "items": { + "type": "object", + "additionalProperties": false, + "properties": { + "description": { + "type": "string" + }, + "key": { + "type": "string" + }, + "options": { + "type": "array", + "items": { + "type": "string" + } + }, + "raw": { + "type": "boolean" + }, + "value": { + "type": "string" + } + } + } +} 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 34ae945bb2831cc745febc7250b6f8dd501c4bcf..0000000000000000000000000000000000000000 --- a/app/validators/json_schemas/ci_job_definitions_config.json +++ /dev/null @@ -1,5 +0,0 @@ -{ - "$schema": "http://json-schema.org/draft-07/schema#", - "description": "CI job configuration options", - "type": "object" -} 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 0000000000000000000000000000000000000000..066233a2fc8d98692c778ab66096286a3194ce0d --- /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.6' +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 7fb663c5027c218bfa4d33fb93800136c9179c3c..e9b594aef1ce9577198929db7e2ec933ed904b84 100644 --- a/spec/models/ci/job_definition_spec.rb +++ b/spec/models/ci/job_definition_spec.rb @@ -24,17 +24,438 @@ 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 } + + context 'when empty tag_list' do + let(:config) { { tag_list: [] } } + + it { is_expected.to be_valid } + end + 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 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( + 'object property at `/unknown_property` is a disallowed additional property') + end + end + + context 'with invalid id_tokens' do + 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( + '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 + + 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 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: {} } } } } + + 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( + 'object at `/secrets/DATABASE_PASSWORD/vault` is missing required properties: path, field, engine') + end + end + + context 'with invalid tag_list' do + let(:config) { { tag_list: 'one-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: [ + 'value at `/tag_list` is not an array' + ] + ) + expect(job_definition).not_to be_valid + expect(job_definition.errors[:config]).to include( + 'value at `/tag_list` is not an array') + 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) + 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 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 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: {} } } } } + + 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 tag_list' do + let(:config) { { tag_list: 'one-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: [ + 'value at `/tag_list` is not an array' + ] + ) + expect(job_definition).to be_valid + 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 + 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 end 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 df4872a313a56f91d2413f017eba051fc9e8affc..b5ad8e1dfeb748ba3b99df3b042c91b104efbb68 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