mirror of
https://github.com/Termix-SSH/Termix.git
synced 2026-10-30 15:15:24 +00:00
fix(audit): store plugin entries with no acting user as a null user_id (#1556)
Plugin audit entries written outside a request used the literal "system" as user_id. That column references users.id, so the insert was refused (Postgres logs it as an FK violation) and the entry was silently dropped. Write null instead and keep "system" / plugin:<id> in username.
This commit is contained in:
1 parent
4c9e12f3b7
commit
abbfca2e94
7 files changed
+40
-9
No files matched your search
@@ -156,7 +156,7 @@ async function writeAudit(
|
|||||||
const { logAudit } = await import("../utils/audit-logger.js");
|
const { logAudit } = await import("../utils/audit-logger.js");
|
||||||
await logAudit({
|
await logAudit({
|
||||||
// Attribution comes from the runtime, never from the plugin.
|
// Attribution comes from the runtime, never from the plugin.
|
||||||
userId: getActor() ?? "system",
|
userId: getActor() ?? null,
|
||||||
username: `plugin:${manifest.id}`,
|
username: `plugin:${manifest.id}`,
|
||||||
action: `plugin_${options.action}`,
|
action: `plugin_${options.action}`,
|
||||||
resourceType: "plugin",
|
resourceType: "plugin",
|
||||||
@@ -965,12 +965,12 @@ export function createPluginContext(
|
|||||||
record: async (entry) => {
|
record: async (entry) => {
|
||||||
try {
|
try {
|
||||||
const { logAudit } = await import("../utils/audit-logger.js");
|
const { logAudit } = await import("../utils/audit-logger.js");
|
||||||
const actor = getActor() ?? "system";
|
const actor = getActor();
|
||||||
const meta = requestMeta(entry.request);
|
const meta = requestMeta(entry.request);
|
||||||
await logAudit({
|
await logAudit({
|
||||||
...meta,
|
...meta,
|
||||||
userId: actor,
|
userId: actor ?? null,
|
||||||
username: actor,
|
username: actor ?? "system",
|
||||||
action: entry.action,
|
action: entry.action,
|
||||||
resourceType: entry.resourceType ?? "plugin",
|
resourceType: entry.resourceType ?? "plugin",
|
||||||
resourceId: entry.resourceId ?? pluginId,
|
resourceId: entry.resourceId ?? pluginId,
|
||||||
|
|||||||
@@ -351,7 +351,7 @@ async function writePublicRouteAudit(
|
|||||||
try {
|
try {
|
||||||
const { logAudit } = await import("../utils/audit-logger.js");
|
const { logAudit } = await import("../utils/audit-logger.js");
|
||||||
await logAudit({
|
await logAudit({
|
||||||
userId: "system",
|
userId: null,
|
||||||
username: `plugin:${manifest.id}`,
|
username: `plugin:${manifest.id}`,
|
||||||
action: "plugin_http_public_routes",
|
action: "plugin_http_public_routes",
|
||||||
resourceType: "plugin",
|
resourceType: "plugin",
|
||||||
|
|||||||
@@ -111,7 +111,7 @@ async function auditRefusal(
|
|||||||
const { logAudit } = await import("../utils/audit-logger.js");
|
const { logAudit } = await import("../utils/audit-logger.js");
|
||||||
await logAudit({
|
await logAudit({
|
||||||
// Attribution comes from the runtime, never from the plugin.
|
// Attribution comes from the runtime, never from the plugin.
|
||||||
userId: getActor() ?? "system",
|
userId: getActor() ?? null,
|
||||||
username: `plugin:${pluginId}`,
|
username: `plugin:${pluginId}`,
|
||||||
action: `plugin_${action}`,
|
action: `plugin_${action}`,
|
||||||
resourceType: "plugin",
|
resourceType: "plugin",
|
||||||
|
|||||||
@@ -382,7 +382,7 @@ function auditPublicSocket(pluginId: string, path: string): void {
|
|||||||
void import("../utils/audit-logger.js")
|
void import("../utils/audit-logger.js")
|
||||||
.then(({ logAudit }) =>
|
.then(({ logAudit }) =>
|
||||||
logAudit({
|
logAudit({
|
||||||
userId: "system",
|
userId: null,
|
||||||
username: `plugin:${pluginId}`,
|
username: `plugin:${pluginId}`,
|
||||||
action: "plugin_ws_public_route",
|
action: "plugin_ws_public_route",
|
||||||
resourceType: "plugin",
|
resourceType: "plugin",
|
||||||
|
|||||||
@@ -77,6 +77,34 @@ describe("AuditLogRepository", () => {
|
|||||||
]);
|
]);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("stores entries with no acting user, and refuses ids that are not users", async () => {
|
||||||
|
const repo = await createRepository();
|
||||||
|
|
||||||
|
await repo.create({
|
||||||
|
userId: null,
|
||||||
|
username: "plugin:example",
|
||||||
|
action: "plugin_http_public_routes",
|
||||||
|
resourceType: "plugin",
|
||||||
|
success: true,
|
||||||
|
});
|
||||||
|
await expect(
|
||||||
|
repo.create({
|
||||||
|
userId: "system",
|
||||||
|
username: "system",
|
||||||
|
action: "cleanup",
|
||||||
|
resourceType: "plugin",
|
||||||
|
success: true,
|
||||||
|
}),
|
||||||
|
).rejects.toThrow();
|
||||||
|
|
||||||
|
const page = await repo.listPage({ filters: {}, limit: 10, offset: 0 });
|
||||||
|
expect(page.logs).toHaveLength(1);
|
||||||
|
expect(page.logs[0]).toMatchObject({
|
||||||
|
userId: null,
|
||||||
|
username: "plugin:example",
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
it("deletes logs by user id and only runs write hook for deleted rows", async () => {
|
it("deletes logs by user id and only runs write hook for deleted rows", async () => {
|
||||||
let writeCount = 0;
|
let writeCount = 0;
|
||||||
const repo = await createRepository(() => {
|
const repo = await createRepository(() => {
|
||||||
|
|||||||
@@ -73,7 +73,8 @@ describe("ctx.audit.record", () => {
|
|||||||
const ctx = contextFor("audit-fixture");
|
const ctx = contextFor("audit-fixture");
|
||||||
await ctx.audit.record({ action: "cleanup", success: false });
|
await ctx.audit.record({ action: "cleanup", success: false });
|
||||||
expect(auditEntries[0]).toMatchObject({
|
expect(auditEntries[0]).toMatchObject({
|
||||||
userId: "system",
|
userId: null,
|
||||||
|
username: "system",
|
||||||
resourceType: "plugin",
|
resourceType: "plugin",
|
||||||
resourceId: "audit-fixture",
|
resourceId: "audit-fixture",
|
||||||
resourceName: "Audit Fixture",
|
resourceName: "Audit Fixture",
|
||||||
|
|||||||
@@ -20,7 +20,9 @@ export async function getAuditUsername(userId: string): Promise<string> {
|
|||||||
}
|
}
|
||||||
|
|
||||||
export interface AuditLogParams {
|
export interface AuditLogParams {
|
||||||
userId: string;
|
// Null when no user acted (a plugin starting up, background work): the
|
||||||
|
// column references users.id, so anything else would be refused.
|
||||||
|
userId: string | null;
|
||||||
username: string;
|
username: string;
|
||||||
action: string;
|
action: string;
|
||||||
resourceType: string;
|
resourceType: string;
|
||||||
|
|||||||
Reference in new issue
Block a user