From 495c5c510132f3b789c91b5b67a2edc727cb354c Mon Sep 17 00:00:00 2001 From: eeminionn <109454414+eeminionn@users.noreply.github.com> Date: Thu, 30 Jul 2026 14:44:39 -0400 Subject: [PATCH 1/2] Fix secure evaluator access --- supabase/functions/_shared/execute.ts | 18 +++-- supabase/functions/_shared/secure-variants.ts | 60 ++++++++++++++ supabase/functions/mission-admin/index.ts | 66 ++++++--------- .../202607300004_secure_variant_rpc.sql | 80 +++++++++++++++++++ supabase/tests/rls.sql | 47 ++++++++++- v2/src/services/runner.ts | 34 +++++++- v2/tests/runner-errors.test.ts | 38 +++++++++ v2/tests/security-contract.test.ts | 27 ++++++- 8 files changed, 317 insertions(+), 53 deletions(-) create mode 100644 supabase/functions/_shared/secure-variants.ts create mode 100644 supabase/migrations/202607300004_secure_variant_rpc.sql create mode 100644 v2/tests/runner-errors.test.ts diff --git a/supabase/functions/_shared/execute.ts b/supabase/functions/_shared/execute.ts index fdd83a6..446d40d 100644 --- a/supabase/functions/_shared/execute.ts +++ b/supabase/functions/_shared/execute.ts @@ -2,6 +2,8 @@ import { createClient } from "npm:@supabase/supabase-js@2"; import { corsHeaders, jsonResponse } from "./cors.ts"; import { executeJudge0 } from "./judge0.ts"; import { syncSubmissionToGitHub } from "./github-submissions.ts"; +import { getSecureVariant } from "./secure-variants.ts"; +import type { SecureVariantData } from "./secure-variants.ts"; import type { Language, MissionTest } from "./types.ts"; interface ExecuteBody { @@ -219,16 +221,16 @@ export function createExecutionHandler(kind: "run" | "submit") { const variant = variantData as unknown as VariantRow; let hiddenTests: MissionTest[] = []; if (kind === "submit") { - const { data: secure, error: secureError } = await admin - .schema("private") - .from("mission_variants_secure") - .select("hidden_tests") - .eq("variant_id", variant.id) - .single(); - if (secureError || !secure) { + let secure: SecureVariantData | null = null; + try { + secure = await getSecureVariant(admin, variant.id); + } catch { + // The secure helper logs the database error without exposing it to students. + } + if (!secure) { return jsonResponse(request, { error: "Tests privados no disponibles." }, 503); } - hiddenTests = secure.hidden_tests as MissionTest[]; + hiddenTests = secure.hiddenTests; } const result = await executeJudge0( diff --git a/supabase/functions/_shared/secure-variants.ts b/supabase/functions/_shared/secure-variants.ts new file mode 100644 index 0000000..2571672 --- /dev/null +++ b/supabase/functions/_shared/secure-variants.ts @@ -0,0 +1,60 @@ +import type { SupabaseClient } from "npm:@supabase/supabase-js@2"; +import type { MissionTest } from "./types.ts"; + +export interface SecureVariantData { + referenceSolution: string; + hiddenTests: MissionTest[]; +} + +interface SecureVariantRpcRow { + reference_solution: string; + hidden_tests: MissionTest[]; +} + +export async function getSecureVariant( + admin: SupabaseClient, + variantId: string, +): Promise { + const { data, error } = await admin.rpc("get_mission_variant_secure", { + p_variant_id: variantId, + }); + if (error) { + console.error("Secure variant read failed.", { + variantId, + code: error.code, + message: error.message, + }); + throw new Error("No se pudieron leer los datos privados de la misión."); + } + + const rows = + (Array.isArray(data) ? data : data ? [data] : []) as SecureVariantRpcRow[]; + const secure = rows[0]; + return secure + ? { + referenceSolution: secure.reference_solution, + hiddenTests: secure.hidden_tests, + } + : null; +} + +export async function upsertSecureVariant( + admin: SupabaseClient, + variantId: string, + referenceSolution: string, + hiddenTests: MissionTest[], +): Promise { + const { error } = await admin.rpc("upsert_mission_variant_secure", { + p_variant_id: variantId, + p_reference_solution: referenceSolution, + p_hidden_tests: hiddenTests, + }); + if (error) { + console.error("Secure variant write failed.", { + variantId, + code: error.code, + message: error.message, + }); + throw new Error("No se pudieron guardar los datos privados de la misión."); + } +} diff --git a/supabase/functions/mission-admin/index.ts b/supabase/functions/mission-admin/index.ts index d7c0c24..6ee03da 100644 --- a/supabase/functions/mission-admin/index.ts +++ b/supabase/functions/mission-admin/index.ts @@ -1,6 +1,10 @@ import { createClient } from "npm:@supabase/supabase-js@2"; import { corsHeaders, jsonResponse } from "../_shared/cors.ts"; import { executeJudge0 } from "../_shared/judge0.ts"; +import { + getSecureVariant, + upsertSecureVariant, +} from "../_shared/secure-variants.ts"; import type { Language, MissionExample, @@ -122,12 +126,7 @@ Deno.serve(async (request) => { (data ?? []).map(async (version) => { const variants = await Promise.all( (version.mission_variants ?? []).map(async (variant) => { - const { data: secure } = await admin - .schema("private") - .from("mission_variants_secure") - .select("reference_solution, hidden_tests") - .eq("variant_id", variant.id) - .single(); + const secure = await getSecureVariant(admin, variant.id); return { id: variant.id, language: variant.language, @@ -136,8 +135,8 @@ Deno.serve(async (request) => { examples: variant.examples ?? [], publicTests: variant.public_tests, hiddenTestCount: variant.hidden_test_count, - referenceSolution: secure?.reference_solution ?? "", - hiddenTests: secure?.hidden_tests ?? [], + referenceSolution: secure?.referenceSolution ?? "", + hiddenTests: secure?.hiddenTests ?? [], }; }), ); @@ -179,13 +178,8 @@ Deno.serve(async (request) => { } const completeVariants = await Promise.all( variants.map(async (variant) => { - const { data: secure, error } = await admin - .schema("private") - .from("mission_variants_secure") - .select("reference_solution, hidden_tests") - .eq("variant_id", variant.id) - .single(); - if (error || !secure) throw new Error("Datos privados incompletos."); + const secure = await getSecureVariant(admin, variant.id); + if (!secure) throw new Error("Datos privados incompletos."); return { ...variant, secure }; }), ); @@ -211,15 +205,12 @@ Deno.serve(async (request) => { .select("id") .single(); if (error || !inserted) throw error ?? new Error("No se pudo copiar la variante."); - const { error: secureError } = await admin - .schema("private") - .from("mission_variants_secure") - .insert({ - variant_id: inserted.id, - reference_solution: variant.secure.reference_solution, - hidden_tests: variant.secure.hidden_tests, - }); - if (secureError) throw secureError; + await upsertSecureVariant( + admin, + inserted.id, + variant.secure.referenceSolution, + variant.secure.hiddenTests, + ); } } @@ -254,13 +245,8 @@ Deno.serve(async (request) => { if (variantError || !variant) { return jsonResponse(request, { error: "Solución no encontrada." }, 404); } - const { data: secure, error: secureError } = await admin - .schema("private") - .from("mission_variants_secure") - .select("reference_solution") - .eq("variant_id", variant.id) - .single(); - if (secureError || !secure) { + const secure = await getSecureVariant(admin, variant.id); + if (!secure) { return jsonResponse( request, { error: "Solución privada no disponible." }, @@ -273,7 +259,7 @@ Deno.serve(async (request) => { missionVersion: body.missionVersion, language: body.language, expectedSignature: variant.expected_signature ?? "", - referenceSolution: secure.reference_solution, + referenceSolution: secure.referenceSolution, explanation: "Esta implementación de referencia cumple el contrato y pasa los tests públicos y privados de la versión seleccionada.", }, @@ -410,16 +396,12 @@ Deno.serve(async (request) => { .select("id") .single(); if (error || !variant) throw error; - const { error: secureError } = await admin - .schema("private") - .from("mission_variants_secure") - .upsert({ - variant_id: variant.id, - reference_solution: body.referenceSolution, - hidden_tests: body.hiddenTests, - updated_at: new Date().toISOString(), - }); - if (secureError) throw secureError; + await upsertSecureVariant( + admin, + variant.id, + body.referenceSolution, + body.hiddenTests, + ); return jsonResponse(request, { drafts: await loadDrafts() }); } diff --git a/supabase/migrations/202607300004_secure_variant_rpc.sql b/supabase/migrations/202607300004_secure_variant_rpc.sql new file mode 100644 index 0000000..980ba6c --- /dev/null +++ b/supabase/migrations/202607300004_secure_variant_rpc.sql @@ -0,0 +1,80 @@ +begin; + +create or replace function public.get_mission_variant_secure( + p_variant_id uuid +) +returns table ( + reference_solution text, + hidden_tests jsonb +) +language plpgsql +security definer +set search_path = '' +as $$ +begin + if auth.role() is distinct from 'service_role' then + raise exception 'Service role required.' using errcode = '42501'; + end if; + + return query + select + secure.reference_solution, + secure.hidden_tests + from private.mission_variants_secure secure + where secure.variant_id = p_variant_id; +end; +$$; + +create or replace function public.upsert_mission_variant_secure( + p_variant_id uuid, + p_reference_solution text, + p_hidden_tests jsonb +) +returns void +language plpgsql +security definer +set search_path = '' +as $$ +begin + if auth.role() is distinct from 'service_role' then + raise exception 'Service role required.' using errcode = '42501'; + end if; + if p_reference_solution is null + or octet_length(p_reference_solution) > 65536 + then + raise exception 'Invalid reference solution.' using errcode = '22023'; + end if; + if p_hidden_tests is null or jsonb_typeof(p_hidden_tests) <> 'array' then + raise exception 'Hidden tests must be an array.' using errcode = '22023'; + end if; + + insert into private.mission_variants_secure ( + variant_id, + reference_solution, + hidden_tests, + updated_at + ) + values ( + p_variant_id, + p_reference_solution, + p_hidden_tests, + now() + ) + on conflict (variant_id) do update set + reference_solution = excluded.reference_solution, + hidden_tests = excluded.hidden_tests, + updated_at = excluded.updated_at; +end; +$$; + +revoke all on function public.get_mission_variant_secure(uuid) +from public, anon, authenticated; +revoke all on function public.upsert_mission_variant_secure(uuid, text, jsonb) +from public, anon, authenticated; + +grant execute on function public.get_mission_variant_secure(uuid) +to service_role; +grant execute on function public.upsert_mission_variant_secure(uuid, text, jsonb) +to service_role; + +commit; diff --git a/supabase/tests/rls.sql b/supabase/tests/rls.sql index 25c2550..85dac45 100644 --- a/supabase/tests/rls.sql +++ b/supabase/tests/rls.sql @@ -1,7 +1,7 @@ begin; create extension if not exists pgtap with schema extensions; -select plan(15); +select plan(18); insert into auth.users ( id, instance_id, aud, role, email, encrypted_password, @@ -213,6 +213,39 @@ select is( 'requesting changes migrates the student to the current mission version' ); +set local role service_role; +select set_config('request.jwt.claim.role', 'service_role', true); + +select is( + ( + select count(*) + from public.get_mission_variant_secure( + ( + select variant.id + from public.mission_variants variant + join public.mission_versions version + on version.id = variant.mission_version_id + where version.mission_id = 'p1-01-la-once' + order by version.version desc + limit 1 + ) + ) + ), + 1::bigint, + 'service role reads one secure variant through the narrow RPC' +); + +select ok( + has_function_privilege( + 'service_role', + 'public.upsert_mission_variant_secure(uuid,text,jsonb)', + 'EXECUTE' + ), + 'service role can maintain secure variants through the write RPC' +); + +reset role; + set local role authenticated; select set_config( 'request.jwt.claim.sub', @@ -279,6 +312,18 @@ select throws_ok( 'students cannot access private tests or solutions' ); +select throws_ok( + $$ + select * + from public.get_mission_variant_secure( + '00000000-0000-0000-0000-000000000000' + ) + $$, + '42501', + 'permission denied for function get_mission_variant_secure', + 'students cannot call the secure variant RPC' +); + select throws_ok( $$ insert into public.attempts ( diff --git a/v2/src/services/runner.ts b/v2/src/services/runner.ts index 10bb426..68c41b4 100644 --- a/v2/src/services/runner.ts +++ b/v2/src/services/runner.ts @@ -16,6 +16,38 @@ interface WorkerResponse { stack?: string; } +export async function edgeFunctionErrorMessage( + error: unknown, +): Promise { + const fallback = + error instanceof Error + ? error.message + : typeof error === "string" + ? error + : "El ejecutor remoto no respondió."; + const context = + typeof error === "object" && error !== null && "context" in error + ? error.context + : null; + if (!(context instanceof Response)) return fallback; + + try { + const payload = (await context.clone().json()) as { + error?: unknown; + message?: unknown; + }; + if (typeof payload.error === "string" && payload.error.trim()) { + return payload.error; + } + if (typeof payload.message === "string" && payload.message.trim()) { + return payload.message; + } + } catch { + // Keep the SDK message when the function did not return JSON. + } + return fallback; +} + function localRunStatus(response: WorkerResponse): RunStatus { if (!response.ok) return "runtime_error"; return response.tests.length > 0 && response.tests.every((entry) => entry.passed) @@ -120,7 +152,7 @@ export async function runMissionCode(request: RunnerRequest): Promise id: crypto.randomUUID(), status: "provider_error", stdout: "", - stderr: error?.message ?? "El ejecutor remoto no respondió.", + stderr: await edgeFunctionErrorMessage(error), diagnostics: [], tests: [], createdAt: new Date().toISOString(), diff --git a/v2/tests/runner-errors.test.ts b/v2/tests/runner-errors.test.ts new file mode 100644 index 0000000..cdc16d7 --- /dev/null +++ b/v2/tests/runner-errors.test.ts @@ -0,0 +1,38 @@ +// @vitest-environment node + +import { describe, expect, it } from "vitest"; +import { edgeFunctionErrorMessage } from "@/services/runner"; + +describe("edge function errors", () => { + it("uses the backend JSON error instead of the generic SDK message", async () => { + const error = Object.assign( + new Error("Edge Function returned a non-2xx status code"), + { + context: new Response( + JSON.stringify({ error: "Tests privados no disponibles." }), + { + status: 503, + headers: { "Content-Type": "application/json" }, + }, + ), + }, + ); + + await expect(edgeFunctionErrorMessage(error)).resolves.toBe( + "Tests privados no disponibles.", + ); + }); + + it("keeps the SDK message when the response is not JSON", async () => { + const error = Object.assign( + new Error("Servicio temporalmente no disponible"), + { + context: new Response("upstream failure", { status: 502 }), + }, + ); + + await expect(edgeFunctionErrorMessage(error)).resolves.toBe( + "Servicio temporalmente no disponible", + ); + }); +}); diff --git a/v2/tests/security-contract.test.ts b/v2/tests/security-contract.test.ts index b812e87..9b013c8 100644 --- a/v2/tests/security-contract.test.ts +++ b/v2/tests/security-contract.test.ts @@ -47,6 +47,20 @@ describe("security contracts", () => { resolve("supabase/functions/mission-admin/index.ts"), "utf8", ); + const execute = readFileSync( + resolve("supabase/functions/_shared/execute.ts"), + "utf8", + ); + const secureVariants = readFileSync( + resolve("supabase/functions/_shared/secure-variants.ts"), + "utf8", + ); + const secureRpc = readFileSync( + resolve( + "supabase/migrations/202607300004_secure_variant_rpc.sql", + ), + "utf8", + ); const judge0 = readFileSync( resolve("supabase/functions/_shared/judge0.ts"), "utf8", @@ -58,7 +72,18 @@ describe("security contracts", () => { expect(contracts).toContain("inline_comments"); expect(missionAdmin).toContain('.in("role", ["owner", "mentor"])'); expect(missionAdmin).toContain('body.action === "get-solution"'); - expect(missionAdmin).toContain('.schema("private")'); + expect(missionAdmin).not.toContain('.schema("private")'); + expect(execute).not.toContain('.schema("private")'); + expect(secureVariants).toContain( + 'admin.rpc("get_mission_variant_secure"', + ); + expect(secureVariants).toContain( + 'admin.rpc("upsert_mission_variant_secure"', + ); + expect(secureRpc).toContain("security definer"); + expect(secureRpc).toContain("set search_path = ''"); + expect(secureRpc).toContain("from public, anon, authenticated"); + expect(secureRpc).toContain("to service_role"); expect(judge0).toContain( "Revisa los casos límite y el contrato de la misión.", ); From 9ad844d0502564e0eb29c3b470eb93cae53b00ef Mon Sep 17 00:00:00 2001 From: eeminionn <109454414+eeminionn@users.noreply.github.com> Date: Thu, 30 Jul 2026 14:48:26 -0400 Subject: [PATCH 2/2] Grant explicit backend database access --- supabase/migrations/202607300004_secure_variant_rpc.sql | 6 ++++++ v2/tests/security-contract.test.ts | 3 +++ 2 files changed, 9 insertions(+) diff --git a/supabase/migrations/202607300004_secure_variant_rpc.sql b/supabase/migrations/202607300004_secure_variant_rpc.sql index 980ba6c..15e400f 100644 --- a/supabase/migrations/202607300004_secure_variant_rpc.sql +++ b/supabase/migrations/202607300004_secure_variant_rpc.sql @@ -72,6 +72,12 @@ from public, anon, authenticated; revoke all on function public.upsert_mission_variant_secure(uuid, text, jsonb) from public, anon, authenticated; +grant usage on schema public to service_role; +grant select, insert, update, delete on all tables in schema public +to service_role; +grant usage, select on all sequences in schema public +to service_role; + grant execute on function public.get_mission_variant_secure(uuid) to service_role; grant execute on function public.upsert_mission_variant_secure(uuid, text, jsonb) diff --git a/v2/tests/security-contract.test.ts b/v2/tests/security-contract.test.ts index 9b013c8..3445051 100644 --- a/v2/tests/security-contract.test.ts +++ b/v2/tests/security-contract.test.ts @@ -83,6 +83,9 @@ describe("security contracts", () => { expect(secureRpc).toContain("security definer"); expect(secureRpc).toContain("set search_path = ''"); expect(secureRpc).toContain("from public, anon, authenticated"); + expect(secureRpc).toContain( + "grant select, insert, update, delete on all tables in schema public", + ); expect(secureRpc).toContain("to service_role"); expect(judge0).toContain( "Revisa los casos límite y el contrato de la misión.",